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

[Doerr, Martin]

Hi Matthias,

looks good to me.

Please make sure that this change got built on all platforms we have.
The adlc is used during build so if it has passed on all platforms, it should be ok.

Best regards,
Martin


> -----Original Message-----
> From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net> On Behalf Of
> coleen.phillimore at oracle.com
> Sent: Freitag, 12. Juli 2019 14:49
> To: hotspot-dev at openjdk.java.net
> Subject: Re: RFR: 8227633: avoid comparing this pointers to NULL - was : RE:
> this-pointer NULL-checks in hotspot codebase [-Wtautological-undefined-
> compare]
> 
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8227633.0/src/hotspot/sha
> re/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