8357155: [asan] ZGC does not work #25549
Conversation
|
👋 Welcome back mbaesken! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
openjdk
bot
changed the title
| @@ -91,10 +91,14 @@ static size_t probe_valid_max_address_bit() { | |||
| size_t ZPlatformAddressOffsetBits() { | |||
| static const size_t valid_max_address_offset_bits = probe_valid_max_address_bit() + 1; | |||
| const size_t max_address_offset_bits = valid_max_address_offset_bits - 3; | |||
| #ifdef ADDRESS_SANITIZER | |||
| return max_address_offset_bits; | |||
There was a problem hiding this comment.
I think this actually has to be
return MIN2(valid_max_address_offset_bits, 44);Because the way we probe we may otherwise return 45 here. Which could result in more than 44 bits in a ZOffset which our internal data structures cannot handle. Hopefully this still works for ASAN on PPC. (The -3 is a left over from non-generational ZGC). Aarch64 could do the same, but it does not have this issue as it starts its probing at bit 46, not bit 47.
Side note: This makes me realise that there probably is a bug here on PPC and RISCV if running on a NUMA machine with more than 8 TB heap. As after ZGlobalsPointers::min_address_offset_request() was introduced we can return 45 from this function.
| @@ -94,10 +94,14 @@ static size_t probe_valid_max_address_bit() { | |||
| size_t ZPlatformAddressOffsetBits() { | |||
| static const size_t valid_max_address_offset_bits = probe_valid_max_address_bit() + 1; | |||
| const size_t max_address_offset_bits = valid_max_address_offset_bits - 3; | |||
| #ifdef ADDRESS_SANITIZER | |||
| return max_address_offset_bits; | |||
There was a problem hiding this comment.
I was hoping this could work for Linux with 47/48 bit aarch64 VMA. But it is unclear how ASAN selects its mappings on such platforms.
On 39/42 bit VMA returning MIN2(valid_max_address_offset_bits, 44) as I suggested in the PPC function may be a better best effort, as we are using addresses where we actually probed that reservations could be possible). Or even MIN2(valid_max_address_offset_bits - 1, 44). Feel free to try it out, but I think this is otherwise an alright approach until we implement a better heap base selection strategy where we can test multiple base candidates.
Many (all?) ZGC related jtreg tests do not work when the JDK is built with address sanitizer asan enabled (configure flag --enable-asan).
This can be seen on SUSE Linux x86_64 and also on ppc64le , opt binaries were used.
It has been suggested to do a workaround - 'But I think that simply adapting the zAddress_[...].cpp implementations to always select the largest heap base would go a long way for providing ASAN compatibility.'
This seems to work nicely on x86_64 and ppc64le, however the zgc related tests still fail on Linux aarch64 (should I exclude this platform from my patch?) .
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25549/head:pull/25549$ git checkout pull/25549Update a local copy of the PR:
$ git checkout pull/25549$ git pull https://git.openjdk.org/jdk.git pull/25549/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 25549View PR using the GUI difftool:
$ git pr show -t 25549Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25549.diff
Using Webrev
Link to Webrev Comment