RFR (L, tedious) 8160399: is_oop_or_null involves undefined behavior

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Wed Aug 23 11:42:34 UTC 2017



On 8/17/17 9:31 PM, David Holmes wrote:
> Hi Coleen,
>
> Not that you need another Review but this looks good to me :)
>
> One comment below - feel free to ignore.
>
> On 18/08/2017 3:32 AM, coleen.phillimore at oracle.com wrote:
>>
>> Maybe the links would help:
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8160399.01/webrev
>
> src/share/vm/code/dependencies.cpp
>
> -  assert(result == NULL || result->is_oop(), "must be");
> +  assert(result == NULL || oopDesc::is_oop(result), "must be");
>
> That could just use oopDesc::is_oop_or_null(result). There are a 
> number of places that could do this. No big deal though.

Hi David, I did see these too.  There were only about 8 obvious cases of 
these, so I changed them also.   2 were in comments.

Thanks,
Coleen

>
> Thanks,
> David
> -----
>
>> bug link https://bugs.openjdk.java.net/browse/JDK-8160399
>> bug link https://bugs.openjdk.java.net/browse/JDK-8164984
>>
>>
>> On 8/17/17 1:21 PM, coleen.phillimore at oracle.com wrote:
>>> Summary: replace oop->is_oop*() with oopDesc::is_oop*(oop) so this 
>>> pointer can be verified
>>>
>>> Also included is:
>>> 8164984: Improper use of is_oop in production code
>>>
>>> http://cr.openjdk.java.net/~coleenp/8160399.01/webrev/src/share/vm/classfile/javaClasses.cpp.udiff.html 
>>>
>>>
>>> I also moved is_oop() to the .cpp file, which reduces the size of 
>>> fastdebug libjvm.so a little, since is_oop is only used for assert 
>>> and verification.
>>>
>>> before: -rw-r--r-- 1 cphillim   35086408 Aug 17 12:08 libjvm.so
>>> after: -rw-r--r-- 1 cphillim      35073384 Aug 17 12:12 libjvm.so
>>>
>>> Ran all platforms nightly tests (still in progress but no new 
>>> failures).
>>>
>>> Sorry for the boring code review request.
>>>
>>> Thanks,
>>> Coleen
>>



More information about the hotspot-dev mailing list