[Development] Are we free of code that checks this isn't null?

[Thiago Macieira]

On sexta-feira, 4 de março de 2016 09:58:53 PST Marc Mutz wrote:
> On Friday 04 March 2016 07:52:15 Thiago Macieira wrote:
> > Found in GCC 6's changelog (http://gcc.gnu.org/gcc-6/changes.html):
> > > Value range propagation now assumes that the this pointer of C++ member
> > > functions is non-null. This eliminates common null pointer checks but
> > > also breaks some non-conforming code-bases (such as Qt-5, Chromium,
> > > KDevelop). As a temporary work-around -fno-delete-null-pointer-checks
> > > can be used. Wrong code can be identified by using -fsanitize=undefined.
> > 
> > Are we free of such mistakes? Or do we need to enable -fno-delete-null-
> > pointer-checks?
> 
> This comes to mind;
> 
>   QPointer<QObject> that = this;
>   // ...
>   if (!that) return;
> 
> Not sure it's affected, though.

Shouldn't be because that !that call isn't checking the this pointer, but 
instead is checking the strong refcount inside the QSharedPointer.

> Don't know off-handedly anything else that where this == nullptr would be
> valid. Only way to find out is to enable -fno-delete-null-pointer-checks
> except for -developer-build and trying to fix the ubsan issues.

It's not valid. The this pointer is never null, according to the standard.

And yet we had code like that in V4 (qtdeclarative). Clang started complaining 
about it and I tried to fix it by just removing the checks, but they were 
apparently in use. Since I don't remember seeing that warning anymore, it's 
likely to be fixed.

But I am asking to be sure.

> /me upgrades to GCC 6.

I might still have a patch or two that aren't merged that fix GCC 6 warnings. 
But when you compile with it, can you make sure it doesn't print any 
misleading-identation warnings? There were a couple of false positives that I 
reported and GCC fixed.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center