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