RFR: 8227633: avoid comparing this pointers to NULL - was : RE: this-pointer NULL-checks in hotspot codebase [-Wtautological-undefined-compare]

[coleen.phillimore at oracle.com]

http://cr.openjdk.java.net/~mbaesken/webrevs/8227633.0/src/hotspot/share/adlc/formssel.cpp.udiff.html

+ if (mnode) mnode->count_instr_names(names);


We also try to avoid implicit checks against null for pointers so change 
this to:

+ if (mnode != NULL) mnode->count_instr_names(names);

I didn't see that you added a check for NULL in the callers of 
print_opcodes or setstr.  Can those callers never pass NULL?

We've done a few passes to clean up these this == NULL checks. Thank you 
for doing this!

Coleen


On 7/12/19 8:30 AM, Baesken, Matthias wrote:
> Hello Erik,   thanks for the input .
>
> We still have a few places in the HS codebase where "this" is compared to NULL.
> When compiling with xlc16 / xlclang we get these warnings :
>
>   warning: 'this' pointer cannot be null in well-defined C++ code; comparison may be assumed to always evaluate to false [-Wtautological-undefined-compare]
>
> so those places should be removed where possible.
>
>
> I adjusted 3  checks , please review !
>
>
>
> Bug/webrev :
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8227633.0/
>
> https://bugs.openjdk.java.net/browse/JDK-8227633
>
> Thanks , Matthias
>
>
>> -----Original Message-----
>> From: Erik Österlund <erik.osterlund at oracle.com>
>> Sent: Freitag, 12. Juli 2019 10:22
>> To: Baesken, Matthias <matthias.baesken at sap.com>; 'hotspot-
>> dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>
>> Subject: Re: this-pointer NULL-checks in hotspot codebase [-Wtautological-
>> undefined-compare]
>>
>> Hi Matthias,
>>
>> Removing such NULL checks seems like a good idea in general due to the
>> undefined behaviour.
>> Worth mentioning though that there are some tricky ones, like in
>> markOopDesc* where this == NULL
>> means that the mark word has the "inflating" value. So we explicitly
>> check if this == NULL and
>> hope the compiler will not elide the check. Just gonna drop that one
>> here and run for it.
>>
>> Thanks,
>> /Erik
>>
>> On 2019-07-12 09:48, Baesken, Matthias wrote:
>>> Hello , when looking  into  the  recent  xlc16 / xlclang   warnings I came
>> across  those  3 :
>>> /nightly/jdk/src/hotspot/share/adlc/formssel.cpp:1729:7: warning: 'this'
>> pointer cannot be null in well-defined C++ code;
>>> comparison may be assumed to always evaluate to true [-Wtautological-
>> undefined-compare]
>>>     if( this != NULL ) {
>>>         ^~~~    ~~~~
>>>
>>> /nightly/jdk/src/hotspot/share/adlc/formssel.cpp:3416:7: warning: 'this'
>> pointer cannot be null in well-defined C++ code;
>>> comparison may be assumed to always evaluate to false [-Wtautological-
>> undefined-compare]
>>>     if( this == NULL ) return;
>>>
>>> /nightly/jdk/src/hotspot/share/libadt/set.cpp:46:7: warning: 'this' pointer
>> cannot be null in well-defined C++ code;
>>> comparison may be assumed to always evaluate to false [-Wtautological-
>> undefined-compare]
>>>     if( this == NULL ) return os::strdup("{no set}");
>>>
>>>
>>> Do you think the  NULL-checks can be removed or is there still some value
>> in doing them ?
>>> Best regards, Matthias