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

[Knoll Lars]

On 04/03/16 08:59, "Development on behalf of Thiago Macieira" <development-bounces+lars.knoll=theqtcompany.com at qt-project.org on behalf of thiago.macieira at intel.com> wrote:



>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.

Qt QML should be ok. I went through those issues and fixed the at some point.

Cheers,
Lars

>
>> /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
>
>_______________________________________________
>Development mailing list
>Development at qt-project.org
>http://lists.qt-project.org/mailman/listinfo/development