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