RFR: 8331725: ubsan: pc may not always be the entry point for a VtableStub
Axel Boldt-Christmas
aboldtch at openjdk.org
Mon Jul 1 10:24:29 UTC 2024
`VtableStub* VtableStubs::entry_point(address pc)` has multiple issues causing undefined behaviour. Most of it stems from the fact that the pc sent in may sometimes not be the entry point of a `VtableStub`. We check this by doing a lookup in a hash table, but to generate the hash we need to extract data from the `VtableStub`. The original implementation simply cast the address to a `*VtableStub` and called the member function getters. This caused both unaligned accesses and bad bool values, all symptoms of the fact that we reinterpret_cast the memory as a `VtableStub`.
This change will instead look at the raw bytes when generating the hash which neither suffers from unaligned access nor bad reinterpret_casts.
My initial patch did not create an `enum class` to represent the boolean value but instead looked at the object representation of the `bool` field. However this suffers from the fact that the object representation of `false` is implementation defined. I assumed it to be all 0 bits. It was also required unfortunate extra logic to handle the cases when `sizeof(bool) != 1`. Alt patch: https://github.com/openjdk/jdk/commit/b20140f49983b1903f9038e4e47def792c4493ac
There may be some better name than `unsafe_hash`. Suggestions?
Verified that this patch resolves the UB.
Running testing.
Side note:
There is still an unaligned access issue on aarch64 because of the way we construct `VtableStub`. We align the chunk where we create the `VtableStub` so that the `<VtableStub address> + sizeof(VtableStub)` is aligned to the platforms code entry alignment. On all platforms but `aarch64` `<VtableStub address> + sizeof(VtableStub) % pd_code_alignment == 0` implies `<VtableStub address> % alignof(VtableStub) == 0`.
https://github.com/xmas92/jdk/blob/beea5305b071820e2b128a55c5ca384caf470fdd/src/hotspot/share/code/vtableStubs.hpp#L168-L171
Also note that there are already assumptions in the JVM about the object representation of bool. On `x86` `MacroAssembler::testbool` assumes that `false` is all 0 bits, and in addition to that `MacroAssembler::movbool` assumes that `1` is `true`.
-------------
Commit messages:
- 8331725: ubsan: pc may not always be the entry point for a VtableStub
Changes: https://git.openjdk.org/jdk/pull/19968/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19968&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8331725
Stats: 38 lines in 2 files changed: 24 ins; 5 del; 9 mod
Patch: https://git.openjdk.org/jdk/pull/19968.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19968/head:pull/19968
PR: https://git.openjdk.org/jdk/pull/19968
More information about the hotspot-compiler-dev
mailing list