Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8282475

SafeFetch should not rely on existence of Thread::current

XMLWordPrintable

    • Icon: Enhancement Enhancement
    • Resolution: Duplicate
    • Icon: P4 P4
    • tbd
    • 11, 16, 17, 18, 19
    • hotspot
    • aarch64
    • os_x

      Since JDK-8253795, SafeFetch on MacOS aarch64 needs the current Thread to function:

      ```
      // Safefetch allows to load a value from a location that's not known
      // to be valid. If the load causes a fault, the error value is returned.
      inline int SafeFetch32(int* adr, int errValue) {
        assert(StubRoutines::SafeFetch32_stub(), "stub not yet generated");
      #if defined(__APPLE__) && defined(AARCH64)
        Thread* thread = Thread::current_or_null_safe();
        assert(thread != NULL, "required for W^X management");
        ThreadWXEnable wx(WXExec, thread);
      #endif // __APPLE__ && AARCH64
        return StubRoutines::SafeFetch32_stub()(adr, errValue);
      }
      ```

      This is not good, since SafeFetch, by design, is supposed to be safe and never crash as long as the SafeFetch routines have been initialized (StubRoutines::CanUseSafeFetch() == true). By definition, its job is to be safe. And it can be used at unpredictable times (error handling, AsyncGetCallTrace etc) so no guarantee that we always have a current thread.

      Also, this is an assert, so in release builds we would just crash at that point.

      Thread::current is needed here to save/restore the state of the WX flag for the current thread. I assume this is done to enable the safe nesting of WX state switches. Would be interesting to know whether
      1) We ever do nest ThreadWXEnable - is this realistic? If not, we don't need to know the current WX flag state, we could just switch it on before calling into generated code, off when leaving.
      2) whether there is a way to query the current thread's wx state from the OS instead of storing it into Thread (I did not find an API for that but maybe I'm looking at the wrong place)

      If this cannot be changed for MacOS aarch64, we should make "Thread::current != NULL" a precondition for CanUseSafeFetch() on that platform. Callers are supposed to call CanUseSafeFetch before using SafeFetch, so that would at least prevent asserts.

            stuefe Thomas Stuefe
            stuefe Thomas Stuefe
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated:
              Resolved: