Re: [C++] Add -fnull-this-pointer

[Richard Biener <richard dot guenther at gmail dot com>]

On Tue, Jan 19, 2016 at 2:18 PM, Trevor Saunders <tbsaunde@tbsaunde.org> wrote:
> On Tue, Jan 19, 2016 at 01:11:44PM +0100, Jan Hubicka wrote:
>> Hi,
>> according to Trevor, the assumption about THIS pointer being non-NULL breaks
>
> That was Markus, not me.
>
>> several bigger C++ packages (definitly including Firefox, but I believe
>> kdevelop was mentioned, too).  This patch makes the feature to be controlable
>> by a dedicated flag.  I am not sure about the default. We now have ubsan check
>> for the bug so I would hope the codebases to be updated soon, but it did not
>> happen for Firefox for quite a while despite the fact that Martin Liska reported
>> it.
>>
>> This patch defaults to -fno-null-this-pointer, but I would be OK with changing
>
> fwiw I find the naming a bit confusing maybe I'm just tired but it takes
> some puzlling for me to know which way is being strict and which way is
> allowing this.
>
>> the default and setting it on only in GCC 6. Main point of the patch is to
>> avoid need of those packages to be built with -fno-delete-null-pointer-checks
>> (which still subsumes the flag).
>
> Personally I'd rather try and be strict.  I suspect it often will be
> easy to find and fix the bugs when the optimization is enabled.  Of
> course if some projects don't care they can pass flags themselves.

Agreed.  As we already have a flag that can be used as a workaround I don't see
a reason to add another more specific one.  That just makes it a
lesser incentive
for people to fix their code.

Richard.

> Trev
>
>>
>> The patch is bit inconsistent, becuase C++ FE wil still assume that this pointer
>> is non-NULL when expanding multiple inheritance accesses.  We did this from
>> very beginning. I do not know FE enough to see if it is easy to change the
>> behaviour here or if it is desired.
>>
>> Bootstrapped/regtsted x86_64-linux.
>>
>> Honza
>>
>>       * c-family/c.opt (fnull-this-pointer): New flag.
>>       (nonnull_arg_p): Honnor flag_null_this_pointer.
>> Index: tree.c
>> ===================================================================
>> --- tree.c    (revision 232553)
>> +++ tree.c    (working copy)
>> @@ -14016,6 +14022,7 @@ nonnull_arg_p (const_tree arg)
>>    /* THIS argument of method is always non-NULL.  */
>>    if (TREE_CODE (TREE_TYPE (cfun->decl)) == METHOD_TYPE
>>        && arg == DECL_ARGUMENTS (cfun->decl)
>> +      && flag_null_this_pointer
>>        && flag_delete_null_pointer_checks)
>>      return true;
>>
>> Index: c-family/c.opt
>> ===================================================================
>> --- c-family/c.opt    (revision 232553)
>> +++ c-family/c.opt    (working copy)
>> @@ -1321,6 +1321,10 @@ Enum(ivar_visibility) String(public) Val
>>  EnumValue
>>  Enum(ivar_visibility) String(package) Value(IVAR_VISIBILITY_PACKAGE)
>>
>> +fnull-this-pointer
>> +C++ ObjC++ Optimization Report Var(flag_null_this_pointer)
>> +Allow calling methods of NULL pointer
>> +
>>  fnonansi-builtins
>>  C++ ObjC++ Var(flag_no_nonansi_builtin, 0)
>>
>> Index: doc/invoke.texi
>> ===================================================================
>> --- doc/invoke.texi   (revision 232553)
>> +++ doc/invoke.texi   (working copy)
>> @@ -232,6 +232,7 @@ Objective-C and Objective-C++ Dialects}.
>>  -fobjc-std=objc1 @gol
>>  -fno-local-ivars @gol
>>  -fivar-visibility=@r{[}public@r{|}protected@r{|}private@r{|}package@r{]} @gol
>> +-fnull-this-pointer @gol
>>  -freplace-objc-classes @gol
>>  -fzero-link @gol
>>  -gen-decls @gol
>> @@ -2361,6 +2362,11 @@ errors if these functions are not inline
>>  Disable Wpedantic warnings about constructs used in MFC, such as implicit
>>  int and getting a pointer to member function via non-standard syntax.
>>
>> +@item -fnull-this-pointer
>> +@opindex fnull-this-pointer
>> +Disable optimization which take advantage of the fact that calling method
>> +of @code{NULL} pointer is undefined.
>> +
>>  @item -fno-nonansi-builtins
>>  @opindex fno-nonansi-builtins
>>  Disable built-in declarations of functions that are not mandated by