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

David Holmes david.holmes at oracle.com
Fri Aug 18 01:31:37 UTC 2017


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.

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