RFR: 8282306: os::is_first_C_frame(frame*) crashes on invalid link access [v13]

[Thomas Stuefe]

On Tue, 15 Mar 2022 07:50:10 GMT, Johannes Bechberger <duke at openjdk.java.net> wrote:

>> src/hotspot/share/runtime/safefetch.inline.hpp line 61:
>> 
>>> 59:     return false;
>>> 60:   }
>>> 61: #endif // __APPLE__ && AARCH64
>> 
>> Totally valid way to work around JDK-8282475, but note that `os::is_readable_pointer()` defaults to true for `CanSafeFetch=false` (see https://github.com/openjdk/jdk/blob/6013d09e82693a1c07cf0bf6daffd95114b3cbfa/src/hotspot/share/runtime/os.cpp#L1042-L1046)
>> 
>> The story behind that is that there are more use cases of `is_readable_memory()` where - if one cannot use SafeFetch - it makes sense to optimistically assume the location is readable. E.g. in error handling, where secondary crashes are annoying but not deadly. The contract is that if you really care about this, use CanUseSafeFetch beforehand (usually it was fine to ignore it since CanUseSafeFetch only affected a small time window during VM initialization).
>> 
>> With your patch, it means that now os::is_readable_memory always returns true if Thread::current is NULL. So in that case, we bypass your protection. I think that is acceptable for now. Once JDK-8282475 is solved we can remove this again.
>> 
>> I wondered whether this affects your gtest test case, but in that test case Thread::current is not NULL because its a TEST_VM case, so we are in the VM.
>
> I could add a non VM test case? Or is it to difficult?

Would not work, since signal handling is not present. I think not worth the effort. Lets ship this, has been cooking long enough.

In theory, you could temporarily reset Thread::current to null, but that's hackish and possibly exposes you to a whole other range of follow up problems. Also I am currently extending SafeFetch tests in the course of JDK-8282475 and will test Thread::current=null there, so it will be covered eventually.

-------------

PR: https://git.openjdk.java.net/jdk/pull/7591