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