RFR: 8341178: TypeRawPtr::add_offset may be "miscompiled" due to UB [v2]

Dean Long dlong at openjdk.org
Mon Oct 7 21:30:38 UTC 2024


On Fri, 4 Oct 2024 09:27:52 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Please review this change to TypeRawPtr::add_offset to prevent a compiler from
>> inferring things based on prior pointer arithmetic not invoking UB.  As noted in
>> the bug report, clang is actually doing this.
>> 
>> To accomplish this, changed to integral arithmetic.  Also added over/underflow
>> checks.
>> 
>> Also made a couple of minor touchups.  Replaced an implicit conversion to bool
>> with an explicit compare to nullptr (per style guide).  Removed a no longer
>> needed dummy return after a (now) noreturn function.
>> 
>> Testing: mach5 tier1-7
>> That testing was with calls to "fatal" for the over/underflow cases and the
>> sum==0 case.  There were no hits.  I'm not sure how to construct a test that
>> would hit those.
>
> Kim Barrett has updated the pull request incrementally with one additional commit since the last revision:
> 
>   remove surrounding whitespace

src/hotspot/share/opto/type.cpp line 3226:

> 3224:     return this;
> 3225:   case TypePtr::Null:
> 3226:     return make( (address)offset );

Shouldn't this assert that _bits == 0?  Looking at the code, however, I can't find anywhere that we actually create a TypeRawPtr with TypePtr::Null.  We could probably remove this case and let it fall through to the default ShouldNotReachHere().

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21324#discussion_r1790898473


More information about the hotspot-compiler-dev mailing list