Do not optimize some polymorphic calls with -fsanitize=undefined

Do not optimize some polymorphic calls with -fsanitize=undefined

[Jan Hubicka]

Hi,
as requested by Jakub, this patch makes devirtualization code to turn off
transformations based on assumption that cxa_pure_virtual will never be called
by a virtual call when -fsanitize=undefined is used.

Bootstrapped/regtested x86_64-linux, will commit it shortly.

	PR ipa/66223
	* ipa-devirt.c (maybe_record_node): Do not optimize cxa_pure_virtual
	calls when sanitizing.
	(possible_polymorphic_call_target_p)" FIx formating.

	* g++.dg/ipa/devirt-51.C: New testcase.
Index: ipa-devirt.c
===================================================================
--- ipa-devirt.c	(revision 234715)
+++ ipa-devirt.c	(working copy)
@@ -2438,10 +2438,14 @@ maybe_record_node (vec <cgraph_node *> &
     {
       gcc_assert (!target_node->global.inlined_to);
       gcc_assert (target_node->real_symbol_p ());
+      /* When sanitizing, do not asume that cxa_pure_virutal is not called
+	 by valid program.  */
+      if (flag_sanitize & SANITIZE_UNDEFINED)
+	;
       /* Only add pure virtual if it is the only possible target.  This way
 	 we will preserve the diagnostics about pure virtual called in many
 	 cases without disabling optimization in other.  */
-      if (pure_virtual)
+      else if (pure_virtual)
 	{
 	  if (nodes.length ())
 	    return;
@@ -3374,8 +3378,7 @@ possible_polymorphic_call_target_p (tree
   bool final;
 
   if (TREE_CODE (TREE_TYPE (n->decl)) == FUNCTION_TYPE
-      && ((fcode = DECL_FUNCTION_CODE (n->decl))
-	  == BUILT_IN_UNREACHABLE
+      && ((fcode = DECL_FUNCTION_CODE (n->decl)) == BUILT_IN_UNREACHABLE
           || fcode == BUILT_IN_TRAP))
     return true;
 
Index: testsuite/g++.dg/ipa/devirt-51.C
===================================================================
--- testsuite/g++.dg/ipa/devirt-51.C	(revision 0)
+++ testsuite/g++.dg/ipa/devirt-51.C	(working copy)
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fsanitize=undefined -fdump-tree-optimized"  } */
+namespace {
+  struct B {
+        B* self;
+        B() : self( this ) { self->f(); }
+	void E(void);
+        virtual void f() = 0;
+    };
+
+    struct D : B
+    {
+        void f() {}
+    };
+}
+
+struct D e;
+
+__attribute__ ((used))
+void B::E(void)
+  {
+    this->f();
+}
+
+    int main()
+    {
+        D d;
+    }
+/* { dg-final { scan-tree-dump "cxa_pure_virtual" "optimized"  } } */

Re: Do not optimize some polymorphic calls with -fsanitize=undefined

[Jakub Jelinek]

On Mon, Apr 04, 2016 at 05:50:53PM +0200, Jan Hubicka wrote:
> Hi,
> as requested by Jakub, this patch makes devirtualization code to turn off
> transformations based on assumption that cxa_pure_virtual will never be called
> by a virtual call when -fsanitize=undefined is used.
> 
> Bootstrapped/regtested x86_64-linux, will commit it shortly.
> 
> 	PR ipa/66223
> 	* ipa-devirt.c (maybe_record_node): Do not optimize cxa_pure_virtual
> 	calls when sanitizing.
> 	(possible_polymorphic_call_target_p)" FIx formating.
> 
> 	* g++.dg/ipa/devirt-51.C: New testcase.
> Index: ipa-devirt.c
> ===================================================================
> --- ipa-devirt.c	(revision 234715)
> +++ ipa-devirt.c	(working copy)
> @@ -2438,10 +2438,14 @@ maybe_record_node (vec <cgraph_node *> &
>      {
>        gcc_assert (!target_node->global.inlined_to);
>        gcc_assert (target_node->real_symbol_p ());
> +      /* When sanitizing, do not asume that cxa_pure_virutal is not called

s/asume/assume/
s/cxa/__cxa/
s/virutal/virtual/

> +	 by valid program.  */
> +      if (flag_sanitize & SANITIZE_UNDEFINED)
> +	;

I'd use SANITIZE_UNREACHABLE instead, that is the sanitizer for
__builtin_unreachable ().  Unless we want to split that into
-fsanitize=unreachable
-fsanitize=pure-virtual

	Jakub

Re: Do not optimize some polymorphic calls with -fsanitize=undefined

[Jan Hubicka]

> On Mon, Apr 04, 2016 at 05:50:53PM +0200, Jan Hubicka wrote:
> > Hi,
> > as requested by Jakub, this patch makes devirtualization code to turn off
> > transformations based on assumption that cxa_pure_virtual will never be called
> > by a virtual call when -fsanitize=undefined is used.
> > 
> > Bootstrapped/regtested x86_64-linux, will commit it shortly.
> > 
> > 	PR ipa/66223
> > 	* ipa-devirt.c (maybe_record_node): Do not optimize cxa_pure_virtual
> > 	calls when sanitizing.
> > 	(possible_polymorphic_call_target_p)" FIx formating.
> > 
> > 	* g++.dg/ipa/devirt-51.C: New testcase.
> > Index: ipa-devirt.c
> > ===================================================================
> > --- ipa-devirt.c	(revision 234715)
> > +++ ipa-devirt.c	(working copy)
> > @@ -2438,10 +2438,14 @@ maybe_record_node (vec <cgraph_node *> &
> >      {
> >        gcc_assert (!target_node->global.inlined_to);
> >        gcc_assert (target_node->real_symbol_p ());
> > +      /* When sanitizing, do not asume that cxa_pure_virutal is not called
> 
> s/asume/assume/
> s/cxa/__cxa/
> s/virutal/virtual/
> 
> > +	 by valid program.  */
> > +      if (flag_sanitize & SANITIZE_UNDEFINED)
> > +	;
> 
> I'd use SANITIZE_UNREACHABLE instead, that is the sanitizer for
> __builtin_unreachable ().  Unless we want to split that into
> -fsanitize=unreachable
> -fsanitize=pure-virtual

Thanks. This is about case where we optimize undefined call (which would
otherwise land in cxa_pure_virtual) into some other virtual method (that
is the only resonable choice).  I suppose UNREACHABLE makes sense here.
I think I already commited the patch but I will update this.

Honza

> 
> 	Jakub

Re: Do not optimize some polymorphic calls with -fsanitize=undefined

[Jan Hubicka]

Hi,
this is patch I commited to address the feedback.

Regtested on x86_64-linux

Honza

	* ipa-devirt.c (maybe_record_node): Fix comment; use
	SANITIZE_UNREACHABLE instead of SANITIZE_UNDEFINED.

	* g++.dg/ipa/devirt-51.C: Use -fsanitize=unreachable.
Index: ipa-devirt.c
===================================================================
--- ipa-devirt.c	(revision 234761)
+++ ipa-devirt.c	(working copy)
@@ -2438,9 +2438,9 @@ maybe_record_node (vec <cgraph_node *> &
     {
       gcc_assert (!target_node->global.inlined_to);
       gcc_assert (target_node->real_symbol_p ());
-      /* When sanitizing, do not asume that cxa_pure_virutal is not called
+      /* When sanitizing, do not assume that __cxa_pure_virtual is not called
 	 by valid program.  */
-      if (flag_sanitize & SANITIZE_UNDEFINED)
+      if (flag_sanitize & SANITIZE_UNREACHABLE)
 	;
       /* Only add pure virtual if it is the only possible target.  This way
 	 we will preserve the diagnostics about pure virtual called in many
Index: testsuite/g++.dg/ipa/devirt-51.C
===================================================================
--- testsuite/g++.dg/ipa/devirt-51.C	(revision 234761)
+++ testsuite/g++.dg/ipa/devirt-51.C	(working copy)
@@ -2,7 +2,7 @@
    variant.  Either keeping virtual call or optimizing to cxa_pure_virtual
    is fine.  */
 /* { dg-do compile } */
-/* { dg-options "-O2 -fsanitize=undefined -fdump-tree-optimized"  } */
+/* { dg-options "-O2 -fsanitize=unreachable -fdump-tree-optimized"  } */
 namespace {
   struct B {
         B* self;