Fix PR83388

Message ID alpine.LSU.2.20.1712151105070.12252@zhemvz.fhfr.qr
State New
Headers show
Series
  • Fix PR83388
Related show

Commit Message

Richard Biener Dec. 15, 2017, 10:08 a.m.
The following fixes removal of sanitizer IFN calls during LTO streaming
in when not linking with -fsanitize= to not interfer with IPA reference
nodes that might be attached to those stmts.  The easiest idea I could
come up with that would also work with IPA passes refering to those
refs is to replace the IFN call with a NOP-like one.  The following
patch does this, the next DCE pass will then remove those stmts
(or at -O0 expand will expand them to nothing).  If we start to
use this replacement trick for calls with a LHS DCE would need to
be taught to replace the LHS with a default-def (or if aggregate
just ignore it).  OTOH such replacement would be fishy.

Bootstrap and regtest in progress.

Any comments?

Thanks,
Richard.

2017-12-15  Richard Biener  <rguenther@suse.de>

	PR lto/83388
	* internal-fn.def (IFN_NOP): Add.
	* internal-fn.c (expand_NOP): Do nothing.
	* lto-streamer-in.c (input_function): Instead of removing
	sanitizer calls replace them with IFN_NOP calls.

	* gcc.dg/lto/pr83388_0.c: New testcase.

Comments

Jakub Jelinek Dec. 15, 2017, 10:26 a.m. | #1
On Fri, Dec 15, 2017 at 11:08:43AM +0100, Richard Biener wrote:
> --- gcc/internal-fn.def	(revision 255678)

> +++ gcc/internal-fn.def	(working copy)

> @@ -254,6 +254,9 @@ DEF_INTERNAL_FN (LAUNDER, ECF_LEAF | ECF

>  /* Divmod function.  */

>  DEF_INTERNAL_FN (DIVMOD, ECF_CONST | ECF_LEAF, NULL)

>  

> +/* A NOP function with aribtrary arguments and return value.  */


arbitrary

> +static void

> +expand_NOP (internal_fn, gcall *call_stmt)

> +{

> +  /* Nothing.  But it shouldn't really prevail.  */


It could with -O0 (but who would use -flto -O0) or with -fno-tree-dce
-fno-tree-whateverelse.

LGTM otherwise.

	Jakub
Richard Biener Dec. 15, 2017, 10:54 a.m. | #2
On Fri, 15 Dec 2017, Jakub Jelinek wrote:

> On Fri, Dec 15, 2017 at 11:08:43AM +0100, Richard Biener wrote:

> > --- gcc/internal-fn.def	(revision 255678)

> > +++ gcc/internal-fn.def	(working copy)

> > @@ -254,6 +254,9 @@ DEF_INTERNAL_FN (LAUNDER, ECF_LEAF | ECF

> >  /* Divmod function.  */

> >  DEF_INTERNAL_FN (DIVMOD, ECF_CONST | ECF_LEAF, NULL)

> >  

> > +/* A NOP function with aribtrary arguments and return value.  */

> 

> arbitrary

> 

> > +static void

> > +expand_NOP (internal_fn, gcall *call_stmt)

> > +{

> > +  /* Nothing.  But it shouldn't really prevail.  */

> 

> It could with -O0 (but who would use -flto -O0) or with -fno-tree-dce

> -fno-tree-whateverelse.


Yeah, with the -O0 variant of the testcase it does prevail and then
we expand it to nothing.

Richard.

Patch

Index: gcc/internal-fn.def
===================================================================
--- gcc/internal-fn.def	(revision 255678)
+++ gcc/internal-fn.def	(working copy)
@@ -254,6 +254,9 @@  DEF_INTERNAL_FN (LAUNDER, ECF_LEAF | ECF
 /* Divmod function.  */
 DEF_INTERNAL_FN (DIVMOD, ECF_CONST | ECF_LEAF, NULL)
 
+/* A NOP function with aribtrary arguments and return value.  */
+DEF_INTERNAL_FN (NOP, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
+
 #undef DEF_INTERNAL_INT_FN
 #undef DEF_INTERNAL_FLT_FN
 #undef DEF_INTERNAL_FLT_FLOATN_FN
Index: gcc/internal-fn.c
===================================================================
--- gcc/internal-fn.c	(revision 255678)
+++ gcc/internal-fn.c	(working copy)
@@ -2722,6 +2722,14 @@  expand_DIVMOD (internal_fn, gcall *call_
 	       target, VOIDmode, EXPAND_NORMAL);
 }
 
+/* Expand a NOP.  */
+
+static void
+expand_NOP (internal_fn, gcall *call_stmt)
+{
+  /* Nothing.  But it shouldn't really prevail.  */
+}
+
 /* Expand a call to FN using the operands in STMT.  FN has a single
    output operand and NARGS input operands.  */
 
Index: gcc/lto-streamer-in.c
===================================================================
--- gcc/lto-streamer-in.c	(revision 255678)
+++ gcc/lto-streamer-in.c	(working copy)
@@ -1136,41 +1136,47 @@  input_function (tree fn_decl, struct dat
 	      if (is_gimple_call (stmt)
 		  && gimple_call_internal_p (stmt))
 		{
+		  bool replace = false;
 		  switch (gimple_call_internal_fn (stmt))
 		    {
 		    case IFN_UBSAN_NULL:
 		      if ((flag_sanitize
 			  & (SANITIZE_NULL | SANITIZE_ALIGNMENT)) == 0)
-			remove = true;
+			replace = true;
 		      break;
 		    case IFN_UBSAN_BOUNDS:
 		      if ((flag_sanitize & SANITIZE_BOUNDS) == 0)
-			remove = true;
+			replace = true;
 		      break;
 		    case IFN_UBSAN_VPTR:
 		      if ((flag_sanitize & SANITIZE_VPTR) == 0)
-			remove = true;
+			replace = true;
 		      break;
 		    case IFN_UBSAN_OBJECT_SIZE:
 		      if ((flag_sanitize & SANITIZE_OBJECT_SIZE) == 0)
-			remove = true;
+			replace = true;
 		      break;
 		    case IFN_UBSAN_PTR:
 		      if ((flag_sanitize & SANITIZE_POINTER_OVERFLOW) == 0)
-			remove = true;
+			replace = true;
 		      break;
 		    case IFN_ASAN_MARK:
 		      if ((flag_sanitize & SANITIZE_ADDRESS) == 0)
-			remove = true;
+			replace = true;
 		      break;
 		    case IFN_TSAN_FUNC_EXIT:
 		      if ((flag_sanitize & SANITIZE_THREAD) == 0)
-			remove = true;
+			replace = true;
 		      break;
 		    default:
 		      break;
 		    }
-		  gcc_assert (!remove || gimple_call_lhs (stmt) == NULL_TREE);
+		  if (replace)
+		    {
+		      gimple_call_set_internal_fn (as_a <gcall *> (stmt),
+						   IFN_NOP);
+		      update_stmt (stmt);
+		    }
 		}
 	    }
 	  if (remove)
Index: gcc/testsuite/gcc.dg/lto/pr83388_0.c
===================================================================
--- gcc/testsuite/gcc.dg/lto/pr83388_0.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/lto/pr83388_0.c	(working copy)
@@ -0,0 +1,18 @@ 
+/* { dg-lto-do link } */
+/* { dg-lto-options { { -O2 -flto -fsanitize=null } { -O0 -flto -fsanitize=null } } } */
+/* { dg-extra-ld-options { -fno-sanitize=null -r -nostdlib } } */
+
+enum { a } e(void);
+struct C {
+    int d;
+} c;
+long f;
+void g(long);
+static void i(_Bool h) {
+    struct C *a = ({ ({ &c; }); });
+    if (e()) {
+	int b = a->d;
+	g(f);
+    }
+}
+void j(void) { i(a); }