RFR: 8313796: AsyncGetCallTrace crash on unreadable interpreter method pointer [v2]

[Richard Startin]

On Mon, 7 Aug 2023 21:23:48 GMT, Johannes Bechberger <jbechberger at openjdk.org> wrote:

>>> Thanks for raising this. It doesn't look like `SafeFetchN` is async signal safe because it calls into this:
>>> 
>>> ```c++
>>> template <class T>
>>> ATTRIBUTE_NO_ASAN static bool _SafeFetchXX_internal(const T *adr, T* result) {
>>> 
>>> ```
>>> 
>>> This means we shouldn't be calling `os:: is_readable_pointer` either here, because it calls `SafeFetch32` which calls into the same function above. I had originally intended to simply perform a null check here (because that's the condition we've actually observed) and will push a change to revert to the null check.
>> 
>> That's not how `SafeFetchN` is normally implemented. On Linux and BSD it's more like
>> 
>> 
>>     # Support for intptr_t SafeFetchN(intptr_t* address, intptr_t defaultval);
>>     #
>>     #  x1 : address
>>     #  x0 : defaultval
>> SafeFetchN_impl:
>> _SafeFetchN_fault:
>>     ldr      x0, [x0]
>>     ret
>> _SafeFetchN_continuation:
>>     mov      x0, x1
>>     ret
>> 
>> 
>> which should be fine.
>
> @tstuefe implemented explicitly for being signal safe.

OK let's go with your suggestion, thanks for explaining. I'm actually skeptical this can actually be a non-null bad pointer, as we've only seen this crash happen once, and the pointer was null in that instance. But this solution looks robust, so thanks for suggesting it.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15178#discussion_r1286397215