[PATCH] Do UBSAN sanitization just when current_function_decl != NULL_TREE (PR sanitize/81530).

[Marek Polacek]

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