Hi! While we have a warning for falling through out of a noreturn function or return in such function, the actual UB occurs only if we actually return from those functions.
This patch attempts to instrument it. Will need to submit the libsanitizer part upstream first though. 2017-10-17 Jakub Jelinek <ja...@redhat.com> * flag-types.h (enum sanitize_code): Add SANITIZE_NORETURN. Or SANITIZE_NORETURN into SANITIZE_UNDEFINED. * sanitizer.def (BUILT_IN_UBSAN_HANDLE_NORETURN): New builtin. * common.opt (flag_sanitize_recover): Don't set SANITIZE_NORETURN. * opts.c (finish_options): Don't clear aggressive loop opts just for SANITIZE_RETURN or SANITIZE_NORETURN. (sanitizer_opts): Add noreturn. (parse_sanitizer_options, common_handle_option): Handle SANITIZE_NORETURN like other non-recoverable sanitizers. * ubsan.c (instrument_noreturn): New function. (pass_ubsan::execute): Call it. (pass_ubsan::gate): Enable even for SANITIZE_NORETURN. * doc/invoke.texi: Document -fsanitize=noreturn. * c-c++-common/ubsan/noreturn-1.c: New test. * c-c++-common/ubsan/noreturn-2.c: New test. * ubsan/ubsan_handlers.h (NoreturnData): New type. (noreturn): New UNRECOVERABLE handler. * ubsan/ubsan_handlers.cc (handleNoreturn): New function. (__ubsan::__ubsan_handle_noreturn): Likewise. * ubsan/ubsan_checks.inc (InvalidNoreturn): New UBSAN_CHECK. --- gcc/flag-types.h.jj 2017-10-17 11:08:00.000000000 +0200 +++ gcc/flag-types.h 2017-10-17 13:47:25.905757381 +0200 @@ -247,6 +247,7 @@ enum sanitize_code { SANITIZE_BOUNDS_STRICT = 1UL << 23, SANITIZE_POINTER_OVERFLOW = 1UL << 24, SANITIZE_BUILTIN = 1UL << 25, + SANITIZE_NORETURN = 1UL << 26, SANITIZE_SHIFT = SANITIZE_SHIFT_BASE | SANITIZE_SHIFT_EXPONENT, SANITIZE_UNDEFINED = SANITIZE_SHIFT | SANITIZE_DIVIDE | SANITIZE_UNREACHABLE | SANITIZE_VLA | SANITIZE_NULL | SANITIZE_RETURN @@ -255,7 +256,8 @@ enum sanitize_code { | SANITIZE_NONNULL_ATTRIBUTE | SANITIZE_RETURNS_NONNULL_ATTRIBUTE | SANITIZE_OBJECT_SIZE | SANITIZE_VPTR - | SANITIZE_POINTER_OVERFLOW | SANITIZE_BUILTIN, + | SANITIZE_POINTER_OVERFLOW | SANITIZE_BUILTIN + | SANITIZE_NORETURN, SANITIZE_UNDEFINED_NONDEFAULT = SANITIZE_FLOAT_DIVIDE | SANITIZE_FLOAT_CAST | SANITIZE_BOUNDS_STRICT }; --- gcc/sanitizer.def.jj 2017-10-17 11:03:15.000000000 +0200 +++ gcc/sanitizer.def 2017-10-17 14:51:16.157455132 +0200 @@ -532,6 +532,10 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_UBSAN_HAN "__ubsan_handle_invalid_builtin_abort", BT_FN_VOID_PTR, ATTR_COLD_NORETURN_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_UBSAN_HANDLE_NORETURN, + "__ubsan_handle_noreturn", + BT_FN_VOID_PTR_PTR, + ATTR_COLD_NORETURN_NOTHROW_LEAF_LIST) DEF_SANITIZER_BUILTIN(BUILT_IN_UBSAN_HANDLE_DYNAMIC_TYPE_CACHE_MISS, "__ubsan_handle_dynamic_type_cache_miss", BT_FN_VOID_PTR_PTR_PTR, --- gcc/common.opt.jj 2017-10-12 20:51:36.000000000 +0200 +++ gcc/common.opt 2017-10-17 15:16:34.945668469 +0200 @@ -231,7 +231,7 @@ unsigned int flag_sanitize ; What sanitizers should recover from errors Variable -unsigned int flag_sanitize_recover = (SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT | SANITIZE_KERNEL_ADDRESS) & ~(SANITIZE_UNREACHABLE | SANITIZE_RETURN) +unsigned int flag_sanitize_recover = (SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT | SANITIZE_KERNEL_ADDRESS) & ~(SANITIZE_UNREACHABLE | SANITIZE_RETURN | SANITIZE_NORETURN) ; What the coverage sanitizers should instrument Variable --- gcc/opts.c.jj 2017-10-17 11:08:42.000000000 +0200 +++ gcc/opts.c 2017-10-17 14:49:34.716638724 +0200 @@ -985,7 +985,8 @@ finish_options (struct gcc_options *opts opts->x_flag_delete_null_pointer_checks = 0; /* Aggressive compiler optimizations may cause false negatives. */ - if (opts->x_flag_sanitize & ~(SANITIZE_LEAK | SANITIZE_UNREACHABLE)) + if (opts->x_flag_sanitize & ~(SANITIZE_LEAK | SANITIZE_UNREACHABLE + | SANITIZE_RETURN | SANITIZE_NORETURN)) opts->x_flag_aggressive_loop_optimizations = 0; /* Enable -fsanitize-address-use-after-scope if address sanitizer is @@ -1522,6 +1523,7 @@ const struct sanitizer_opts_s sanitizer_ SANITIZER_OPT (vptr, SANITIZE_VPTR, true), SANITIZER_OPT (pointer-overflow, SANITIZE_POINTER_OVERFLOW, true), SANITIZER_OPT (builtin, SANITIZE_BUILTIN, true), + SANITIZER_OPT (noreturn, SANITIZE_NORETURN, false), SANITIZER_OPT (all, ~0U, true), #undef SANITIZER_OPT { NULL, 0U, 0UL, false } @@ -1646,7 +1648,8 @@ parse_sanitizer_options (const char *p, } else flags |= ~(SANITIZE_THREAD | SANITIZE_LEAK - | SANITIZE_UNREACHABLE | SANITIZE_RETURN); + | SANITIZE_UNREACHABLE | SANITIZE_RETURN + | SANITIZE_NORETURN); } else if (value) { @@ -1656,7 +1659,8 @@ parse_sanitizer_options (const char *p, if (code == OPT_fsanitize_recover_ && opts[i].flag == SANITIZE_UNDEFINED) flags |= (SANITIZE_UNDEFINED - & ~(SANITIZE_UNREACHABLE | SANITIZE_RETURN)); + & ~(SANITIZE_UNREACHABLE | SANITIZE_RETURN + | SANITIZE_NORETURN)); else flags |= opts[i].flag; } @@ -1977,7 +1981,8 @@ common_handle_option (struct gcc_options if (value) opts->x_flag_sanitize_recover |= (SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT) - & ~(SANITIZE_UNREACHABLE | SANITIZE_RETURN); + & ~(SANITIZE_UNREACHABLE | SANITIZE_RETURN + | SANITIZE_NORETURN); else opts->x_flag_sanitize_recover &= ~(SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT); --- gcc/ubsan.c.jj 2017-10-17 15:55:42.409131863 +0200 +++ gcc/ubsan.c 2017-10-17 15:56:07.441836231 +0200 @@ -2287,6 +2287,37 @@ instrument_builtin (gimple_stmt_iterator } } +/* Instrument returns in functions with noreturn attribute. */ + +static void +instrument_noreturn (gimple_stmt_iterator *gsi) +{ + location_t loc[2]; + gimple *stmt = gsi_stmt (*gsi), *g; + loc[0] = gimple_location (stmt); + if (loc[0] == UNKNOWN_LOCATION) + loc[0] = cfun->function_end_locus; + loc[1] = UNKNOWN_LOCATION; + + if (flag_sanitize_undefined_trap_on_error) + g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); + else + { + tree data = ubsan_create_data ("__ubsan_noreturn_data", + 1, &loc[1], NULL_TREE, NULL_TREE); + data = build_fold_addr_expr_loc (loc[0], data); + tree data2 = ubsan_create_data ("__ubsan_noreturn_data", + 1, &loc[0], NULL_TREE, NULL_TREE); + data2 = build_fold_addr_expr_loc (loc[0], data2); + tree fn = builtin_decl_explicit (BUILT_IN_UBSAN_HANDLE_NORETURN); + g = gimple_build_call (fn, 2, data, data2); + } + gimple_set_location (g, loc[0]); + gsi_insert_before (gsi, g, GSI_SAME_STMT); + ubsan_create_edge (g); + *gsi = gsi_for_stmt (stmt); +} + namespace { const pass_data pass_data_ubsan = @@ -2319,7 +2350,7 @@ public: | SANITIZE_RETURNS_NONNULL_ATTRIBUTE | SANITIZE_OBJECT_SIZE | SANITIZE_POINTER_OVERFLOW - | SANITIZE_BUILTIN)); + | SANITIZE_BUILTIN | SANITIZE_NORETURN)); } virtual unsigned int execute (function *); @@ -2398,6 +2429,14 @@ pass_ubsan::execute (function *fun) bb = gimple_bb (stmt); } + if (sanitize_flags_p (SANITIZE_NORETURN, fun->decl) + && TREE_THIS_VOLATILE (fun->decl) + && gimple_code (stmt) == GIMPLE_RETURN) + { + instrument_noreturn (&gsi); + bb = gimple_bb (stmt); + } + if (sanitize_flags_p (SANITIZE_OBJECT_SIZE, fun->decl)) { if (gimple_store_p (stmt)) --- gcc/doc/invoke.texi.jj 2017-10-17 13:51:26.000000000 +0200 +++ gcc/doc/invoke.texi 2017-10-17 15:12:20.737616739 +0200 @@ -11150,6 +11150,13 @@ error is issued. E.g.@ passing 0 as the or @code{__builtin_clz} invokes undefined behavior and is diagnosed by this option. +@item -fsanitize=noreturn +@opindex fsanitize=noreturn + +This option enables instrumentation of functions with @code{noreturn} +attribute or @code{_Noreturn} function specifier. If such a function +returns or end of such a function is reached, a run-time error is issued. + @end table While @option{-ftrapv} causes traps for signed overflows to be emitted, --- gcc/testsuite/c-c++-common/ubsan/noreturn-1.c.jj 2017-10-17 15:29:50.170726971 +0200 +++ gcc/testsuite/c-c++-common/ubsan/noreturn-1.c 2017-10-17 15:50:29.274829919 +0200 @@ -0,0 +1,23 @@ +/* { dg-do run } */ +/* { dg-shouldfail "ubsan" } */ +/* { dg-options "-fsanitize=undefined" } */ + +#if __cplusplus >= 201103L +[[noreturn]] +#elif __STDC_VERSION__ >= 201112L +_Noreturn +#else +__attribute__((noreturn)) +#endif +void +foo (int x) +{ +} /* { dg-warning "function does return" } */ + +int +main () +{ + foo (2); +} + +/* { dg-output "\.c:15:\[0-9]*:\[^\n\r]*return from function declared to never return" } */ --- gcc/testsuite/c-c++-common/ubsan/noreturn-2.c.jj 2017-10-17 15:34:26.863623094 +0200 +++ gcc/testsuite/c-c++-common/ubsan/noreturn-2.c 2017-10-17 15:53:24.996754676 +0200 @@ -0,0 +1,26 @@ +/* { dg-do run } */ +/* { dg-shouldfail "ubsan" } */ +/* { dg-options "-fsanitize=noreturn" } */ + +#if __cplusplus >= 201103L +[[noreturn]] +#elif __STDC_VERSION__ >= 201112L +_Noreturn +#else +__attribute__((noreturn)) +#endif +void +foo (int x) +{ + if (x < 2) + __builtin_exit (0); + return; /* { dg-warning "function does return" } */ +} /* { dg-warning "function declared 'noreturn' has a 'return' statement" "" { target *-*-* } 17 } */ + +int +main () +{ + foo (2); +} + +/* { dg-output "\.c:17:\[0-9]*:\[^\n\r]*return from function declared to never return" } */ --- libsanitizer/ubsan/ubsan_handlers.h.jj 2017-10-17 10:10:27.000000000 +0200 +++ libsanitizer/ubsan/ubsan_handlers.h 2017-10-17 10:49:22.735633089 +0200 @@ -164,6 +164,14 @@ struct NonNullArgData { RECOVERABLE(nonnull_arg, NonNullArgData *Data) RECOVERABLE(nullability_arg, NonNullArgData *Data) +struct NoreturnData { + SourceLocation AttrLoc; +}; + +/// \brief Handle returning from function with the noreturn +/// attribute. +UNRECOVERABLE(noreturn, NoreturnData *Data, SourceLocation *Loc) + struct PointerOverflowData { SourceLocation Loc; }; --- libsanitizer/ubsan/ubsan_handlers.cc.jj 2017-10-17 10:10:27.000000000 +0200 +++ libsanitizer/ubsan/ubsan_handlers.cc 2017-10-17 10:51:26.187090562 +0200 @@ -583,6 +583,32 @@ void __ubsan::__ubsan_handle_nullability Die(); } +static void handleNoreturn(NoreturnData *Data, SourceLocation *LocPtr, + ReportOptions Opts) { + if (!LocPtr) + UNREACHABLE("source location pointer is null!"); + + SourceLocation Loc = LocPtr->acquire(); + ErrorType ET = ErrorType::InvalidNoreturn; + + if (ignoreReport(Loc, Opts, ET)) + return; + + ScopedReport R(Opts, Loc, ET); + + Diag(Loc, DL_Error, "return from function declared to never " + "return"); + if (!Data->AttrLoc.isInvalid()) + Diag(Data->AttrLoc, DL_Note, "noreturn attribute specified here"); +} + +void __ubsan::__ubsan_handle_noreturn(NoreturnData *Data, + SourceLocation *LocPtr) { + GET_REPORT_OPTIONS(true); + handleNoreturn(Data, LocPtr, Opts); + Die(); +} + static void handlePointerOverflowImpl(PointerOverflowData *Data, ValueHandle Base, ValueHandle Result, --- libsanitizer/ubsan/ubsan_checks.inc.jj 2017-10-17 10:10:27.000000000 +0200 +++ libsanitizer/ubsan/ubsan_checks.inc 2017-10-17 10:46:28.621808642 +0200 @@ -41,5 +41,6 @@ UBSAN_CHECK(FunctionTypeMismatch, "funct UBSAN_CHECK(InvalidNullReturn, "invalid-null-return", "returns-nonnull-attribute") UBSAN_CHECK(InvalidNullArgument, "invalid-null-argument", "nonnull-attribute") +UBSAN_CHECK(InvalidNoreturn, "invalid-noreturn", "noreturn") UBSAN_CHECK(DynamicTypeMismatch, "dynamic-type-mismatch", "vptr") UBSAN_CHECK(CFIBadType, "cfi-bad-type", "cfi") Jakub