As Richi pointed in the pr audit trail, instrumenting via folding is bad. In this case we changed __builtin_unreachable, created by the inliner, into BUILT_IN_UBSAN_HANDLE_BUILTIN_UNREACHABLE which requires VOPS, which is a no-no in folding. So this patch: - marks BUILT_IN_UBSAN_HANDLE_BUILTIN_UNREACHABLE as const to match BUILT_IN_UNREACHABLE, - moves the __builtin_unreachable instrumentation into sanopt pass, - disables optimize_unreachable when doing the __builtin_unreachable instrumentation, - marks BUILT_IN_UNREACHABLE as "cold" (I don't see how this could make a difference?). Now, BUILT_IN_TRAP should probably be also marked as const+cold; I'm happy to do that as a follow-up.
Bootstrapped/regtested on power8-linux, ok for trunk? 2014-11-13 Marek Polacek <pola...@redhat.com> PR sanitizer/63839 * asan.c (ATTR_CONST_NORETURN_NOTHROW_LEAF_LIST, ATTR_COLD_CONST_NORETURN_NOTHROW_LEAF_LIST): Define. * builtin-attrs.def (ATTR_COLD_CONST_NORETURN_NOTHROW_LEAF_LIST): Define. * builtins.c (fold_builtin_0): Don't include ubsan.h. Don't instrument BUILT_IN_UNREACHABLE here. * builtins.def (BUILT_IN_UNREACHABLE): Make cold. * sanitizer.def (BUILT_IN_UBSAN_HANDLE_BUILTIN_UNREACHABLE): Make const. * sanopt.c (pass_sanopt::execute): Instrument BUILT_IN_UNREACHABLE. * tree-ssa-ccp.c (optimize_unreachable): Bail out if SANITIZE_UNREACHABLE. * ubsan.c (ubsan_instrument_unreachable): Rewrite for GIMPLE. * ubsan.h (ubsan_instrument_unreachable): Adjust declaration. testsuite/ * c-c++-common/ubsan/pr63839.c: New test. * c-c++-common/ubsan/unreachable-2.c: New test. diff --git gcc/asan.c gcc/asan.c index 79dede7..2961b44 100644 --- gcc/asan.c +++ gcc/asan.c @@ -2346,6 +2346,9 @@ initialize_sanitizer_builtins (void) #define ATTR_TMPURE_NOTHROW_LEAF_LIST ECF_TM_PURE | ATTR_NOTHROW_LEAF_LIST #undef ATTR_NORETURN_NOTHROW_LEAF_LIST #define ATTR_NORETURN_NOTHROW_LEAF_LIST ECF_NORETURN | ATTR_NOTHROW_LEAF_LIST +#undef ATTR_CONST_NORETURN_NOTHROW_LEAF_LIST +#define ATTR_CONST_NORETURN_NOTHROW_LEAF_LIST \ + ECF_CONST | ATTR_NORETURN_NOTHROW_LEAF_LIST #undef ATTR_TMPURE_NORETURN_NOTHROW_LEAF_LIST #define ATTR_TMPURE_NORETURN_NOTHROW_LEAF_LIST \ ECF_TM_PURE | ATTR_NORETURN_NOTHROW_LEAF_LIST @@ -2355,6 +2358,9 @@ initialize_sanitizer_builtins (void) #undef ATTR_COLD_NORETURN_NOTHROW_LEAF_LIST #define ATTR_COLD_NORETURN_NOTHROW_LEAF_LIST \ /* ECF_COLD missing */ ATTR_NORETURN_NOTHROW_LEAF_LIST +#undef ATTR_COLD_CONST_NORETURN_NOTHROW_LEAF_LIST +#define ATTR_COLD_CONST_NORETURN_NOTHROW_LEAF_LIST \ + /* ECF_COLD missing */ ATTR_CONST_NORETURN_NOTHROW_LEAF_LIST #undef DEF_SANITIZER_BUILTIN #define DEF_SANITIZER_BUILTIN(ENUM, NAME, TYPE, ATTRS) \ decl = add_builtin_function ("__builtin_" NAME, TYPE, ENUM, \ diff --git gcc/builtin-attrs.def gcc/builtin-attrs.def index 9c05a94..c707367 100644 --- gcc/builtin-attrs.def +++ gcc/builtin-attrs.def @@ -145,6 +145,8 @@ DEF_ATTR_TREE_LIST (ATTR_SENTINEL_NOTHROW_LIST, ATTR_SENTINEL, \ ATTR_NULL, ATTR_NOTHROW_LIST) DEF_ATTR_TREE_LIST (ATTR_SENTINEL_NOTHROW_LEAF_LIST, ATTR_SENTINEL, \ ATTR_NULL, ATTR_NOTHROW_LEAF_LIST) +DEF_ATTR_TREE_LIST (ATTR_COLD_CONST_NORETURN_NOTHROW_LEAF_LIST, ATTR_CONST,\ + ATTR_NULL, ATTR_COLD_NORETURN_NOTHROW_LEAF_LIST) /* Functions whose pointer parameter(s) are all nonnull. */ DEF_ATTR_TREE_LIST (ATTR_NONNULL_LIST, ATTR_NONNULL, ATTR_NULL, ATTR_NULL) diff --git gcc/builtins.c gcc/builtins.c index 1cd65ed..311c0e3 100644 --- gcc/builtins.c +++ gcc/builtins.c @@ -64,7 +64,6 @@ along with GCC; see the file COPYING3. If not see #include "diagnostic-core.h" #include "builtins.h" #include "asan.h" -#include "ubsan.h" #include "cilk.h" #include "ipa-ref.h" #include "lto-streamer.h" @@ -9803,14 +9802,6 @@ fold_builtin_0 (location_t loc, tree fndecl, bool ignore ATTRIBUTE_UNUSED) case BUILT_IN_CLASSIFY_TYPE: return fold_builtin_classify_type (NULL_TREE); - case BUILT_IN_UNREACHABLE: - if (flag_sanitize & SANITIZE_UNREACHABLE - && (current_function_decl == NULL - || !lookup_attribute ("no_sanitize_undefined", - DECL_ATTRIBUTES (current_function_decl)))) - return ubsan_instrument_unreachable (loc); - break; - default: break; } diff --git gcc/builtins.def gcc/builtins.def index 0406016..8699a41 100644 --- gcc/builtins.def +++ gcc/builtins.def @@ -803,7 +803,7 @@ DEF_GCC_BUILTIN (BUILT_IN_SETJMP, "setjmp", BT_FN_INT_PTR, ATTR_NOTHROW_L DEF_EXT_LIB_BUILTIN (BUILT_IN_STRFMON, "strfmon", BT_FN_SSIZE_STRING_SIZE_CONST_STRING_VAR, ATTR_FORMAT_STRFMON_NOTHROW_3_4) DEF_LIB_BUILTIN (BUILT_IN_STRFTIME, "strftime", BT_FN_SIZE_STRING_SIZE_CONST_STRING_CONST_PTR, ATTR_FORMAT_STRFTIME_NOTHROW_3_0) DEF_GCC_BUILTIN (BUILT_IN_TRAP, "trap", BT_FN_VOID, ATTR_NORETURN_NOTHROW_LEAF_LIST) -DEF_GCC_BUILTIN (BUILT_IN_UNREACHABLE, "unreachable", BT_FN_VOID, ATTR_CONST_NORETURN_NOTHROW_LEAF_LIST) +DEF_GCC_BUILTIN (BUILT_IN_UNREACHABLE, "unreachable", BT_FN_VOID, ATTR_COLD_CONST_NORETURN_NOTHROW_LEAF_LIST) DEF_GCC_BUILTIN (BUILT_IN_UNWIND_INIT, "unwind_init", BT_FN_VOID, ATTR_NULL) DEF_GCC_BUILTIN (BUILT_IN_UPDATE_SETJMP_BUF, "update_setjmp_buf", BT_FN_VOID_PTR_INT, ATTR_NULL) DEF_GCC_BUILTIN (BUILT_IN_VA_COPY, "va_copy", BT_FN_VOID_VALIST_REF_VALIST_ARG, ATTR_NOTHROW_LEAF_LIST) diff --git gcc/sanitizer.def gcc/sanitizer.def index cddc5ea..3fc8c83 100644 --- gcc/sanitizer.def +++ gcc/sanitizer.def @@ -394,7 +394,7 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS, DEF_SANITIZER_BUILTIN(BUILT_IN_UBSAN_HANDLE_BUILTIN_UNREACHABLE, "__ubsan_handle_builtin_unreachable", BT_FN_VOID_PTR, - ATTR_COLD_NORETURN_NOTHROW_LEAF_LIST) + ATTR_COLD_CONST_NORETURN_NOTHROW_LEAF_LIST) DEF_SANITIZER_BUILTIN(BUILT_IN_UBSAN_HANDLE_MISSING_RETURN, "__ubsan_handle_missing_return", BT_FN_VOID_PTR, diff --git gcc/sanopt.c gcc/sanopt.c index 0fc032a..fe2e42d 100644 --- gcc/sanopt.c +++ gcc/sanopt.c @@ -312,6 +312,21 @@ pass_sanopt::execute (function *fun) break; } } + else if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL)) + { + tree callee = gimple_call_fndecl (stmt); + switch (DECL_FUNCTION_CODE (callee)) + { + case BUILT_IN_UNREACHABLE: + if (flag_sanitize & SANITIZE_UNREACHABLE + && !lookup_attribute ("no_sanitize_undefined", + DECL_ATTRIBUTES (fun->decl))) + no_next = ubsan_instrument_unreachable (&gsi); + break; + default: + break; + } + } if (dump_file && (dump_flags & TDF_DETAILS)) { diff --git gcc/testsuite/c-c++-common/ubsan/pr63839.c gcc/testsuite/c-c++-common/ubsan/pr63839.c index e69de29..e3933f7 100644 --- gcc/testsuite/c-c++-common/ubsan/pr63839.c +++ gcc/testsuite/c-c++-common/ubsan/pr63839.c @@ -0,0 +1,23 @@ +/* { dg-do run } */ +/* { dg-options "-fsanitize=unreachable" } */ +/* { dg-skip-if "" { *-*-* } { "*" } { "-O2" } } */ +/* { dg-shouldfail "ubsan" } */ + +static void __attribute__ ((noreturn)) +bar () +{ +} /* { dg-warning "function does return" } */ + +void +foo () +{ + bar (); +} + +int +main (void) +{ + foo (); +} + +/* { dg-output "execution reached a __builtin_unreachable\\(\\) call" } */ diff --git gcc/testsuite/c-c++-common/ubsan/unreachable-2.c gcc/testsuite/c-c++-common/ubsan/unreachable-2.c index e69de29..783ebc2 100644 --- gcc/testsuite/c-c++-common/ubsan/unreachable-2.c +++ gcc/testsuite/c-c++-common/ubsan/unreachable-2.c @@ -0,0 +1,14 @@ +/* { dg-do run } */ +/* { dg-options "-fsanitize=unreachable" } */ +/* { dg-skip-if "" { *-*-* } { "*" } { "-O2" } } */ +/* { dg-shouldfail "ubsan" } */ + +int e; + +int +main (void) +{ + return e ? 0 : (__builtin_unreachable (), 1); +} + +/* { dg-output "execution reached a __builtin_unreachable\\(\\) call" } */ diff --git gcc/tree-ssa-ccp.c gcc/tree-ssa-ccp.c index 52d8503..31ca0e1 100644 --- gcc/tree-ssa-ccp.c +++ gcc/tree-ssa-ccp.c @@ -2568,6 +2568,9 @@ optimize_unreachable (gimple_stmt_iterator i) edge e; bool ret; + if (flag_sanitize & SANITIZE_UNREACHABLE) + return false; + for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) { stmt = gsi_stmt (gsi); diff --git gcc/ubsan.c gcc/ubsan.c index 41cf546..b5b1b92 100644 --- gcc/ubsan.c +++ gcc/ubsan.c @@ -588,17 +588,26 @@ ubsan_create_data (const char *name, int loccnt, const location_t *ploc, ...) /* Instrument the __builtin_unreachable call. We just call the libubsan routine instead. */ -tree -ubsan_instrument_unreachable (location_t loc) +bool +ubsan_instrument_unreachable (gimple_stmt_iterator *gsi) { - if (flag_sanitize_undefined_trap_on_error) - return build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), 0); + gimple g; + location_t loc = gimple_location (gsi_stmt (*gsi)); - initialize_sanitizer_builtins (); - tree data = ubsan_create_data ("__ubsan_unreachable_data", 1, &loc, NULL_TREE, - NULL_TREE); - tree t = builtin_decl_explicit (BUILT_IN_UBSAN_HANDLE_BUILTIN_UNREACHABLE); - return build_call_expr_loc (loc, t, 1, build_fold_addr_expr_loc (loc, data)); + 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_unreachable_data", 1, &loc, + NULL_TREE, NULL_TREE); + data = build_fold_addr_expr_loc (loc, data); + tree fn + = builtin_decl_explicit (BUILT_IN_UBSAN_HANDLE_BUILTIN_UNREACHABLE); + g = gimple_build_call (fn, 1, data); + } + gimple_set_location (g, loc); + gsi_replace (gsi, g, false); + return false; } /* Return true if T is a call to a libubsan routine. */ diff --git gcc/ubsan.h gcc/ubsan.h index 27c18eb..dcdbb4f 100644 --- gcc/ubsan.h +++ gcc/ubsan.h @@ -41,7 +41,7 @@ enum ubsan_print_style { extern bool ubsan_expand_bounds_ifn (gimple_stmt_iterator *); extern bool ubsan_expand_null_ifn (gimple_stmt_iterator *); extern bool ubsan_expand_objsize_ifn (gimple_stmt_iterator *); -extern tree ubsan_instrument_unreachable (location_t); +extern bool ubsan_instrument_unreachable (gimple_stmt_iterator *); extern tree ubsan_create_data (const char *, int, const location_t *, ...); extern tree ubsan_type_descriptor (tree, enum ubsan_print_style = UBSAN_PRINT_NORMAL); extern tree ubsan_encode_value (tree, bool = false); Marek