RFR: 8273967: gtest os.dll_address_to_function_and_library_name_vm fails on macOS12
Daniel D.Daugherty
dcubed at openjdk.java.net
Thu Nov 4 17:29:16 UTC 2021
On Wed, 3 Nov 2021 21:14:17 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:
>> macOS12 has changed the dladdr() function to accept "-1" as a valid address and
>> we have functions that use dladdr() to convert DLL addresses into function or
>> library names. We also have a gtest that verifies that "-1" is not a valid value to use
>> as a symbol address.
>>
>> As you might imagine, existing code that uses `os::dll_address_to_function_name()`
>> or `os::dll_address_to_library_name()` can get quite confused (and sometimes crash)
>> if an `addr` parameter of `-1` was allowed to be used.
>>
>> I've also made two cleanup changes as part of this fix:
>>
>> 1) In `src/hotspot/os/bsd/os_bsd.cpp` there is some macOS specific code that should
>> be properly `#ifdef`'ed. There is also some code that makes sense for ELF format
>> files, but not for Mach-O format files so that code needs to be excluded on macOS.
>>
>> 2) In `src/hotspot/share/runtime/os.cpp` I noticed a simple typo in a comment on an
>> `#endif` that I fixed. That typo does not appear anywhere else in the HotSpot code
>> base so I'd like to fix it with this bug ID since I'm in related areas.
>>
>> This fix has been tested with Mach5 Tier[1-6].
>
> src/hotspot/os/bsd/os_bsd.cpp line 929:
>
>> 927: #endif
>> 928:
>> 929: #if defined(__APPLE__)
>
> Why not just do:
>
> `#else`
>
> here instead and collapse these 3 lines into 1?
Hmmm... I'll take a look at doing that.
> src/hotspot/os/bsd/os_bsd.cpp line 930:
>
>> 928:
>> 929: #if defined(__APPLE__)
>> 930: char localbuf[MACH_MAXSYMLEN];
>
> This `__APPLE__` section is the only one, that I can see, using `MACH_MAXSYMLEN`, why not move:
>
>
> #if defined(__APPLE__)
> #define MACH_MAXSYMLEN 256
> #endif
>
>
> here (i.e. just the `#define MACH_MAXSYMLEN 256` and minimize the need for` __APPLE__` sections?
Hmmm.... I'll take a look at cleaning this up a bit.
> src/hotspot/os/bsd/os_bsd.cpp line 964:
>
>> 962: if (offset) *offset = -1;
>> 963: return false;
>> 964: }
>
> Do we need this here? Wouldn't the earlier call to `Decoder::decode(addr, localbuf, MACH_MAXSYMLEN, offset, dlinfo.dli_fbase))` catch this with `ShouldNotReachHere`?
That's the 5-parameter version of decode() and it doesn't have `ShouldNotReachHere`.
So if that code site is called and returns `false`, then we get into
`dll_address_to_library_name()` and reach this `dladdr()` call which
will accept the "-1"...
-------------
PR: https://git.openjdk.java.net/jdk/pull/6193
More information about the serviceability-dev
mailing list