this-pointer NULL-checks in hotspot codebase [-Wtautological-undefined-compare]

[Erik Österlund]

Hi Harold,

It's worse than that though, unfortunately. You are not allowed to
have "this" equal to NULL, whether you perform such explicit NULL
comparisons or not.

The implication is that as long as "inflating" is NULL, we kind of
can't use any of the functions on markOop and hence  mustrewrite
pretty much all uses of markOop to do something else.
The same goes for things like Register, where rax == NULL. To be
compliant, we would similarly have to rewrite all uses of Register.

In other words, if we are to really hunt down uses of this == NULL
and remove them, we will find ourselves with a mountain of work.

Again, just gonna drop that here and run.

/Erik

On 2019-07-12 14:14, Harold Seigel wrote:
> The functions that compare 'this' to NULL could be changed from 
> instance to static functions where 'this' is explicitly passed as a 
> parameter.  Then you could keep the equivalent NULL checks.
>
> Harold
>
> On 7/12/2019 4:22 AM, Erik Österlund wrote:
>> 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
>>