From da268c4c1f9ac0a7eaa4d428791c3ed51cf0994d Mon Sep 17 00:00:00 2001
From: Jason Merrill <jason@redhat.com>
Date: Wed, 15 Jun 2022 15:45:48 -0400
Subject: [PATCH] ubsan: do return check with -fsanitize=unreachable
To: gcc-patches@gcc.gnu.org

The current situation where we get less return checking with just
-fsanitize=unreachable than no sanitize flags seems undesirable; we would
get checking there except that we explicitly avoid emitting a
__builtin_unreachable.  The documented reason is that the use of
BUILTIN_LOCATION makes the message confusing, so let's fix that.

The !optimize check seems unneded since the PR104642 patch.

gcc/cp/ChangeLog:

	* cp-gimplify.cc (cp_maybe_instrument_return): Pass the real
	location to the ubsan unreachable function.

gcc/ChangeLog:

	* tree-cfg.cc (pass_warn_function_return::execute): Check
	for ubsan unreachable.

gcc/testsuite/ChangeLog:

	* g++.dg/ubsan/return-8c.C: New test.
---
 gcc/cp/cp-gimplify.cc                  | 19 ++++++-------------
 gcc/testsuite/g++.dg/ubsan/return-8c.C | 16 ++++++++++++++++
 gcc/tree-cfg.cc                        |  3 +++
 3 files changed, 25 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/ubsan/return-8c.C

diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index c05be833357..fcea9f8d0e0 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -1806,18 +1806,6 @@ cp_maybe_instrument_return (tree fndecl)
       || !targetm.warn_func_return (fndecl))
     return;
 
-  if (!sanitize_flags_p (SANITIZE_RETURN, fndecl)
-      /* Don't add __builtin_unreachable () if not optimizing, it will not
-	 improve any optimizations in that case, just break UB code.
-	 Don't add it if -fsanitize=unreachable -fno-sanitize=return either,
-	 UBSan covers this with ubsan_instrument_return above where sufficient
-	 information is provided, while the __builtin_unreachable () below
-	 if return sanitization is disabled will just result in hard to
-	 understand runtime error without location.  */
-      && ((!optimize && !flag_unreachable_traps)
-	  || sanitize_flags_p (SANITIZE_UNREACHABLE, fndecl)))
-    return;
-
   tree t = DECL_SAVED_TREE (fndecl);
   while (t)
     {
@@ -1864,7 +1852,12 @@ cp_maybe_instrument_return (tree fndecl)
   if (sanitize_flags_p (SANITIZE_RETURN, fndecl))
     t = ubsan_instrument_return (loc);
   else
-    t = build_builtin_unreachable (BUILTINS_LOCATION);
+    {
+      /* Pass the real location to the ubsan function.  */
+      t = build_builtin_unreachable (loc);
+      /* But set BUILTINS_LOCATION for pass_warn_function_return.  */
+      SET_EXPR_LOCATION (t, BUILTINS_LOCATION);
+    }
 
   append_to_statement_list (t, p);
 }
diff --git a/gcc/testsuite/g++.dg/ubsan/return-8c.C b/gcc/testsuite/g++.dg/ubsan/return-8c.C
new file mode 100644
index 00000000000..828b24efa31
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ubsan/return-8c.C
@@ -0,0 +1,16 @@
+// PR c++/104642
+
+// -fsanitize=unreachable should catch missing return even without
+// -fsanitize=return.
+
+// { dg-do run }
+// { dg-shouldfail { *-*-* } }
+// { dg-additional-options "-O -fsanitize=unreachable" }
+
+bool b;
+
+int f() {
+  if (b) return 42;
+} // { dg-warning "-Wreturn-type" }
+
+int main() { f(); }
diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index 734fdddbf97..647d98f0079 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -9551,10 +9551,13 @@ pass_warn_function_return::execute (function *fun)
 	      gimple *last = last_stmt (bb);
 	      const enum built_in_function ubsan_missing_ret
 		= BUILT_IN_UBSAN_HANDLE_MISSING_RETURN;
+	      const enum built_in_function ubsan_unreachable
+		= BUILT_IN_UBSAN_HANDLE_BUILTIN_UNREACHABLE;
 	      if (last
 		  && ((LOCATION_LOCUS (gimple_location (last))
 		       == BUILTINS_LOCATION
 		       && (gimple_call_builtin_p (last, BUILT_IN_UNREACHABLE)
+			   || gimple_call_builtin_p (last, ubsan_unreachable)
 			   || gimple_call_builtin_p (last, BUILT_IN_TRAP)))
 		      || gimple_call_builtin_p (last, ubsan_missing_ret)))
 		{
-- 
2.27.0