Related to PR104642, the current situation where we get less return checking with just -fsanitize=unreachable than no sanitize flags seems undesirable; I propose that we do return checking when -fsanitize=unreachable.
Looks like clang just traps on missing return if not -fsanitize=return, but the approach in this patch seems more helpful to me if we're already sanitizing other should-be-unreachable code. I'm assuming that the difference in treatment of SANITIZE_UNREACHABLE and SANITIZE_RETURN with regard to loop optimization is deliberate. This assumes Jakub's -fsanitize-trap patch. gcc/ChangeLog: * doc/invoke.texi: Note that -fsanitize=unreachable implies -fsanitize=return. * opts.cc (finish_options): Make that so. gcc/cp/ChangeLog: * cp-gimplify.cc (cp_maybe_instrument_return): Remove return vs. unreachable handling. gcc/testsuite/ChangeLog: * g++.dg/ubsan/return-8c.C: New test. --- gcc/doc/invoke.texi | 2 ++ gcc/cp/cp-gimplify.cc | 12 ------------ gcc/opts.cc | 10 ++++++++++ gcc/testsuite/g++.dg/ubsan/return-8c.C | 15 +++++++++++++++ 4 files changed, 27 insertions(+), 12 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ubsan/return-8c.C diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 50f57877477..e572158a1ba 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -15946,6 +15946,8 @@ built with this option turned on will issue an error message when the end of a non-void function is reached without actually returning a value. This option works in C++ only. +This check is also enabled by -fsanitize=unreachable. + @item -fsanitize=signed-integer-overflow @opindex fsanitize=signed-integer-overflow This option enables signed integer overflow checking. We check that diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc index 6f84d157c98..5c2eb61842c 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 - || sanitize_flags_p (SANITIZE_UNREACHABLE, fndecl))) - return; - tree t = DECL_SAVED_TREE (fndecl); while (t) { diff --git a/gcc/opts.cc b/gcc/opts.cc index 062782ac700..a7b02b0f693 100644 --- a/gcc/opts.cc +++ b/gcc/opts.cc @@ -1254,6 +1254,16 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set, if (opts->x_flag_sanitize & ~(SANITIZE_LEAK | SANITIZE_UNREACHABLE)) opts->x_flag_aggressive_loop_optimizations = 0; + /* -fsanitize=unreachable implies -fsanitize=return, but without affecting + aggressive loop optimizations. */ + if ((opts->x_flag_sanitize & (SANITIZE_UNREACHABLE | SANITIZE_RETURN)) + == SANITIZE_UNREACHABLE) + { + opts->x_flag_sanitize |= SANITIZE_RETURN; + if (opts->x_flag_sanitize_trap & SANITIZE_UNREACHABLE) + opts->x_flag_sanitize_trap |= SANITIZE_RETURN; + } + /* Enable -fsanitize-address-use-after-scope if either address sanitizer is enabled. */ if (opts->x_flag_sanitize 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..a67f086d452 --- /dev/null +++ b/gcc/testsuite/g++.dg/ubsan/return-8c.C @@ -0,0 +1,15 @@ +// PR c++/104642 + +// -fsanitize=unreachable should imply -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(); } base-commit: 0f96ac43fa0a5fdbfce317b274233852d5b46d23 prerequisite-patch-id: fa35013a253eae78fe744794172aeed26fe6f166 -- 2.27.0