diff mbox

Implement -fsanitize=return (for C++ only)

Message ID 20131122092852.GU892@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Nov. 22, 2013, 9:28 a.m. UTC 
Hi!

Working virtually out of Pago Pago right now.

In C++, falling through the end of a function that returns a value
without returning a value is undefined behavior (unlike C?), so this patch
implements instrumentation of it.

Ok for trunk? 

2013-11-21  Jakub Jelinek  <jakub@redhat.com>

	* ubsan.c (ubsan_source_location): Don't crash on
	unknown locations.
	(ubsan_pass): Ignore clobber stmts.

	* sanitizer.def (BUILT_IN_UBSAN_HANDLE_MISSING_RETURN): New built-in.
	* opts.c (common_handle_option): Add -fsanitize=return.
	* flag-types.h (enum sanitize_code): Add SANITIZE_RETURN and
	or it into SANITIZE_UNDEFINED.
c-family/
	* c-ubsan.h (ubsan_instrument_return): New prototype.
	* c-ubsan.c (ubsan_instrument_return): New function.
cp/
	* cp-gimplify.c: Include target.h and c-family/c-ubsan.h.
	(cp_genericize): Handle -fsanitize=return.
testsuite/
	* g++.dg/ubsan/return-1.C: New test.
	* g++.dg/ubsan/return-2.C: New test.


	Jakub

Comments

Jason Merrill Nov. 22, 2013, 1:42 p.m. UTC | #1 
On 11/22/2013 04:28 AM, Jakub Jelinek wrote:
> +  if ((flag_sanitize & SANITIZE_RETURN)
> +      && !VOID_TYPE_P (TREE_TYPE (TREE_TYPE (fndecl)))
> +      && !DECL_CONSTRUCTOR_P (fndecl)
> +      && !DECL_DESTRUCTOR_P (fndecl)
> +      && targetm.warn_func_return (fndecl))
> +    {
...
> +    }

Please split this out into a separate function.

OK with this change and the missing space fix.

Jason
diff mbox

Patch

--- gcc/ubsan.c.jj	2013-11-22 01:40:03.000000000 +0100
+++ gcc/ubsan.c	2013-11-22 10:05:29.491725405 +0100
@@ -227,8 +227,8 @@  ubsan_source_location (location_t loc)
   xloc = expand_location (loc);
 
   /* Fill in the values from LOC.  */
-  size_t len = strlen (xloc.file);
-  tree str = build_string (len + 1, xloc.file);
+  size_t len = xloc.file ? strlen (xloc.file) : 0;
+  tree str = build_string (len + 1, xloc.file ? xloc.file : "");
   TREE_TYPE (str) = build_array_type (char_type_node,
 				      build_index_type (size_int (len)));
   TREE_READONLY (str) = 1;
@@ -642,7 +642,7 @@  ubsan_pass (void)
 	{
 	  struct walk_stmt_info wi;
 	  gimple stmt = gsi_stmt (gsi);
-	  if (is_gimple_debug (stmt))
+	  if (is_gimple_debug (stmt) || gimple_clobber_p (stmt))
 	    {
 	      gsi_next (&gsi);
 	      continue;
--- gcc/sanitizer.def.jj	2013-11-22 01:40:03.000000000 +0100
+++ gcc/sanitizer.def	2013-11-22 09:44:03.818203700 +0100
@@ -297,6 +297,10 @@  DEF_SANITIZER_BUILTIN(BUILT_IN_UBSAN_HAN
 		      "__ubsan_handle_builtin_unreachable",
 		      BT_FN_VOID_PTR,
 		      ATTR_COLD_NORETURN_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_UBSAN_HANDLE_MISSING_RETURN,
+		      "__ubsan_handle_missing_return",
+		      BT_FN_VOID_PTR,
+		      ATTR_NORETURN_NOTHROW_LEAF_LIST)
 DEF_SANITIZER_BUILTIN(BUILT_IN_UBSAN_HANDLE_VLA_BOUND_NOT_POSITIVE,
 		      "__ubsan_handle_vla_bound_not_positive",
 		      BT_FN_VOID_PTR_PTR,
--- gcc/opts.c.jj	2013-11-22 01:40:05.000000000 +0100
+++ gcc/opts.c	2013-11-22 09:08:12.537971216 +0100
@@ -1457,6 +1457,7 @@  common_handle_option (struct gcc_options
 	      { "unreachable", SANITIZE_UNREACHABLE,
 		sizeof "unreachable" - 1 },
 	      { "vla-bound", SANITIZE_VLA, sizeof "vla-bound" - 1 },
+	      { "return", SANITIZE_RETURN, sizeof "return" - 1 },
 	      { "null", SANITIZE_NULL, sizeof "null" - 1 },
 	      { NULL, 0, 0 }
 	    };
--- gcc/flag-types.h.jj	2013-11-22 01:40:03.000000000 +0100
+++ gcc/flag-types.h	2013-11-22 09:07:29.150236112 +0100
@@ -212,8 +212,9 @@  enum sanitize_code {
   SANITIZE_UNREACHABLE = 1 << 4,
   SANITIZE_VLA = 1 << 5,
   SANITIZE_NULL = 1 << 6,
+  SANITIZE_RETURN = 1 << 8,
   SANITIZE_UNDEFINED = SANITIZE_SHIFT | SANITIZE_DIVIDE | SANITIZE_UNREACHABLE
-		       | SANITIZE_VLA | SANITIZE_NULL
+		       | SANITIZE_VLA | SANITIZE_NULL | SANITIZE_RETURN
 };
 
 /* flag_vtable_verify initialization levels. */
--- gcc/c-family/c-ubsan.h.jj	2013-11-12 11:31:11.000000000 +0100
+++ gcc/c-family/c-ubsan.h	2013-11-22 09:38:31.654851809 +0100
@@ -24,5 +24,6 @@  along with GCC; see the file COPYING3.
 extern tree ubsan_instrument_division (location_t, tree, tree);
 extern tree ubsan_instrument_shift (location_t, enum tree_code, tree, tree);
 extern tree ubsan_instrument_vla (location_t, tree);
+extern tree ubsan_instrument_return (location_t);
 
 #endif  /* GCC_C_UBSAN_H  */
--- gcc/c-family/c-ubsan.c.jj	2013-11-19 21:56:20.000000000 +0100
+++ gcc/c-family/c-ubsan.c	2013-11-22 09:52:31.810642606 +0100
@@ -179,3 +179,14 @@  ubsan_instrument_vla (location_t loc, tr
 
   return t;
 }
+
+/* Instrument missing return in C++ functions returning non-void.  */
+
+tree
+ubsan_instrument_return (location_t loc)
+{
+  tree data = ubsan_create_data ("__ubsan_missing_return_data", loc,
+				 NULL,NULL_TREE);
+  tree t = builtin_decl_explicit (BUILT_IN_UBSAN_HANDLE_MISSING_RETURN);
+  return build_call_expr_loc (loc, t, 1, build_fold_addr_expr_loc (loc, data));
+}
--- gcc/cp/cp-gimplify.c.jj	2013-11-19 21:56:25.000000000 +0100
+++ gcc/cp/cp-gimplify.c	2013-11-22 09:53:49.852263241 +0100
@@ -34,6 +34,8 @@  along with GCC; see the file COPYING3.
 #include "pointer-set.h"
 #include "flags.h"
 #include "splay-tree.h"
+#include "target.h"
+#include "c-family/c-ubsan.h"
 
 /* Forward declarations.  */
 
@@ -1235,6 +1237,54 @@  cp_genericize (tree fndecl)
      walk_tree's hash functionality.  */
   cp_genericize_tree (&DECL_SAVED_TREE (fndecl));
 
+  if ((flag_sanitize & SANITIZE_RETURN)
+      && !VOID_TYPE_P (TREE_TYPE (TREE_TYPE (fndecl)))
+      && !DECL_CONSTRUCTOR_P (fndecl)
+      && !DECL_DESTRUCTOR_P (fndecl)
+      && targetm.warn_func_return (fndecl))
+    {
+      tree t = DECL_SAVED_TREE (fndecl);
+      while (t)
+	{
+	  switch (TREE_CODE (t))
+	    {
+	    case BIND_EXPR:
+	      t = BIND_EXPR_BODY (t);
+	      continue;
+	    case TRY_FINALLY_EXPR:
+	      t = TREE_OPERAND (t, 0);
+	      continue;
+	    case STATEMENT_LIST:
+	      {
+		tree_stmt_iterator i = tsi_last (t);
+		if (!tsi_end_p (i))
+		  {
+		    t = tsi_stmt (i);
+		    continue;
+		  }
+	      }
+	      break;
+	    case RETURN_EXPR:
+	      t = NULL_TREE;
+	      break;
+	    default:
+	      break;
+	    }
+	  break;
+	}
+      if (t)
+	{
+	  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);
+	    }
+	}
+    }
+
   /* Do everything else.  */
   c_genericize (fndecl);
 
--- gcc/testsuite/g++.dg/ubsan/return-1.C.jj	2013-11-22 10:09:27.730525521 +0100
+++ gcc/testsuite/g++.dg/ubsan/return-1.C	2013-11-22 10:20:25.124405610 +0100
@@ -0,0 +1,27 @@ 
+// { dg-do run }
+// { dg-options "-fsanitize=return" }
+// { dg-shouldfail "ubsan" }
+
+struct S { S (); ~S (); };
+
+S::S () {}
+S::~S () {}
+
+int
+foo (int x)
+{
+  S a;
+  {
+    S b;
+    if (x)
+      return 1;
+  }
+}
+
+int
+main ()
+{
+  foo (0);
+}
+
+// { dg-output "execution reached the end of a value-returning function without returning a value" }
--- gcc/testsuite/g++.dg/ubsan/return-2.C.jj	2013-11-22 10:09:31.050510359 +0100
+++ gcc/testsuite/g++.dg/ubsan/return-2.C	2013-11-22 10:09:23.734570198 +0100
@@ -0,0 +1,25 @@ 
+// { dg-do run }
+// { dg-options "-fsanitize=return" }
+
+struct S { S (); ~S (); };
+
+S::S () {}
+S::~S () {}
+
+int
+foo (int x)
+{
+  S a;
+  {
+    S b;
+    if (x)
+      return 1;
+  }
+}
+
+int
+main ()
+{
+  foo (1);
+  foo (14);
+}