-
Sub-task
-
Resolution: Unresolved
-
P4
-
24
-
aarch64
-
generic
While looking into JDK-8342736 we found there are around ~400 ldr calls and ~180 str calls that would need to be manually updated to add an offset check.
Offsets in C++ classes are used a lot:
$ cpu/aarch64 % grep _offset\(\) *.* | grep ldr | wc -l
250
$ cpu/aarch64 % grep _offset\(\) *.* | grep str | wc -l
94
$ cpu/aarch64 % grep _offset\(\) *.* | grep lea | wc -l
27
IMO, we can use static_assert for them so the offset is checked at compile time. The problem is that the macro offset_of is not constexpr. Making it constexpr is not simple. The standard macro offsetof requires classes to have the standard layout. Most of our classes don't have the standard layout. We'll get warnings about this. One option is to disable the warning https://github.com/openjdk/jdk/blob/51763b67004a8b37d9bf4b8efef8aa1fa7bc9f4a/src/hotspot/share/utilities/globalDefinitions_gcc.hpp#L127-L129
Another option which doesn't require updating offsetof is to check the class size. Something like this:
ldr(dst, Address(rmethod, create_mem_op_imm(Method, const_offset)));
#define create_mem_op_imm(klass, field_offset_func) \
([]() { \
constexpr max_possible_offset = sizeof(klass);
static_assert(Address::offset_ok_for_immed(max_possible_offset, 0)); \
return klass::field_offset_func(); \
}())
If the size of a class fits into a memory instructions then any offset in it will fit. Class sizes greater than 32760 look insane to me.
Offsets in C++ classes are used a lot:
$ cpu/aarch64 % grep _offset\(\) *.* | grep ldr | wc -l
250
$ cpu/aarch64 % grep _offset\(\) *.* | grep str | wc -l
94
$ cpu/aarch64 % grep _offset\(\) *.* | grep lea | wc -l
27
IMO, we can use static_assert for them so the offset is checked at compile time. The problem is that the macro offset_of is not constexpr. Making it constexpr is not simple. The standard macro offsetof requires classes to have the standard layout. Most of our classes don't have the standard layout. We'll get warnings about this. One option is to disable the warning https://github.com/openjdk/jdk/blob/51763b67004a8b37d9bf4b8efef8aa1fa7bc9f4a/src/hotspot/share/utilities/globalDefinitions_gcc.hpp#L127-L129
Another option which doesn't require updating offsetof is to check the class size. Something like this:
ldr(dst, Address(rmethod, create_mem_op_imm(Method, const_offset)));
#define create_mem_op_imm(klass, field_offset_func) \
([]() { \
constexpr max_possible_offset = sizeof(klass);
static_assert(Address::offset_ok_for_immed(max_possible_offset, 0)); \
return klass::field_offset_func(); \
}())
If the size of a class fits into a memory instructions then any offset in it will fit. Class sizes greater than 32760 look insane to me.
- links to
-
Review(master) openjdk/jdk/22623