On Mon, 1 Nov 2021 15:32:54 GMT, Daniel D. Daugherty <dcu...@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].
Hi Daniel,
looks good. Small remarks below. I leave it up to you if you take my
suggestions.
Cheers, Thomas
src/hotspot/os/bsd/os_bsd.cpp line 902:
> 900: return false;
> 901: }
> 902: #endif
We use dladdr() in several places in this code. I wonder whether it would make
sense to fix all of those with a wrapper instead:
static int my_dladdr(const void* addr, Dl_info* info) {
if (addr != (void*)-1) {
return dladdr(addr, info);
} else {
// add comment here
return 0;
}
}
#define dladdr my_dladdr
src/hotspot/os/bsd/os_bsd.cpp line 918:
> 916: // ranges like ELF. That makes sense since Mach-O can contain
> binaries for
> 917: // than one instruction set so there can be more than one address
> range for
> 918: // each "file".
Small nit, it seems confusing to have a Mac-specific comment in the BSD
section.
Maybe this would be better in MachDecoder? E.g. implement the 6-arg version of
decode() but stubbed out returning false, and with your comment there.
-------------
PR: https://git.openjdk.java.net/jdk/pull/6193
