On Wed, Jun 22, 2022 at 12:04:59AM -0400, Jason Merrill wrote: > On 6/20/22 16:16, Jason Merrill wrote: > > On 6/20/22 07:05, Jakub Jelinek wrote: > > > On Fri, Jun 17, 2022 at 05:20:02PM -0400, Jason Merrill wrote: > > > > 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. > > > > > > __builtin_unreachable itself (unless turned into trap or > > > __ubsan_handle_builtin_unreachable) is not any kind of return > > > checking, it > > > is just an optimization. > > > > Yes, but I'm talking about "when -fsanitize=unreachable".
The usual case is that people just use -fsanitize=undefined and get both return and unreachable sanitization, for fall through into end of functions returning non-void done through return sanitization. In the rare case people use something different like -fsanitize=undefined -fno-sanitize=return or -fsanitize=unreachable etc., they presumably don't want the fall through from end of function diagnosed at runtime. I think the behavior we want is: 1) -fsanitize=return is on -> use ubsan_instrument_return (__ubsan_missing_return_data or __builtin_trap depending on -fsanitize-trap=return); otherwise 2) -funreachable-traps is on (from -O0/-Og by default or explicit), emit __builtin_trap; otherwise 3) -fsanitize=unreachable is on, not emit anything (__builtin_unreachable would be just an optimization, but user asked not to instrument returns, only unreachable, so honor user's decision and avoid confusion); otherwise 4) -O0 is on, not emit anything (__builtin_unreachable wouldn't be much of an optimization, just surprising and hard to debug effect); otherwise 5) emit __builtin_unreachable Current trunk with your PR104642 fix in implements 1), will do 2) only if -fsanitize=unreachable is not on, will do 3), will do 4) and 5). So, I'd change cp-gimplify.cc (cp_maybe_instrument_return), change: if (!sanitize_flags_p (SANITIZE_RETURN, fndecl) && ((!optimize && !flag_unreachable_traps) || sanitize_flags_p (SANITIZE_UNREACHABLE, fndecl))) return; to if (!sanitize_flags_p (SANITIZE_RETURN, fndecl) && !flag_unreachable_traps && (!optimize || sanitize_flags_p (SANITIZE_UNREACHABLE, fndecl))) return; and if (sanitize_flags_p (SANITIZE_RETURN, fndecl)) t = ubsan_instrument_return (loc); else t = build_builtin_unreachable (BUILTINS_LOCATION); to if (sanitize_flags_p (SANITIZE_RETURN, fndecl)) t = ubsan_instrument_return (loc); else if (flag_unreachable_traps) t = build_call_expr_loc (BUILTINS_LOCATION, builtin_decl_explicit (BUILT_IN_TRAP), 0); else t = build_builtin_unreachable (BUILTINS_LOCATION); Jakub