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 <mli...@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 <mli...@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 <mli...@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 <mli...@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 <mli...@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
