RFR: 8344880: AArch64: Add compile time check for class offsets

Thomas Stuefe stuefe at openjdk.org
Thu Dec 12 19:54:43 UTC 2024


On Fri, 6 Dec 2024 23:57:41 GMT, Chad Rakoczy <duke at openjdk.org> wrote:

> [JDK-8344880](https://bugs.openjdk.org/browse/JDK-8344880)
> 
> Adds compile time checks for str/ldr instructions to verify that the immediate offset will fit. This adds static_assert for constant offsets that are checked at compile time. The macro offset_of is not constexpr so instead the class size is checked. If the size of a class fits into a memory instructions then any offset in it will fit.

Some drive-by comments. Using sizeof class is a smart workaround that is fine as long as we don't use `create_imm_offset`  to check offsets that are beyond sizeof its class. E.g. things like InstanceKlass::start_of_itable().

src/hotspot/cpu/aarch64/assembler_aarch64.hpp line 675:

> 673:   }
> 674: 
> 675:   #define create_imm_offset(klass, field_offset_func) \

I would avoid "klass" as name, since it is usually implied to mean an object of class `Klass`.

src/hotspot/cpu/aarch64/assembler_aarch64.hpp line 680:

> 678:     static_assert(Address::offset_ok_for_immed_compile_time<max_possible_offset, 0>(), "must fit in instruction"); \
> 679:     return klass::field_offset_func(); \
> 680:   }())

This could be done without a Lambda, which subjectively would be a bit simpler, by using the comma operator like this:


  template <class K> static void class_size_check() {
    constexpr size_t max_possible_offset = sizeof(K); \
    static_assert(Address::offset_ok_for_immed_compile_time<max_possible_offset, 0>(), "must fit in instruction"); \
  }
  #define create_imm_offset(klass, field_offset_func) \
    (Address::class_size_check<klass>(), klass::field_offset_func())

-------------

PR Review: https://git.openjdk.org/jdk/pull/22623#pullrequestreview-2500588169
PR Review Comment: https://git.openjdk.org/jdk/pull/22623#discussion_r1882758934
PR Review Comment: https://git.openjdk.org/jdk/pull/22623#discussion_r1882778201


More information about the shenandoah-dev mailing list