RFR: JDK-8294902: Undefined Behavior in C2 regalloc with null references [v6]

[Kim Barrett]

On Mon, 5 Dec 2022 11:30:17 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> How does this behave with respect to alignment?
>> The previous implementation used 16 which is at least aligned to `std::max_align_t` (for all hotspots build systems?) 
>> My understanding is that `char space[sizeof (klass)];` is 1-byte aligned, so `dummyObj` may be misaligned.
>
>> How does this behave with respect to alignment? The previous implementation used 16 which is at least aligned to `std::max_align_t` (for all hotspots build systems?) My understanding is that `char space[sizeof (klass)];` is 1-byte aligned, so `dummyObj` may be misaligned.
> 
> That's an interesting point. I don't propose to trawl the standard for exact wording, but as far as I recall it's _accesses_ to misaligned members that are problematic.

I had wondered the same thing, and had already spent some time trawling the
standard. I think the only way to get a misaligned pointer (without using
`#pragma pack`) involves casts. The relevant `reinterpret_cast` is defined as
being equivalent to `static_cast` to `void*` then `static_cast` to `T*` (C++14
5.2.10/7). There it says round trip cast is okay with same alignment
requirements, but is silent about when they are different. However,
`static_cast` from `void*` to `T*` is okay if the address satisfies the
alignment requirement for `T`, else unspecified (C++14 5.2.9/13).

So if the space buffer is misaligned for the klass type, the cast will have an
unspecified result. There may also be a question regarding whether
`&dummyObj->field` constitutes an access.  I recall a gcc bug with a long
discussion about whether `&x[n]` constituted an access (with gcc maintainers
saying yes), but I couldn't refind that bug today.

I think the safe thing to do is to align the space buffer.  The simplest way
to do that would be with `alignas`, but using that is pending approval:
https://git.openjdk.org/jdk/pull/11446. `ATTRIBUTE_ALIGNED` is the HotSpot
macro for that.

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

PR: https://git.openjdk.org/jdk/pull/10920