RFR: 8313882: Fix -Wconversion warnings in runtime code [v2]

Dean Long dlong at openjdk.org
Wed Aug 9 01:44:35 UTC 2023


On Wed, 9 Aug 2023 00:12:13 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> src/hotspot/share/runtime/objectMonitor.hpp line 238:
>> 
>>> 236:   //
>>> 237:   #define OM_OFFSET_NO_MONITOR_VALUE_TAG(f) \
>>> 238:     ((in_bytes(ObjectMonitor::f ## _offset())) - (unsigned)markWord::monitor_value)
>> 
>> This makes it int vs uint (-Wsign-conversion warning), and assumes the range of monitor_value.
>> One option is to narrow the type of monitor_value.  Option 2, cast the lhs to uintptr_t.  Option 3, use offset_of instead of signed ByteSize.
>
> I looked at option 1, but monitor_value is used for byte operations, and it seemed unsafe to make it smaller than uintptr_t, eg:
> 
> oops/markWord.hpp:    return markWord(tmp | monitor_value);
> 
> The -Wconversion warning is complaining about OM_OFFSET_NO_MONITOR_VALUE_TAG is an long unsigned int because the expression is promoted to the size of markWord::monitor_value without the cast.
> 
> Option 3 turns the address into a 64 bit int, but the Address constructor expects a 32 bit displacement.
> 
>    88 |   __ movptr(Address(mon, OM_OFFSET_NO_MONITOR_VALUE_TAG(owner)), r15_thread);
> 
> Should this convert to int instead of unsigned instead?

Yes int would be more correct.  The value could be negative if the order of the fields in ObjectMonitor (or the value of monitor_value) were changed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15177#discussion_r1287840806


More information about the graal-dev mailing list