| Message ID | 03b94c1a-f6a2-93b8-90b2-b6a79aa1d49c@suse.cz |
|---|---|
| State | New |
| Headers | show |
On Fri, Jul 28, 2017 at 01:20:57PM +0200, Martin Liška wrote: > Hi. > > In r249158 I added new sanitize_flags_p function. However I removed various calls of > do_ubsan_in_current_function: > > /* True if we want to play UBSan games in the current function. */ > > bool > do_ubsan_in_current_function () > { > return (current_function_decl != NULL_TREE > && !lookup_attribute ("no_sanitize_undefined", > DECL_ATTRIBUTES (current_function_decl))); > } > > Where we also checked for current_function_decl. This putting that condition back to conditions > that used to call do_ubsan_in_current_function is necessary. > > May I ask you Marek (or Jakub) to go through and verify that the check is really necessary? > > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > Ready to be installed? > Martin > > gcc/cp/ChangeLog: > > 2017-07-28 Martin Liska <mliska@suse.cz> > > PR sanitize/81530 > * cp-gimplify.c (cp_genericize): Guard condition with flag_sanitize_p > also with current_function_decl non-null equality. > * cp-ubsan.c (cp_ubsan_instrument_vptr_p): Likewise. > * decl.c (compute_array_index_type): Likewise. > * init.c (finish_length_check): Likewise. > * typeck.c (cp_build_binary_op): Likewise. > > gcc/c/ChangeLog: > > 2017-07-28 Martin Liska <mliska@suse.cz> > > PR sanitize/81530 > * c-convert.c (convert): Guard condition with flag_sanitize_p > also with current_function_decl non-null equality. > * c-decl.c (grokdeclarator): Likewise. > * c-typeck.c (build_binary_op): Likewise. > > gcc/ChangeLog: > > 2017-07-28 Martin Liska <mliska@suse.cz> > > PR sanitize/81530 > * convert.c (convert_to_integer_1): Guard condition with flag_sanitize_p > also with current_function_decl non-null equality. > > gcc/c-family/ChangeLog: > > 2017-07-28 Martin Liska <mliska@suse.cz> > > PR sanitize/81530 > * c-ubsan.c (ubsan_maybe_instrument_array_ref): > Guard condition with flag_sanitize_p also with current_function_decl > non-null equality. > (ubsan_maybe_instrument_reference_or_call): Likewise. > > gcc/testsuite/ChangeLog: > > 2017-07-28 Martin Liska <mliska@suse.cz> > > PR sanitize/81530 > * g++.dg/ubsan/pr81530.C: New test. > --- > gcc/c-family/c-ubsan.c | 6 ++++-- > gcc/c/c-convert.c | 1 + > gcc/c/c-decl.c | 1 + > gcc/c/c-typeck.c | 1 + > gcc/convert.c | 3 ++- > gcc/cp/cp-gimplify.c | 3 ++- > gcc/cp/cp-ubsan.c | 3 +++ > gcc/cp/decl.c | 3 ++- > gcc/cp/init.c | 3 ++- > gcc/cp/typeck.c | 1 + > gcc/testsuite/g++.dg/ubsan/pr81530.C | 6 ++++++ > 11 files changed, 25 insertions(+), 6 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/ubsan/pr81530.C > > > diff --git a/gcc/c-family/c-ubsan.c b/gcc/c-family/c-ubsan.c > index a072d19eda6..541b53009c2 100644 > --- a/gcc/c-family/c-ubsan.c > +++ b/gcc/c-family/c-ubsan.c > @@ -373,7 +373,8 @@ void > ubsan_maybe_instrument_array_ref (tree *expr_p, bool ignore_off_by_one) > { > if (!ubsan_array_ref_instrumented_p (*expr_p) > - && sanitize_flags_p (SANITIZE_BOUNDS | SANITIZE_BOUNDS_STRICT)) > + && sanitize_flags_p (SANITIZE_BOUNDS | SANITIZE_BOUNDS_STRICT) > + && current_function_decl != NULL_TREE) > { > tree op0 = TREE_OPERAND (*expr_p, 0); > tree op1 = TREE_OPERAND (*expr_p, 1); > @@ -393,7 +394,8 @@ static tree > ubsan_maybe_instrument_reference_or_call (location_t loc, tree op, tree ptype, > enum ubsan_null_ckind ckind) > { > - if (!sanitize_flags_p (SANITIZE_ALIGNMENT | SANITIZE_NULL)) > + if (!sanitize_flags_p (SANITIZE_ALIGNMENT | SANITIZE_NULL) > + || current_function_decl == NULL_TREE) > return NULL_TREE; > > tree type = TREE_TYPE (ptype); > diff --git a/gcc/c/c-convert.c b/gcc/c/c-convert.c > index 33c9143e354..07862543334 100644 > --- a/gcc/c/c-convert.c > +++ b/gcc/c/c-convert.c > @@ -108,6 +108,7 @@ convert (tree type, tree expr) > case INTEGER_TYPE: > case ENUMERAL_TYPE: > if (sanitize_flags_p (SANITIZE_FLOAT_CAST) > + && current_function_decl != NULL Should be NULL_TREE. It's non-obvious to prove that some checks might not be necessary, but I verifed that gcc7 had do_ubsan_in_current_function where you're adding the current_function_decl checks, so I think this should be good to go. Marek
On 07/31/2017 10:27 AM, Marek Polacek wrote: > On Fri, Jul 28, 2017 at 01:20:57PM +0200, Martin Liška wrote: >> Hi. >> >> In r249158 I added new sanitize_flags_p function. However I removed various calls of >> do_ubsan_in_current_function: >> >> /* True if we want to play UBSan games in the current function. */ >> >> bool >> do_ubsan_in_current_function () >> { >> return (current_function_decl != NULL_TREE >> && !lookup_attribute ("no_sanitize_undefined", >> DECL_ATTRIBUTES (current_function_decl))); >> } >> >> Where we also checked for current_function_decl. This putting that condition back to conditions >> that used to call do_ubsan_in_current_function is necessary. >> >> May I ask you Marek (or Jakub) to go through and verify that the check is really necessary? >> >> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >> >> Ready to be installed? >> Martin >> >> gcc/cp/ChangeLog: >> >> 2017-07-28 Martin Liska <mliska@suse.cz> >> >> PR sanitize/81530 >> * cp-gimplify.c (cp_genericize): Guard condition with flag_sanitize_p >> also with current_function_decl non-null equality. >> * cp-ubsan.c (cp_ubsan_instrument_vptr_p): Likewise. >> * decl.c (compute_array_index_type): Likewise. >> * init.c (finish_length_check): Likewise. >> * typeck.c (cp_build_binary_op): Likewise. >> >> gcc/c/ChangeLog: >> >> 2017-07-28 Martin Liska <mliska@suse.cz> >> >> PR sanitize/81530 >> * c-convert.c (convert): Guard condition with flag_sanitize_p >> also with current_function_decl non-null equality. >> * c-decl.c (grokdeclarator): Likewise. >> * c-typeck.c (build_binary_op): Likewise. >> >> gcc/ChangeLog: >> >> 2017-07-28 Martin Liska <mliska@suse.cz> >> >> PR sanitize/81530 >> * convert.c (convert_to_integer_1): Guard condition with flag_sanitize_p >> also with current_function_decl non-null equality. >> >> gcc/c-family/ChangeLog: >> >> 2017-07-28 Martin Liska <mliska@suse.cz> >> >> PR sanitize/81530 >> * c-ubsan.c (ubsan_maybe_instrument_array_ref): >> Guard condition with flag_sanitize_p also with current_function_decl >> non-null equality. >> (ubsan_maybe_instrument_reference_or_call): Likewise. >> >> gcc/testsuite/ChangeLog: >> >> 2017-07-28 Martin Liska <mliska@suse.cz> >> >> PR sanitize/81530 >> * g++.dg/ubsan/pr81530.C: New test. >> --- >> gcc/c-family/c-ubsan.c | 6 ++++-- >> gcc/c/c-convert.c | 1 + >> gcc/c/c-decl.c | 1 + >> gcc/c/c-typeck.c | 1 + >> gcc/convert.c | 3 ++- >> gcc/cp/cp-gimplify.c | 3 ++- >> gcc/cp/cp-ubsan.c | 3 +++ >> gcc/cp/decl.c | 3 ++- >> gcc/cp/init.c | 3 ++- >> gcc/cp/typeck.c | 1 + >> gcc/testsuite/g++.dg/ubsan/pr81530.C | 6 ++++++ >> 11 files changed, 25 insertions(+), 6 deletions(-) >> create mode 100644 gcc/testsuite/g++.dg/ubsan/pr81530.C >> >> > >> diff --git a/gcc/c-family/c-ubsan.c b/gcc/c-family/c-ubsan.c >> index a072d19eda6..541b53009c2 100644 >> --- a/gcc/c-family/c-ubsan.c >> +++ b/gcc/c-family/c-ubsan.c >> @@ -373,7 +373,8 @@ void >> ubsan_maybe_instrument_array_ref (tree *expr_p, bool ignore_off_by_one) >> { >> if (!ubsan_array_ref_instrumented_p (*expr_p) >> - && sanitize_flags_p (SANITIZE_BOUNDS | SANITIZE_BOUNDS_STRICT)) >> + && sanitize_flags_p (SANITIZE_BOUNDS | SANITIZE_BOUNDS_STRICT) >> + && current_function_decl != NULL_TREE) >> { >> tree op0 = TREE_OPERAND (*expr_p, 0); >> tree op1 = TREE_OPERAND (*expr_p, 1); >> @@ -393,7 +394,8 @@ static tree >> ubsan_maybe_instrument_reference_or_call (location_t loc, tree op, tree ptype, >> enum ubsan_null_ckind ckind) >> { >> - if (!sanitize_flags_p (SANITIZE_ALIGNMENT | SANITIZE_NULL)) >> + if (!sanitize_flags_p (SANITIZE_ALIGNMENT | SANITIZE_NULL) >> + || current_function_decl == NULL_TREE) >> return NULL_TREE; >> >> tree type = TREE_TYPE (ptype); >> diff --git a/gcc/c/c-convert.c b/gcc/c/c-convert.c >> index 33c9143e354..07862543334 100644 >> --- a/gcc/c/c-convert.c >> +++ b/gcc/c/c-convert.c >> @@ -108,6 +108,7 @@ convert (tree type, tree expr) >> case INTEGER_TYPE: >> case ENUMERAL_TYPE: >> if (sanitize_flags_p (SANITIZE_FLOAT_CAST) >> + && current_function_decl != NULL > > Should be NULL_TREE. Yes, fixed. > > It's non-obvious to prove that some checks might not be necessary, but I > verifed that gcc7 had do_ubsan_in_current_function where you're adding > the current_function_decl checks, so I think this should be good to go. > > Marek > Good, I installed the patch as r250730. Martin
diff --git a/gcc/c-family/c-ubsan.c b/gcc/c-family/c-ubsan.c index a072d19eda6..541b53009c2 100644 --- a/gcc/c-family/c-ubsan.c +++ b/gcc/c-family/c-ubsan.c @@ -373,7 +373,8 @@ void ubsan_maybe_instrument_array_ref (tree *expr_p, bool ignore_off_by_one) { if (!ubsan_array_ref_instrumented_p (*expr_p) - && sanitize_flags_p (SANITIZE_BOUNDS | SANITIZE_BOUNDS_STRICT)) + && sanitize_flags_p (SANITIZE_BOUNDS | SANITIZE_BOUNDS_STRICT) + && current_function_decl != NULL_TREE) { tree op0 = TREE_OPERAND (*expr_p, 0); tree op1 = TREE_OPERAND (*expr_p, 1); @@ -393,7 +394,8 @@ static tree ubsan_maybe_instrument_reference_or_call (location_t loc, tree op, tree ptype, enum ubsan_null_ckind ckind) { - if (!sanitize_flags_p (SANITIZE_ALIGNMENT | SANITIZE_NULL)) + if (!sanitize_flags_p (SANITIZE_ALIGNMENT | SANITIZE_NULL) + || current_function_decl == NULL_TREE) return NULL_TREE; tree type = TREE_TYPE (ptype); diff --git a/gcc/c/c-convert.c b/gcc/c/c-convert.c index 33c9143e354..07862543334 100644 --- a/gcc/c/c-convert.c +++ b/gcc/c/c-convert.c @@ -108,6 +108,7 @@ convert (tree type, tree expr) case INTEGER_TYPE: case ENUMERAL_TYPE: if (sanitize_flags_p (SANITIZE_FLOAT_CAST) + && current_function_decl != NULL && TREE_CODE (TREE_TYPE (expr)) == REAL_TYPE && COMPLETE_TYPE_P (type)) { diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c index 12fbc18bb94..a54e1218434 100644 --- a/gcc/c/c-decl.c +++ b/gcc/c/c-decl.c @@ -6052,6 +6052,7 @@ grokdeclarator (const struct c_declarator *declarator, this_size_varies = size_varies = true; warn_variable_length_array (name, size); if (sanitize_flags_p (SANITIZE_VLA) + && current_function_decl != NULL_TREE && decl_context == NORMAL) { /* Evaluate the array size only once. */ diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index 4d067e96dd3..7451f3249fd 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -11838,6 +11838,7 @@ build_binary_op (location_t location, enum tree_code code, if (sanitize_flags_p ((SANITIZE_SHIFT | SANITIZE_DIVIDE | SANITIZE_FLOAT_DIVIDE)) + && current_function_decl != NULL_TREE && (doing_div_or_mod || doing_shift) && !require_constant_value) { diff --git a/gcc/convert.c b/gcc/convert.c index 429f988cbde..58d8054a724 100644 --- a/gcc/convert.c +++ b/gcc/convert.c @@ -938,7 +938,8 @@ convert_to_integer_1 (tree type, tree expr, bool dofold) return build1 (CONVERT_EXPR, type, expr); case REAL_TYPE: - if (sanitize_flags_p (SANITIZE_FLOAT_CAST)) + if (sanitize_flags_p (SANITIZE_FLOAT_CAST) + && current_function_decl != NULL_TREE) { expr = save_expr (expr); tree check = ubsan_instrument_float_cast (loc, type, expr); diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c index f010f6c63be..a9563b1a8cd 100644 --- a/gcc/cp/cp-gimplify.c +++ b/gcc/cp/cp-gimplify.c @@ -1668,7 +1668,8 @@ cp_genericize (tree fndecl) walk_tree's hash functionality. */ cp_genericize_tree (&DECL_SAVED_TREE (fndecl), true); - if (sanitize_flags_p (SANITIZE_RETURN)) + if (sanitize_flags_p (SANITIZE_RETURN) + && current_function_decl != NULL_TREE) cp_ubsan_maybe_instrument_return (fndecl); /* Do everything else. */ diff --git a/gcc/cp/cp-ubsan.c b/gcc/cp/cp-ubsan.c index f00f870bd3e..3be607c0a42 100644 --- a/gcc/cp/cp-ubsan.c +++ b/gcc/cp/cp-ubsan.c @@ -36,6 +36,9 @@ cp_ubsan_instrument_vptr_p (tree type) if (!sanitize_flags_p (SANITIZE_VPTR)) return false; + if (current_function_decl == NULL_TREE) + return false; + if (type) { type = TYPE_MAIN_VARIANT (type); diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index d98fab370d7..4ec38b82aa9 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -9482,7 +9482,8 @@ compute_array_index_type (tree name, tree size, tsubst_flags_t complain) stabilize_vla_size (itype); - if (sanitize_flags_p (SANITIZE_VLA)) + if (sanitize_flags_p (SANITIZE_VLA) + && current_function_decl != NULL_TREE) { /* We have to add 1 -- in the ubsan routine we generate LE_EXPR rather than LT_EXPR. */ diff --git a/gcc/cp/init.c b/gcc/cp/init.c index 14335388a50..3fe8f18b2a9 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -3910,7 +3910,8 @@ finish_length_check (tree atype, tree iterator, tree obase, unsigned n) } /* Don't check an array new when -fno-exceptions. */ } - else if (sanitize_flags_p (SANITIZE_BOUNDS)) + else if (sanitize_flags_p (SANITIZE_BOUNDS) + && current_function_decl != NULL_TREE) { /* Make sure the last element of the initializer is in bounds. */ finish_expr_stmt diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index 316d57fb38c..3dc64045e1a 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -5256,6 +5256,7 @@ cp_build_binary_op (location_t location, if (sanitize_flags_p ((SANITIZE_SHIFT | SANITIZE_DIVIDE | SANITIZE_FLOAT_DIVIDE)) + && current_function_decl != NULL_TREE && !processing_template_decl && (doing_div_or_mod || doing_shift)) { diff --git a/gcc/testsuite/g++.dg/ubsan/pr81530.C b/gcc/testsuite/g++.dg/ubsan/pr81530.C new file mode 100644 index 00000000000..e21724686c4 --- /dev/null +++ b/gcc/testsuite/g++.dg/ubsan/pr81530.C @@ -0,0 +1,6 @@ +/* PR sanitizer/81530 */ +/* { dg-do compile } */ +/* { dg-options "-fsanitize=undefined" } */ + +int a[(long) 4e20]; /* { dg-error "overflow in constant expression" } */ +/* { dg-error "size of array .a. is too large" "" { target *-*-* } .-1 } */
Hi. In r249158 I added new sanitize_flags_p function. However I removed various calls of do_ubsan_in_current_function: /* True if we want to play UBSan games in the current function. */ bool do_ubsan_in_current_function () { return (current_function_decl != NULL_TREE && !lookup_attribute ("no_sanitize_undefined", DECL_ATTRIBUTES (current_function_decl))); } Where we also checked for current_function_decl. This putting that condition back to conditions that used to call do_ubsan_in_current_function is necessary. May I ask you Marek (or Jakub) to go through and verify that the check is really necessary? Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. Ready to be installed? Martin gcc/cp/ChangeLog: 2017-07-28 Martin Liska <mliska@suse.cz> PR sanitize/81530 * cp-gimplify.c (cp_genericize): Guard condition with flag_sanitize_p also with current_function_decl non-null equality. * cp-ubsan.c (cp_ubsan_instrument_vptr_p): Likewise. * decl.c (compute_array_index_type): Likewise. * init.c (finish_length_check): Likewise. * typeck.c (cp_build_binary_op): Likewise. gcc/c/ChangeLog: 2017-07-28 Martin Liska <mliska@suse.cz> PR sanitize/81530 * c-convert.c (convert): Guard condition with flag_sanitize_p also with current_function_decl non-null equality. * c-decl.c (grokdeclarator): Likewise. * c-typeck.c (build_binary_op): Likewise. gcc/ChangeLog: 2017-07-28 Martin Liska <mliska@suse.cz> PR sanitize/81530 * convert.c (convert_to_integer_1): Guard condition with flag_sanitize_p also with current_function_decl non-null equality. gcc/c-family/ChangeLog: 2017-07-28 Martin Liska <mliska@suse.cz> PR sanitize/81530 * c-ubsan.c (ubsan_maybe_instrument_array_ref): Guard condition with flag_sanitize_p also with current_function_decl non-null equality. (ubsan_maybe_instrument_reference_or_call): Likewise. gcc/testsuite/ChangeLog: 2017-07-28 Martin Liska <mliska@suse.cz> PR sanitize/81530 * g++.dg/ubsan/pr81530.C: New test. --- gcc/c-family/c-ubsan.c | 6 ++++-- gcc/c/c-convert.c | 1 + gcc/c/c-decl.c | 1 + gcc/c/c-typeck.c | 1 + gcc/convert.c | 3 ++- gcc/cp/cp-gimplify.c | 3 ++- gcc/cp/cp-ubsan.c | 3 +++ gcc/cp/decl.c | 3 ++- gcc/cp/init.c | 3 ++- gcc/cp/typeck.c | 1 + gcc/testsuite/g++.dg/ubsan/pr81530.C | 6 ++++++ 11 files changed, 25 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ubsan/pr81530.C