public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [C++ PATCH] Fix -fsanitize=return on small functions (PR c++/77722)
@ 2016-09-27  8:01 Jakub Jelinek
  2016-09-27 15:01 ` Jason Merrill
  0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2016-09-27  8:01 UTC (permalink / raw)
  To: Jason Merrill, Marek Polacek; +Cc: gcc-patches

Hi!

As the testcases show, we actually instrument -fsanitize=return only if
the DECL_SAVED_TREE (fndecl) is a BIND_EXPR with STATEMENT_LIST as its
BIND_EXPR_BODY (plus tests that it actually needs such an instrumentation).
The return-{4,5}.C tests show that it isn't always the case, sometimes we
have just a STATEMENT_LIST without BIND_EXPR around it (for empty
functions), sometimes BIND_EXPR_BODY is a single statement, not a
STATEMENT_LIST.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2016-09-27  Jakub Jelinek  <jakub@redhat.com>

	PR c++/77722
	* cp-gimplify.c (cp_ubsan_maybe_instrument_return): Instrument also
	functions that have just a STATEMENT_LIST instead of BIND_EXPR, or
	BIND_EXPR with some statement rather than STATEMENT_LIST as body.

	* g++.dg/ubsan/return-4.C: New test.
	* g++.dg/ubsan/return-5.C: New test.
	* g++.dg/ubsan/return-6.C: New test.

--- gcc/cp/cp-gimplify.c.jj	2016-08-12 17:33:42.000000000 +0200
+++ gcc/cp/cp-gimplify.c	2016-09-26 09:37:30.679543731 +0200
@@ -1570,14 +1570,22 @@ cp_ubsan_maybe_instrument_return (tree f
     }
   if (t == NULL_TREE)
     return;
-  t = DECL_SAVED_TREE (fndecl);
-  if (TREE_CODE (t) == BIND_EXPR
-      && TREE_CODE (BIND_EXPR_BODY (t)) == STATEMENT_LIST)
+  tree *p = &DECL_SAVED_TREE (fndecl);
+  if (TREE_CODE (*p) == BIND_EXPR)
+    p = &BIND_EXPR_BODY (*p);
+  t = ubsan_instrument_return (DECL_SOURCE_LOCATION (fndecl));
+  if (TREE_CODE (*p) == STATEMENT_LIST)
     {
-      tree_stmt_iterator i = tsi_last (BIND_EXPR_BODY (t));
-      t = ubsan_instrument_return (DECL_SOURCE_LOCATION (fndecl));
+      tree_stmt_iterator i = tsi_last (*p);
       tsi_link_after (&i, t, TSI_NEW_STMT);
     }
+  else
+    {
+      tree list = NULL_TREE;
+      append_to_statement_list_force (*p, &list);
+      append_to_statement_list (t, &list);
+      *p = list;
+    }
 }
 
 void
--- gcc/testsuite/g++.dg/ubsan/return-4.C.jj	2016-09-26 09:42:45.306503657 +0200
+++ gcc/testsuite/g++.dg/ubsan/return-4.C	2016-09-26 09:54:08.778728718 +0200
@@ -0,0 +1,18 @@
+// PR c++/77722
+// { dg-do run }
+// { dg-options "-fsanitize=return -w" }
+// { dg-shouldfail "ubsan" }
+
+int
+foo ()
+{
+}
+
+int
+main ()
+{
+  foo ();
+  return 0;
+}
+
+// { dg-output "execution reached the end of a value-returning function without returning a value" }
--- gcc/testsuite/g++.dg/ubsan/return-5.C.jj	2016-09-26 09:53:37.135134983 +0200
+++ gcc/testsuite/g++.dg/ubsan/return-5.C	2016-09-26 09:54:03.273799395 +0200
@@ -0,0 +1,19 @@
+// PR c++/77722
+// { dg-do run }
+// { dg-options "-fsanitize=return -w" }
+// { dg-shouldfail "ubsan" }
+
+int
+foo ()
+{
+  int a = 5;
+}
+
+int
+main ()
+{
+  foo ();
+  return 0;
+}
+
+// { dg-output "execution reached the end of a value-returning function without returning a value" }
--- gcc/testsuite/g++.dg/ubsan/return-6.C.jj	2016-09-26 09:53:40.024097892 +0200
+++ gcc/testsuite/g++.dg/ubsan/return-6.C	2016-09-26 09:54:14.327657477 +0200
@@ -0,0 +1,20 @@
+// PR c++/77722
+// { dg-do run }
+// { dg-options "-fsanitize=return -w" }
+// { dg-shouldfail "ubsan" }
+
+int
+foo ()
+{
+  int a = 5;
+  int b = 5;
+}
+
+int
+main ()
+{
+  foo ();
+  return 0;
+}
+
+// { dg-output "execution reached the end of a value-returning function without returning a value" }

	Jakub

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [C++ PATCH] Fix -fsanitize=return on small functions (PR c++/77722)
  2016-09-27  8:01 [C++ PATCH] Fix -fsanitize=return on small functions (PR c++/77722) Jakub Jelinek
@ 2016-09-27 15:01 ` Jason Merrill
  2016-09-27 18:27   ` Jakub Jelinek
  0 siblings, 1 reply; 3+ messages in thread
From: Jason Merrill @ 2016-09-27 15:01 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Marek Polacek, gcc-patches List

On Tue, Sep 27, 2016 at 3:57 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> @@ -1570,14 +1570,22 @@ cp_ubsan_maybe_instrument_return (tree f
> +  if (TREE_CODE (*p) == STATEMENT_LIST)
>      {
> +      tree_stmt_iterator i = tsi_last (*p);
>        tsi_link_after (&i, t, TSI_NEW_STMT);
>      }
> +  else
> +    {
> +      tree list = NULL_TREE;
> +      append_to_statement_list_force (*p, &list);
> +      append_to_statement_list (t, &list);
> +      *p = list;
> +    }

Can't you replace all of this with

append_to_statement_list (t, p);

?

OK either way.

Jason

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [C++ PATCH] Fix -fsanitize=return on small functions (PR c++/77722)
  2016-09-27 15:01 ` Jason Merrill
@ 2016-09-27 18:27   ` Jakub Jelinek
  0 siblings, 0 replies; 3+ messages in thread
From: Jakub Jelinek @ 2016-09-27 18:27 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Marek Polacek, gcc-patches List

On Tue, Sep 27, 2016 at 10:56:51AM -0400, Jason Merrill wrote:
> On Tue, Sep 27, 2016 at 3:57 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> > @@ -1570,14 +1570,22 @@ cp_ubsan_maybe_instrument_return (tree f
> > +  if (TREE_CODE (*p) == STATEMENT_LIST)
> >      {
> > +      tree_stmt_iterator i = tsi_last (*p);
> >        tsi_link_after (&i, t, TSI_NEW_STMT);
> >      }
> > +  else
> > +    {
> > +      tree list = NULL_TREE;
> > +      append_to_statement_list_force (*p, &list);
> > +      append_to_statement_list (t, &list);
> > +      *p = list;
> > +    }
> 
> Can't you replace all of this with
> 
> append_to_statement_list (t, p);
> 
> ?
> 
> OK either way.

That works too and is cleaner, thanks.  Bootstrapped/regtested on
x86_64-linux and i686-linux, committed to trunk.

2016-09-27  Jakub Jelinek  <jakub@redhat.com>

	PR c++/77722
	* cp-gimplify.c (cp_ubsan_maybe_instrument_return): Instrument also
	functions that have just a STATEMENT_LIST instead of BIND_EXPR, or
	BIND_EXPR with some statement rather than STATEMENT_LIST as body.

	* g++.dg/ubsan/return-4.C: New test.
	* g++.dg/ubsan/return-5.C: New test.
	* g++.dg/ubsan/return-6.C: New test.

--- gcc/cp/cp-gimplify.c.jj	2016-09-26 12:06:36.060910355 +0200
+++ gcc/cp/cp-gimplify.c	2016-09-27 18:06:50.780686666 +0200
@@ -1570,14 +1570,11 @@ cp_ubsan_maybe_instrument_return (tree f
     }
   if (t == NULL_TREE)
     return;
-  t = DECL_SAVED_TREE (fndecl);
-  if (TREE_CODE (t) == BIND_EXPR
-      && TREE_CODE (BIND_EXPR_BODY (t)) == STATEMENT_LIST)
-    {
-      tree_stmt_iterator i = tsi_last (BIND_EXPR_BODY (t));
-      t = ubsan_instrument_return (DECL_SOURCE_LOCATION (fndecl));
-      tsi_link_after (&i, t, TSI_NEW_STMT);
-    }
+  tree *p = &DECL_SAVED_TREE (fndecl);
+  if (TREE_CODE (*p) == BIND_EXPR)
+    p = &BIND_EXPR_BODY (*p);
+  t = ubsan_instrument_return (DECL_SOURCE_LOCATION (fndecl));
+  append_to_statement_list (t, p);
 }
 
 void
--- gcc/testsuite/g++.dg/ubsan/return-5.C.jj	2016-09-27 18:05:37.138615332 +0200
+++ gcc/testsuite/g++.dg/ubsan/return-5.C	2016-09-27 18:05:37.138615332 +0200
@@ -0,0 +1,19 @@
+// PR c++/77722
+// { dg-do run }
+// { dg-options "-fsanitize=return -w" }
+// { dg-shouldfail "ubsan" }
+
+int
+foo ()
+{
+  int a = 5;
+}
+
+int
+main ()
+{
+  foo ();
+  return 0;
+}
+
+// { dg-output "execution reached the end of a value-returning function without returning a value" }
--- gcc/testsuite/g++.dg/ubsan/return-4.C.jj	2016-09-27 18:05:37.137615345 +0200
+++ gcc/testsuite/g++.dg/ubsan/return-4.C	2016-09-27 18:05:37.137615345 +0200
@@ -0,0 +1,18 @@
+// PR c++/77722
+// { dg-do run }
+// { dg-options "-fsanitize=return -w" }
+// { dg-shouldfail "ubsan" }
+
+int
+foo ()
+{
+}
+
+int
+main ()
+{
+  foo ();
+  return 0;
+}
+
+// { dg-output "execution reached the end of a value-returning function without returning a value" }
--- gcc/testsuite/g++.dg/ubsan/return-6.C.jj	2016-09-27 18:05:37.138615332 +0200
+++ gcc/testsuite/g++.dg/ubsan/return-6.C	2016-09-27 18:05:37.138615332 +0200
@@ -0,0 +1,20 @@
+// PR c++/77722
+// { dg-do run }
+// { dg-options "-fsanitize=return -w" }
+// { dg-shouldfail "ubsan" }
+
+int
+foo ()
+{
+  int a = 5;
+  int b = 5;
+}
+
+int
+main ()
+{
+  foo ();
+  return 0;
+}
+
+// { dg-output "execution reached the end of a value-returning function without returning a value" }

	Jakub

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-09-27 18:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-27  8:01 [C++ PATCH] Fix -fsanitize=return on small functions (PR c++/77722) Jakub Jelinek
2016-09-27 15:01 ` Jason Merrill
2016-09-27 18:27   ` Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).