RFR: 8213411: JDK-8209189 incorrect for Big Endian (JVM crashes)

Vladimir Kozlov vladimir.kozlov at oracle.com
Wed Nov 7 16:43:29 UTC 2018


Looks good.

Thanks,
Vladimir

On 11/7/18 5:45 AM, Erik Österlund wrote:
> Hi Vladimir,
> 
> Thank you for the review. I made an abstraction for the bit fiddling as suggested to make it more readable.
> There is now an AllStatic IsUnloadingState class that has a mask and shift for each field, expresses getters and setters 
> using them, and a create function for setting some passed in fields into a new bit field.
> 
> Webrev:
> http://cr.openjdk.java.net/~eosterlund/8213411/webrev.01/
> 
> Thanks,
> /Erik
> 
> On 2018-11-06 17:21, Vladimir Kozlov wrote:
>> Hi Erik,
>>
>> It would be nice if you use enum for values, masks and shift (like for markOop). It will become more readable.
>>
>> Thanks,
>> Vladimir
>>
>> On 11/6/18 2:34 AM, Erik Österlund wrote:
>>> Hi,
>>>
>>> In my patch for 8209189 I assumed that the following struct containing a bit field using 3 bits, would be packed to 
>>> one byte:
>>>
>>> struct IsUnloadingStruct {
>>>    unsigned int _is_unloading:1;
>>>    unsigned int _unloading_cycle:2;
>>> };
>>>
>>> Turns out though that the size of this 3 bit bitfield is typically 4 bytes, causing trouble on big endian machines. 
>>> The compilers can be tamed to turn this to a byte (as expected) by using struct packing.
>>>
>>> So the two most obvious solutions are either
>>> 1) Make a struct packing macro, and trust the compilers won't mess up then.
>>> 2) Do manual bit fiddling because we do not trust compilers.
>>>
>>> Martin convinced me to go with #2.
>>>
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8213411
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~eosterlund/8213411/webrev.00/
>>>
>>> Running a bunch of tests while waiting for review.
>>>
>>> Thanks,
>>> /Erik
> 


More information about the hotspot-dev mailing list