Sorry I let this slide. On Mon, Jul 14, 2014 at 01:54:13PM +0200, Jakub Jelinek wrote: > On Sun, Jul 13, 2014 at 07:55:44PM +0200, Marek Polacek wrote: > > 2014-07-13 Marek Polacek <pola...@redhat.com> > > > > * ubsan.h (struct ubsan_mismatch_data): > > Missing description.
Fixed. > > + gcc_assert (TREE_CODE (size) == INTEGER_CST); > > + /* See if we can discard the check. */ > > + if (tree_to_uhwi (size) == (unsigned HOST_WIDE_INT) -1) > > This would be integer_all_onesp (size). Fixed. > > +static void > > +instrument_object_size (gimple_stmt_iterator *gsi, bool is_lhs) > > +{ > > + gimple stmt = gsi_stmt (*gsi); > > + location_t loc = gimple_location (stmt); > > + tree t = is_lhs ? gimple_get_lhs (stmt) : gimple_assign_rhs1 (stmt); > > + > > + if (TREE_CODE (t) != MEM_REF) > > + return; > > I think this is undesirable. IMHO you want to call here > get_inner_reference, and if the given size is equal to maxsize, consider > instrumenting it, otherwise you don't instrument e.g. COMPONENT_REFs and > many other things. Look at what e.g. asan.c or even ubsan.c does; the > question is what exactly to do with bitfields, but supposedly we should > require that the DECL_BIT_FIELD_REPRESENTATIVE is accessible in that case. Adjusted. For bit-fields I just bail out; it's this check: || bitsize != size_in_bytes * BITS_PER_UNIT) > Also, I wonder if using base, ptr, objsz, ckind arguments are best for the > builtin, I'd think you want instead base, ptr+size-base, objsz, ckind. > Reasons: > a) the size addition when expanding UBSAN_OBJECT_SIZE will not work > reliably, the middle end considers all pointer conversions useless, > so you can very well end up with a different TREE_TYPE of the pointer > type > b) sanopt runs very late, there aren't many GIMPLE optimization passes, > so to optimize the condition checks you pretty much rely on RTL passes > c) for e.g. gimple_fold_call it will be much easier if it can remove > redundant UBSAN_OBJECT_SIZE calls if it can just compare two constants Ok, that's better. I rewrote this part. > > + tree ptr = TREE_OPERAND (t, 0); > > + tree sizet, base = ptr; > > + gimple g; > > + gimple def_stmt; > > + > > + while (TREE_CODE (base) == SSA_NAME) > > + { > > + def_stmt = SSA_NAME_DEF_STMT (base); > > + if (is_gimple_assign (def_stmt)) > > + base = gimple_assign_rhs1 (def_stmt); > > This looks too dangerous. All you should look through are: > a) gimple_assign_ssa_name_copy_p > b) gimple_assign_cast_p if the rhs1 also has POINTER_TYPE_P > c) gimple_assign_rhs_code == POINTER_PLUS_EXPR Fixed. > I'm also including a testcase, which shows why instrumenting > also COMPONENT_REFs etc. is important (see my reference to > get_inner_reference above) and also that IMHO we should instrument > not just when the base is a pointer, but also when it is a decl, Fixed, we now properly instrument the testcase you posted, because I've added instrumentation even of VAR_DECLs. > but in that case we should avoid instrumenting when -fsanitize=bounds > is on and we know it will handle it (in particular, if there was e.g. > char d[8]; int e; in the struct definition instead). I haven't fixed this, because it seems that when doing the object-size instrumentation I can't tell whether the array has been bounds-instrumented or not. So for some structs we can issue both =bounds and =object-size diagnostics. > Note, the testcase ICEs with -O2 -fsanitize=bounds, can you please look > at that first and fix it separately? Already fixed. > Other comments, in a form of a patch: > 1) the gimple_fold_call bit shows that we should for the quite common > case where __bos is folded into -1 remove the UBSAN_OBJECT_SIZE call > immediately, not worth keeping it around through many other passes Sure. > 2) if you add -O2 to the dg-options, that just means the tests are done > 8 times or how many with -O2 all the time. Better skip it unless > -O2 Ugh. > 3) when the second argument is something that can be directly compared > against the third argument, you can in gimple_fold_call fold not just > the "don't know" cases, but also when the third argument is >= the > second and both are INTEGER_CSTs - then we know at compile time > we are ok. Thanks, I applied the patch. And I've added some more optimizations to gimple_fold_call. So, how does this look now? Bootstrapped/regtested on x86_64-linux, passes even bootstrap-ubsan. 2014-09-11 Marek Polacek <pola...@redhat.com> * asan.c (pass_sanopt::execute): Handle IFN_UBSAN_OBJECT_SIZE. * doc/invoke.texi: Document -fsanitize=object-size. * flag-types.h (sanitize_code): Add SANITIZE_OBJECT_SIZE and or it into SANITIZE_UNDEFINED. * gimple-fold.c (gimple_fold_call): Optimize IFN_UBSAN_OBJECT_SIZE. * internal-fn.c (expand_UBSAN_OBJECT_SIZE): New function. * internal-fn.def (UBSAN_OBJECT_SIZE): Define. * opts.c (common_handle_option): Handle -fsanitize=object-size. * ubsan.c: Include "tree-object-size.h". (ubsan_type_descriptor): Call tree_to_uhwi instead of tree_to_shwi. (ubsan_expand_bounds_ifn): Use false instead of 0. (ubsan_expand_objsize_ifn): New function. (instrument_object_size): New function. (pass_ubsan::execute): Add object size instrumentation. * ubsan.h (ubsan_expand_objsize_ifn): Declare. testsuite/ * c-c++-common/ubsan/object-size-1.c: New test. * c-c++-common/ubsan/object-size-2.c: New test. * c-c++-common/ubsan/object-size-3.c: New test. * c-c++-common/ubsan/object-size-4.c: New test. * c-c++-common/ubsan/object-size-5.c: New test. * c-c++-common/ubsan/object-size-6.c: New test. * c-c++-common/ubsan/object-size-7.c: New test. * c-c++-common/ubsan/object-size-8.c: New test. * g++.dg/ubsan/object-size-1.C: New test. * gcc.dg/ubsan/object-size-9.c: New test. diff --git gcc/asan.c gcc/asan.c index cf5de27..44bc627 100644 --- gcc/asan.c +++ gcc/asan.c @@ -2822,6 +2822,9 @@ pass_sanopt::execute (function *fun) case IFN_UBSAN_BOUNDS: no_next = ubsan_expand_bounds_ifn (&gsi); break; + case IFN_UBSAN_OBJECT_SIZE: + no_next = ubsan_expand_objsize_ifn (&gsi); + break; case IFN_ASAN_CHECK: { no_next = asan_expand_check_ifn (&gsi, use_calls); diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi index 863b382..bb97b88 100644 --- gcc/doc/invoke.texi +++ gcc/doc/invoke.texi @@ -5569,6 +5569,12 @@ This option enables checking of alignment of pointers when they are dereferenced, or when a reference is bound to insufficiently aligned target, or when a method or constructor is invoked on insufficiently aligned object. +@item -fsanitize=object-size +@opindex fsanitize=object-size +This option enables instrumentation of memory references using the +@code{__builtin_object_size} function. Various out of bounds pointer +accesses are detected. + @item -fsanitize=float-divide-by-zero @opindex fsanitize=float-divide-by-zero Detect floating-point division by zero. Unlike other similar options, diff --git gcc/flag-types.h gcc/flag-types.h index d0818e5..3d01c49 100644 --- gcc/flag-types.h +++ gcc/flag-types.h @@ -236,12 +236,14 @@ enum sanitize_code { SANITIZE_ALIGNMENT = 1 << 17, SANITIZE_NONNULL_ATTRIBUTE = 1 << 18, SANITIZE_RETURNS_NONNULL_ATTRIBUTE = 1 << 19, + SANITIZE_OBJECT_SIZE = 1 << 20, SANITIZE_UNDEFINED = SANITIZE_SHIFT | SANITIZE_DIVIDE | SANITIZE_UNREACHABLE | SANITIZE_VLA | SANITIZE_NULL | SANITIZE_RETURN | SANITIZE_SI_OVERFLOW | SANITIZE_BOOL | SANITIZE_ENUM | SANITIZE_BOUNDS | SANITIZE_ALIGNMENT | SANITIZE_NONNULL_ATTRIBUTE - | SANITIZE_RETURNS_NONNULL_ATTRIBUTE, + | SANITIZE_RETURNS_NONNULL_ATTRIBUTE + | SANITIZE_OBJECT_SIZE, SANITIZE_NONDEFAULT = SANITIZE_FLOAT_DIVIDE | SANITIZE_FLOAT_CAST }; diff --git gcc/gimple-fold.c gcc/gimple-fold.c index 4aa1f4c..a6f6892 100644 --- gcc/gimple-fold.c +++ gcc/gimple-fold.c @@ -2661,6 +2661,19 @@ gimple_fold_call (gimple_stmt_iterator *gsi, bool inplace) gimple_call_arg (stmt, 1), gimple_call_arg (stmt, 2)); break; + case IFN_UBSAN_OBJECT_SIZE: + if (integer_all_onesp (gimple_call_arg (stmt, 2)) + || (TREE_CODE (gimple_call_arg (stmt, 1)) == INTEGER_CST + && TREE_CODE (gimple_call_arg (stmt, 2)) == INTEGER_CST + && tree_int_cst_le (gimple_call_arg (stmt, 1), + gimple_call_arg (stmt, 2)))) + { + gsi_replace (gsi, gimple_build_nop (), true); + unlink_stmt_vdef (stmt); + release_defs (stmt); + return true; + } + break; case IFN_UBSAN_CHECK_ADD: subcode = PLUS_EXPR; break; diff --git gcc/internal-fn.c gcc/internal-fn.c index c2a44b6..c71259d 100644 --- gcc/internal-fn.c +++ gcc/internal-fn.c @@ -184,6 +184,14 @@ expand_UBSAN_BOUNDS (gimple stmt ATTRIBUTE_UNUSED) /* This should get expanded in the sanopt pass. */ static void +expand_UBSAN_OBJECT_SIZE (gimple stmt ATTRIBUTE_UNUSED) +{ + gcc_unreachable (); +} + +/* This should get expanded in the sanopt pass. */ + +static void expand_ASAN_CHECK (gimple stmt ATTRIBUTE_UNUSED) { gcc_unreachable (); diff --git gcc/internal-fn.def gcc/internal-fn.def index 7ae60f3..fdb2d11 100644 --- gcc/internal-fn.def +++ gcc/internal-fn.def @@ -53,6 +53,7 @@ DEF_INTERNAL_FN (UBSAN_BOUNDS, ECF_LEAF | ECF_NOTHROW, NULL) DEF_INTERNAL_FN (UBSAN_CHECK_ADD, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL) DEF_INTERNAL_FN (UBSAN_CHECK_SUB, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL) DEF_INTERNAL_FN (UBSAN_CHECK_MUL, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL) +DEF_INTERNAL_FN (UBSAN_OBJECT_SIZE, ECF_LEAF | ECF_NOTHROW, NULL) DEF_INTERNAL_FN (ABNORMAL_DISPATCHER, ECF_NORETURN, NULL) DEF_INTERNAL_FN (BUILTIN_EXPECT, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL) DEF_INTERNAL_FN (ASAN_CHECK, ECF_TM_PURE | ECF_LEAF | ECF_NOTHROW, ".W..") diff --git gcc/opts.c gcc/opts.c index 0a49bc0..835f859 100644 --- gcc/opts.c +++ gcc/opts.c @@ -1504,6 +1504,8 @@ common_handle_option (struct gcc_options *opts, { "returns-nonnull-attribute", SANITIZE_RETURNS_NONNULL_ATTRIBUTE, sizeof "returns-nonnull-attribute" - 1 }, + { "object-size", SANITIZE_OBJECT_SIZE, + sizeof "object-size" - 1 }, { NULL, 0, 0 } }; const char *comma; diff --git gcc/testsuite/c-c++-common/ubsan/object-size-1.c gcc/testsuite/c-c++-common/ubsan/object-size-1.c index e69de29..7a3c87a 100644 --- gcc/testsuite/c-c++-common/ubsan/object-size-1.c +++ gcc/testsuite/c-c++-common/ubsan/object-size-1.c @@ -0,0 +1,125 @@ +/* { dg-do run } */ +/* { dg-skip-if "" { *-*-* } { "*" } { "-O2" } } */ +/* { dg-options "-fsanitize=undefined" } */ + +/* Sanity-test -fsanitize=object-size. We use -fsanitize=undefined option + to check that this feature doesn't clash with -fsanitize=bounds et al. */ + +#define N 20 + +__attribute__((noinline, noclone)) void +f1 (int i) +{ + volatile int j; + char *p, *orig; + orig = p = (char *) __builtin_calloc (N, 1); + j = *(p + i); + j = p[i]; + p++; + j = p[i - 1]; + j = *(p + i - 1); + __builtin_free (orig); +} + +/* { dg-output "load of address \[^\n\r]* with insufficient space for an object of type 'char'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'char'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'char'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'char'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ + +__attribute__((noinline, noclone)) void +f2 (int i) +{ + volatile int j; + char a[N]; + __builtin_memset (a, 0, N); + j = *(a + i); + char *p = a; + j = *(p + i); + j = p[i]; + p += 10; + j = *(p + i - 10); + j = p[i - 10]; +} + +/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'char'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'char'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'char'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'char'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ + +__attribute__((noinline, noclone)) void +f3 (int i) +{ + volatile int j; + int *p = (int *) __builtin_calloc (N, sizeof (*p)); + int *o = &p[i]; + j = *o; + j = o[0]; + __builtin_free (p); +} + +/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'int'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'int'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ + +__attribute__((noinline, noclone)) void +f4 (void) +{ + /* The second argument to __builtin_calloc is intentional. */ + int *p = (int *) __builtin_calloc (3, 1); + *p = 42; + __builtin_free (p); +} + +/* { dg-output "\[^\n\r]*store to address \[^\n\r]* with insufficient space for an object of type 'int'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ + +__attribute__((noinline, noclone)) void +f5 (int *p) +{ + /* This is not instrumented. But don't ICE, etc. */ + volatile int i = p[N]; +} + +int +main () +{ + f1 (N); + f2 (N); + f3 (N); + f4 (); + int *p = (int *) __builtin_calloc (N, sizeof (*p)); + f5 (p); + __builtin_free (p); + return 0; +} diff --git gcc/testsuite/c-c++-common/ubsan/object-size-2.c gcc/testsuite/c-c++-common/ubsan/object-size-2.c index e69de29..dba1243 100644 --- gcc/testsuite/c-c++-common/ubsan/object-size-2.c +++ gcc/testsuite/c-c++-common/ubsan/object-size-2.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-skip-if "" { *-*-* } { "*" } { "-O2" } } */ +/* { dg-options "-fsanitize=undefined" } */ + +void +foo (unsigned long ul) +{ + unsigned int u; + u = *(unsigned long *) ul; +} diff --git gcc/testsuite/c-c++-common/ubsan/object-size-3.c gcc/testsuite/c-c++-common/ubsan/object-size-3.c index e69de29..62dc76f 100644 --- gcc/testsuite/c-c++-common/ubsan/object-size-3.c +++ gcc/testsuite/c-c++-common/ubsan/object-size-3.c @@ -0,0 +1,56 @@ +/* { dg-do run } */ +/* { dg-skip-if "" { *-*-* } { "*" } { "-O2" } } */ +/* { dg-options "-fsanitize=object-size -fno-sanitize-recover" } */ + +/* Test valid uses. */ + +#define N 20 + +__attribute__((noinline, noclone)) void +f1 (int i) +{ + volatile int j; + char *p, *orig; + orig = p = (char *) __builtin_calloc (N, 1); + j = *(p + i); + j = p[i]; + p++; + j = p[i - 1]; + j = *(p + i - 1); + __builtin_free (orig); +} + +__attribute__((noinline, noclone)) void +f2 (int i) +{ + volatile int j; + char a[N]; + __builtin_memset (a, 0, N); + j = *(a + i); + char *p = a; + j = *(p + i); + j = p[i]; + p += 10; + j = *(p + i - 10); + j = p[i - 10]; +} + +__attribute__((noinline, noclone)) void +f3 (int i) +{ + volatile int j; + int *p = (int *) __builtin_calloc (N, sizeof (*p)); + int *o = &p[i]; + j = *o; + j = o[0]; + __builtin_free (p); +} + +int +main () +{ + f1 (N - 1); + f2 (N - 1); + f3 (N - 1); + return 0; +} diff --git gcc/testsuite/c-c++-common/ubsan/object-size-4.c gcc/testsuite/c-c++-common/ubsan/object-size-4.c index e69de29..8b95ec9 100644 --- gcc/testsuite/c-c++-common/ubsan/object-size-4.c +++ gcc/testsuite/c-c++-common/ubsan/object-size-4.c @@ -0,0 +1,31 @@ +/* { dg-do run } */ +/* { dg-skip-if "" { *-*-* } { "*" } { "-O2" } } */ +/* { dg-options "-fsanitize=object-size" } */ + +/* Test that we instrument flexible array members. */ + +struct T { int l; int a[]; }; +struct U { int l; int a[0]; }; + +int +main (void) +{ + volatile int i; + struct T *t = (struct T *) __builtin_calloc (sizeof (struct T) + + sizeof (int), 1); + i = t->a[1]; + + struct U *u = (struct U *) __builtin_calloc (sizeof (struct U) + + sizeof (int), 1); + i = u->a[1]; + return 0; +} + +/* { dg-output "load of address \[^\n\r]* with insufficient space for an object of type 'int'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'int'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ diff --git gcc/testsuite/c-c++-common/ubsan/object-size-5.c gcc/testsuite/c-c++-common/ubsan/object-size-5.c index e69de29..3dada10 100644 --- gcc/testsuite/c-c++-common/ubsan/object-size-5.c +++ gcc/testsuite/c-c++-common/ubsan/object-size-5.c @@ -0,0 +1,38 @@ +/* { dg-do run } */ +/* { dg-skip-if "" { *-*-* } { "*" } { "-O2" } } */ +/* { dg-options "-fsanitize=object-size" } */ + +/* Test structures with -fsanitize=object-size. */ + +#define N 20 + +struct S { char *p; int i; }; +struct T { struct S *s; }; + +__attribute__((noinline, noclone)) void +f1 (int i) +{ + volatile int j; + struct S s; + s.p = (char *) __builtin_calloc (N, 1); + j = s.p[i]; + j = *(s.p + i); + __builtin_free (s.p); +} + +/* { dg-output "load of address \[^\n\r]* with insufficient space for an object of type 'char'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'char'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ + +int +main () +{ + f1 (N); + f1 (N - 1); + return 0; +} diff --git gcc/testsuite/c-c++-common/ubsan/object-size-6.c gcc/testsuite/c-c++-common/ubsan/object-size-6.c index e69de29..0e6035d 100644 --- gcc/testsuite/c-c++-common/ubsan/object-size-6.c +++ gcc/testsuite/c-c++-common/ubsan/object-size-6.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-skip-if "" { *-*-* } { "*" } { "-O2" } } */ +/* { dg-options "-fsanitize=object-size" } */ + +char +foo (void *v) +{ + return *(char *) v; +} diff --git gcc/testsuite/c-c++-common/ubsan/object-size-7.c gcc/testsuite/c-c++-common/ubsan/object-size-7.c index e69de29..f5b26e5 100644 --- gcc/testsuite/c-c++-common/ubsan/object-size-7.c +++ gcc/testsuite/c-c++-common/ubsan/object-size-7.c @@ -0,0 +1,29 @@ +/* { dg-do run } */ +/* { dg-skip-if "" { *-*-* } { "*" } { "-O2" } } */ +/* { dg-options "-fsanitize=object-size" } */ + +#define N 20 + +struct S { int a; }; + +__attribute__((noinline, noclone)) struct S +f1 (int i) +{ + struct S a[N]; + struct S *p = a; + struct S s; + s = p[i]; + return s; +} + +int +main () +{ + f1 (N); + return 0; +} + +/* { dg-output "load of address \[^\n\r]* with insufficient space for an object of type\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ diff --git gcc/testsuite/c-c++-common/ubsan/object-size-8.c gcc/testsuite/c-c++-common/ubsan/object-size-8.c index e69de29..ee0945b 100644 --- gcc/testsuite/c-c++-common/ubsan/object-size-8.c +++ gcc/testsuite/c-c++-common/ubsan/object-size-8.c @@ -0,0 +1,32 @@ +/* { dg-do run } */ +/* { dg-skip-if "" { *-*-* } { "*" } { "-O2" } } */ +/* { dg-options "-fsanitize=undefined" } */ + +struct S { int a; int b; }; + +static inline __attribute__((always_inline)) int +foo (struct S *p) +{ + volatile int a; + a = p->a; /* OK */ + return p->b; +} + +int +bar (void) +{ + struct S *p = (struct S *) __builtin_calloc (sizeof (int) + sizeof (int) / 2, 1); + return foo (p); +} + +int +main (void) +{ + bar (); + return 0; +} + +/* { dg-output "load of address \[^\n\r]* with insufficient space for an object of type 'int'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ diff --git gcc/testsuite/g++.dg/ubsan/object-size-1.C gcc/testsuite/g++.dg/ubsan/object-size-1.C index e69de29..e2aad46 100644 --- gcc/testsuite/g++.dg/ubsan/object-size-1.C +++ gcc/testsuite/g++.dg/ubsan/object-size-1.C @@ -0,0 +1,18 @@ +// { dg-do compile } +// { dg-options "-fsanitize=undefined -fpermissive" } + +struct T { int c; char d[]; }; + +struct T t = { 1, "a" }; // { dg-warning "initializer-string for array of chars is too long" } + +int +baz (int i) +{ + return t.d[i]; +} + +int +main (void) +{ + baz (3); +} diff --git gcc/testsuite/gcc.dg/ubsan/object-size-9.c gcc/testsuite/gcc.dg/ubsan/object-size-9.c index e69de29..bb9aa1b 100644 --- gcc/testsuite/gcc.dg/ubsan/object-size-9.c +++ gcc/testsuite/gcc.dg/ubsan/object-size-9.c @@ -0,0 +1,24 @@ +/* { dg-do run } */ +/* { dg-skip-if "" { *-*-* } { "*" } { "-O2" } } */ +/* { dg-options "-fsanitize=undefined" } */ + +struct T { int c; char d[]; }; +struct T t = { 1, "a" }; + +int +baz (int i) +{ + return t.d[i]; +} + +int +main (void) +{ + baz (2); + return 0; +} + +/* { dg-output "load of address \[^\n\r]* with insufficient space for an object of type 'char'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ diff --git gcc/ubsan.c gcc/ubsan.c index e3128ad..739a6e7 100644 --- gcc/ubsan.c +++ gcc/ubsan.c @@ -50,6 +50,7 @@ along with GCC; see the file COPYING3. If not see #include "realmpfr.h" #include "dfp.h" #include "builtins.h" +#include "tree-object-size.h" /* Map from a tree to a VAR_DECL tree. */ @@ -384,7 +385,7 @@ ubsan_type_descriptor (tree type, enum ubsan_print_style pstyle) tree dom = TYPE_DOMAIN (t); if (dom && TREE_CODE (TYPE_MAX_VALUE (dom)) == INTEGER_CST) pos += sprintf (&pretty_name[pos], HOST_WIDE_INT_PRINT_DEC, - tree_to_shwi (TYPE_MAX_VALUE (dom)) + 1); + tree_to_uhwi (TYPE_MAX_VALUE (dom)) + 1); else /* ??? We can't determine the variable name; print VLA unspec. */ pretty_name[pos++] = '*'; @@ -607,12 +608,12 @@ ubsan_expand_bounds_ifn (gimple_stmt_iterator *gsi) /* Create condition "if (index > bound)". */ basic_block then_bb, fallthru_bb; gimple_stmt_iterator cond_insert_point - = create_cond_insert_point (gsi, 0/*before_p*/, false, true, + = create_cond_insert_point (gsi, false, false, true, &then_bb, &fallthru_bb); index = fold_convert (TREE_TYPE (bound), index); index = force_gimple_operand_gsi (&cond_insert_point, index, - true/*simple_p*/, NULL_TREE, - false/*before*/, GSI_NEW_STMT); + true, NULL_TREE, + false, GSI_NEW_STMT); gimple g = gimple_build_cond (GT_EXPR, index, bound, NULL_TREE, NULL_TREE); gimple_set_location (g, loc); gsi_insert_after (&cond_insert_point, g, GSI_NEW_STMT); @@ -823,6 +824,76 @@ ubsan_expand_null_ifn (gimple_stmt_iterator *gsip) return false; } +/* Expand UBSAN_OBJECT_SIZE internal call. */ + +bool +ubsan_expand_objsize_ifn (gimple_stmt_iterator *gsi) +{ + gimple stmt = gsi_stmt (*gsi); + location_t loc = gimple_location (stmt); + gcc_assert (gimple_call_num_args (stmt) == 4); + + tree ptr = gimple_call_arg (stmt, 0); + tree offset = gimple_call_arg (stmt, 1); + tree size = gimple_call_arg (stmt, 2); + tree ckind = gimple_call_arg (stmt, 3); + gimple_stmt_iterator gsi_orig = *gsi; + gimple g; + + gcc_assert (TREE_CODE (size) == INTEGER_CST); + /* See if we can discard the check. */ + if (integer_all_onesp (size)) + /* Yes, __builtin_object_size couldn't determine the + object size. */; + else + { + /* if (offset > objsize) */ + basic_block then_bb, fallthru_bb; + gimple_stmt_iterator cond_insert_point + = create_cond_insert_point (gsi, false, false, true, + &then_bb, &fallthru_bb); + g = gimple_build_cond (GT_EXPR, offset, size, NULL_TREE, NULL_TREE); + gimple_set_location (g, loc); + gsi_insert_after (&cond_insert_point, g, GSI_NEW_STMT); + + /* Generate __ubsan_handle_type_mismatch call. */ + *gsi = gsi_after_labels (then_bb); + 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_objsz_data", 1, &loc, + ubsan_type_descriptor (TREE_TYPE (ptr), + UBSAN_PRINT_POINTER), + NULL_TREE, + build_zero_cst (pointer_sized_int_node), + ckind, + NULL_TREE); + data = build_fold_addr_expr_loc (loc, data); + enum built_in_function bcode + = flag_sanitize_recover + ? BUILT_IN_UBSAN_HANDLE_TYPE_MISMATCH + : BUILT_IN_UBSAN_HANDLE_TYPE_MISMATCH_ABORT; + tree p = make_ssa_name (pointer_sized_int_node, NULL); + g = gimple_build_assign_with_ops (NOP_EXPR, p, ptr, NULL_TREE); + gimple_set_location (g, loc); + gsi_insert_before (gsi, g, GSI_SAME_STMT); + g = gimple_build_call (builtin_decl_explicit (bcode), 2, data, p); + } + gimple_set_location (g, loc); + gsi_insert_before (gsi, g, GSI_SAME_STMT); + + /* Point GSI to next logical statement. */ + *gsi = gsi_start_bb (fallthru_bb); + } + + /* Get rid of the UBSAN_OBJECT_SIZE call from the IR. */ + unlink_stmt_vdef (stmt); + gsi_remove (&gsi_orig, true); + return gsi_end_p (*gsi); +} + /* Instrument a memory reference. BASE is the base of MEM, IS_LHS says whether the pointer is on the left hand side of the assignment. */ @@ -1332,6 +1403,118 @@ instrument_nonnull_return (gimple_stmt_iterator *gsi) flag_delete_null_pointer_checks = save_flag_delete_null_pointer_checks; } +/* Instrument memory references. Here we check whether the pointer + points to an out-of-bounds location. */ + +static void +instrument_object_size (gimple_stmt_iterator *gsi, bool is_lhs) +{ + gimple stmt = gsi_stmt (*gsi); + location_t loc = gimple_location (stmt); + tree t = is_lhs ? gimple_get_lhs (stmt) : gimple_assign_rhs1 (stmt); + tree type; + HOST_WIDE_INT size_in_bytes; + + type = TREE_TYPE (t); + if (VOID_TYPE_P (type)) + return; + + switch (TREE_CODE (t)) + { + case ARRAY_REF: + case COMPONENT_REF: + case INDIRECT_REF: + case MEM_REF: + case VAR_DECL: + break; + default: + return; + } + + size_in_bytes = int_size_in_bytes (type); + if (size_in_bytes <= 0) + return; + + HOST_WIDE_INT bitsize, bitpos; + tree offset; + enum machine_mode mode; + int volatilep = 0, unsignedp = 0; + tree inner = get_inner_reference (t, &bitsize, &bitpos, &offset, &mode, + &unsignedp, &volatilep, false); + + if (bitpos % BITS_PER_UNIT != 0 + || bitsize != size_in_bytes * BITS_PER_UNIT) + return; + + bool decl_p = VAR_P (inner) || TREE_CODE (inner) == PARM_DECL + || TREE_CODE (inner) == RESULT_DECL; + tree base = decl_p ? inner : TREE_OPERAND (inner, 0); + tree ptr = build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (t)), t); + + while (TREE_CODE (base) == SSA_NAME) + { + gimple def_stmt = SSA_NAME_DEF_STMT (base); + if (gimple_assign_ssa_name_copy_p (def_stmt) + || (gimple_assign_cast_p (def_stmt) + && POINTER_TYPE_P (TREE_TYPE (gimple_assign_rhs1 (def_stmt)))) + || (is_gimple_assign (def_stmt) + && gimple_assign_rhs_code (def_stmt) == POINTER_PLUS_EXPR)) + base = gimple_assign_rhs1 (def_stmt); + else + break; + } + + if (!POINTER_TYPE_P (TREE_TYPE (base)) && !VAR_P (base)) + return; + + tree sizet; + tree base_addr = build1 (ADDR_EXPR, + build_pointer_type (TREE_TYPE (base)), base); + unsigned HOST_WIDE_INT size = compute_builtin_object_size (decl_p ? base_addr + : base, 0); + if (size != (unsigned HOST_WIDE_INT) -1) + sizet = build_int_cst (sizetype, size); + else if (optimize) + { + /* Generate __builtin_object_size call. */ + sizet = builtin_decl_explicit (BUILT_IN_OBJECT_SIZE); + + sizet = build_call_expr_loc (loc, sizet, 2, decl_p ? base_addr : base, + integer_zero_node); + sizet = force_gimple_operand_gsi (gsi, sizet, false, NULL_TREE, true, + GSI_SAME_STMT); + } + else + return; + + /* Generate UBSAN_OBJECT_SIZE (ptr, ptr+sizeof(*ptr)-base, objsize, ckind) + call. */ + /* ptr + sizeof (*ptr) - base */ + t = fold_build2 (MINUS_EXPR, sizetype, + fold_convert (pointer_sized_int_node, ptr), + fold_convert (pointer_sized_int_node, + decl_p ? base_addr : base)); + t = fold_build2 (PLUS_EXPR, sizetype, t, TYPE_SIZE_UNIT (type)); + + /* Perhaps we can omit the check. */ + if (TREE_CODE (t) == INTEGER_CST + && TREE_CODE (sizet) == INTEGER_CST + && tree_int_cst_le (t, sizet)) + return; + + /* Nope. Emit the check. */ + t = force_gimple_operand_gsi (gsi, t, true, NULL_TREE, true, + GSI_SAME_STMT); + ptr = force_gimple_operand_gsi (gsi, ptr, true, NULL_TREE, true, + GSI_SAME_STMT); + tree ckind = build_int_cst (unsigned_char_type_node, + is_lhs ? UBSAN_STORE_OF : UBSAN_LOAD_OF); + gimple g = gimple_build_call_internal (IFN_UBSAN_OBJECT_SIZE, 4, + ptr, t, sizet, ckind); + gimple_set_location (g, loc); + gsi_insert_before (gsi, g, GSI_SAME_STMT); +} + namespace { const pass_data pass_data_ubsan = @@ -1361,7 +1544,8 @@ public: | SANITIZE_BOOL | SANITIZE_ENUM | SANITIZE_ALIGNMENT | SANITIZE_NONNULL_ATTRIBUTE - | SANITIZE_RETURNS_NONNULL_ATTRIBUTE) + | SANITIZE_RETURNS_NONNULL_ATTRIBUTE + | SANITIZE_OBJECT_SIZE) && current_function_decl != NULL_TREE && !lookup_attribute ("no_sanitize_undefined", DECL_ATTRIBUTES (current_function_decl)); @@ -1424,6 +1608,14 @@ pass_ubsan::execute (function *fun) bb = gimple_bb (stmt); } + if (flag_sanitize & SANITIZE_OBJECT_SIZE) + { + if (gimple_store_p (stmt)) + instrument_object_size (&gsi, true); + if (gimple_assign_load_p (stmt)) + instrument_object_size (&gsi, false); + } + gsi_next (&gsi); } } diff --git gcc/ubsan.h gcc/ubsan.h index cd26940..2a261d8 100644 --- gcc/ubsan.h +++ gcc/ubsan.h @@ -40,6 +40,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 tree ubsan_create_data (const char *, int, const location_t *, ...); extern tree ubsan_type_descriptor (tree, enum ubsan_print_style = UBSAN_PRINT_NORMAL); Marek