This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On 2016.01.19 at 13:11 +0100, Jan Hubicka wrote:
> according to Trevor, the assumption about THIS pointer being non-NULL breaks
> 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
> 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).
> 
> 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

This should be:
+      && !flag_null_this_pointer

Honza, can you please repost the patch. Richard said on IRC that he may
reconsider his rejection after all.

I've tested the patch on my Gentoo test machine and it fixes segfaults
in LLVM, QT, Chromium, Kdevelop.

If the patch gets accepted, changes.html and porting_to.html need to be
updated to reflect the new flag.

-- 
Markus


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]