This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][Middle-end]79538 missing -Wformat-overflow with %s and non-member array arguments


On Wed, 13 Dec 2017, Qing Zhao wrote:

> Hi,
> 
> I updated gimple-fold.c as you suggested, bootstrapped and re-tested on both x86 and aarch64. no any issue.
> 
> ====
> diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
> index 353a46e..eb6a87a 100644
> --- a/gcc/gimple-fold.c
> +++ b/gcc/gimple-fold.c
> @@ -1323,6 +1323,19 @@ get_range_strlen (tree arg, tree length[2], bitmap *visited, int type,
>  		 the array could have zero length.  */
>  	      *minlen = ssize_int (0);
>  	    }
> +
> +	  if (VAR_P (arg) 
> +	      && TREE_CODE (TREE_TYPE (arg)) == ARRAY_TYPE)
> +	    {
> +	      val = TYPE_SIZE_UNIT (TREE_TYPE (arg));
> +	      if (!val || TREE_CODE (val) != INTEGER_CST || integer_zerop (val))
> +		return false;
> +	      val = wide_int_to_tree (TREE_TYPE (val), 
> +				      wi::sub(wi::to_wide (val), 1));
> +	      /* Set the minimum size to zero since the string in
> +		 the array could have zero length.  */
> +	      *minlen = ssize_int (0);
> +	    }
>  	}
> ====
> 
> I plan to commit the change very soon. 
> let me know if you have further comment.

Looks good to me.

Richard.

> thanks.
> 
> Qing
> 
> ==========
> 
> the updated full patch is as following:
> 
> gcc/ChangeLog
> 
> 2017-12-13  Qing Zhao  <qing.zhao@oracle.com >
> 
>      PR middle_end/79538
>      * gimple-fold.c (get_range_strlen): Add the handling of non-member array.
> 
> gcc/fortran/ChangeLog
> 
> 2017-12-13  Qing Zhao  <qing.zhao@oracle.com >
> 
>       PR middle_end/79538
>       * class.c (gfc_build_class_symbol): Replace call to
>       sprintf with xasprintf to avoid format-overflow warning.
>       (generate_finalization_wrapper): Likewise.
>       (gfc_find_derived_vtab): Likewise.
>       (find_intrinsic_vtab): Likewise.
> 
> 
> gcc/c-family/ChangeLog
> 
> 2017-12-13  Qing Zhao  <qing.zhao@oracle.com >
> 
>      PR middle_end/79538 
> 	* c-cppbuiltin.c (builtin_define_with_hex_fp_value):
>      Adjust the size of buf1 and buf2, add a new buf to avoid
>      format-overflow warning.
> 
> gcc/testsuite/ChangeLog
> 
> 2017-12-13  Qing Zhao  <qing.zhao@oracle.com >
> 
>      PR middle_end/79538
>      * gcc.dg/pr79538.c: New test.
> 
> ---
>  gcc/c-family/c-cppbuiltin.c    | 10 ++++-----
>  gcc/fortran/class.c            | 49 ++++++++++++++++++++++++------------------
>  gcc/gimple-fold.c              | 13 +++++++++++
>  gcc/testsuite/gcc.dg/pr79538.c | 23 ++++++++++++++++++++
>  4 files changed, 69 insertions(+), 26 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/pr79538.c
> 
> diff --git a/gcc/c-family/c-cppbuiltin.c b/gcc/c-family/c-cppbuiltin.c
> index 2ac9616..9e33aed 100644
> --- a/gcc/c-family/c-cppbuiltin.c
> +++ b/gcc/c-family/c-cppbuiltin.c
> @@ -1613,7 +1613,7 @@ builtin_define_with_hex_fp_value (const char *macro,
>  				  const char *fp_cast)
>  {
>    REAL_VALUE_TYPE real;
> -  char dec_str[64], buf1[256], buf2[256];
> +  char dec_str[64], buf[256], buf1[128], buf2[64];
>  
>    /* This is very expensive, so if possible expand them lazily.  */
>    if (lazy_hex_fp_value_count < 12
> @@ -1656,11 +1656,11 @@ builtin_define_with_hex_fp_value (const char *macro,
>  
>    /* Assemble the macro in the following fashion
>       macro = fp_cast [dec_str fp_suffix] */
> -  sprintf (buf1, "%s%s", dec_str, fp_suffix);
> -  sprintf (buf2, fp_cast, buf1);
> -  sprintf (buf1, "%s=%s", macro, buf2);
> +  sprintf (buf2, "%s%s", dec_str, fp_suffix);
> +  sprintf (buf1, fp_cast, buf2);
> +  sprintf (buf, "%s=%s", macro, buf1);
>  
> -  cpp_define (parse_in, buf1);
> +  cpp_define (parse_in, buf);
>  }
>  
>  /* Return a string constant for the suffix for a value of type TYPE
> diff --git a/gcc/fortran/class.c b/gcc/fortran/class.c
> index ebbd41b..a08fb8d 100644
> --- a/gcc/fortran/class.c
> +++ b/gcc/fortran/class.c
> @@ -602,7 +602,8 @@ bool
>  gfc_build_class_symbol (gfc_typespec *ts, symbol_attribute *attr,
>  			gfc_array_spec **as)
>  {
> -  char name[GFC_MAX_SYMBOL_LEN+1], tname[GFC_MAX_SYMBOL_LEN+1];
> +  char tname[GFC_MAX_SYMBOL_LEN+1];
> +  char *name;
>    gfc_symbol *fclass;
>    gfc_symbol *vtab;
>    gfc_component *c;
> @@ -633,17 +634,17 @@ gfc_build_class_symbol (gfc_typespec *ts, symbol_attribute *attr,
>    rank = !(*as) || (*as)->rank == -1 ? GFC_MAX_DIMENSIONS : (*as)->rank;
>    get_unique_hashed_string (tname, ts->u.derived);
>    if ((*as) && attr->allocatable)
> -    sprintf (name, "__class_%s_%d_%da", tname, rank, (*as)->corank);
> +    name = xasprintf ("__class_%s_%d_%da", tname, rank, (*as)->corank);
>    else if ((*as) && attr->pointer)
> -    sprintf (name, "__class_%s_%d_%dp", tname, rank, (*as)->corank);
> +    name = xasprintf ("__class_%s_%d_%dp", tname, rank, (*as)->corank);
>    else if ((*as))
> -    sprintf (name, "__class_%s_%d_%dt", tname, rank, (*as)->corank);
> +    name = xasprintf ("__class_%s_%d_%dt", tname, rank, (*as)->corank);
>    else if (attr->pointer)
> -    sprintf (name, "__class_%s_p", tname);
> +    name = xasprintf ("__class_%s_p", tname);
>    else if (attr->allocatable)
> -    sprintf (name, "__class_%s_a", tname);
> +    name = xasprintf ("__class_%s_a", tname);
>    else
> -    sprintf (name, "__class_%s_t", tname);
> +    name = xasprintf ("__class_%s_t", tname);
>  
>    if (ts->u.derived->attr.unlimited_polymorphic)
>      {
> @@ -738,6 +739,7 @@ gfc_build_class_symbol (gfc_typespec *ts, symbol_attribute *attr,
>    ts->u.derived = fclass;
>    attr->allocatable = attr->pointer = attr->dimension = attr->codimension = 0;
>    (*as) = NULL;
> +  free (name);
>    return true;
>  }
>  
> @@ -1527,7 +1529,7 @@ generate_finalization_wrapper (gfc_symbol *derived, gfc_namespace *ns,
>    gfc_component *comp;
>    gfc_namespace *sub_ns;
>    gfc_code *last_code, *block;
> -  char name[GFC_MAX_SYMBOL_LEN+1];
> +  char *name;
>    bool finalizable_comp = false;
>    bool expr_null_wrapper = false;
>    gfc_expr *ancestor_wrapper = NULL, *rank;
> @@ -1606,7 +1608,7 @@ generate_finalization_wrapper (gfc_symbol *derived, gfc_namespace *ns,
>    sub_ns->resolved = 1;
>  
>    /* Set up the procedure symbol.  */
> -  sprintf (name, "__final_%s", tname);
> +  name = xasprintf ("__final_%s", tname);
>    gfc_get_symbol (name, sub_ns, &final);
>    sub_ns->proc_name = final;
>    final->attr.flavor = FL_PROCEDURE;
> @@ -2172,6 +2174,7 @@ generate_finalization_wrapper (gfc_symbol *derived, gfc_namespace *ns,
>    gfc_free_expr (rank);
>    vtab_final->initializer = gfc_lval_expr_from_sym (final);
>    vtab_final->ts.interface = final;
> +  free (name);
>  }
>  
>  
> @@ -2239,10 +2242,11 @@ gfc_find_derived_vtab (gfc_symbol *derived)
>  
>    if (ns)
>      {
> -      char name[GFC_MAX_SYMBOL_LEN+1], tname[GFC_MAX_SYMBOL_LEN+1];
> +      char tname[GFC_MAX_SYMBOL_LEN+1];
> +      char *name;
>  
>        get_unique_hashed_string (tname, derived);
> -      sprintf (name, "__vtab_%s", tname);
> +      name = xasprintf ("__vtab_%s", tname);
>  
>        /* Look for the vtab symbol in various namespaces.  */
>        if (gsym && gsym->ns)
> @@ -2270,7 +2274,7 @@ gfc_find_derived_vtab (gfc_symbol *derived)
>  	  vtab->attr.vtab = 1;
>  	  vtab->attr.access = ACCESS_PUBLIC;
>  	  gfc_set_sym_referenced (vtab);
> -	  sprintf (name, "__vtype_%s", tname);
> +	  name = xasprintf ("__vtype_%s", tname);
>  
>  	  gfc_find_symbol (name, ns, 0, &vtype);
>  	  if (vtype == NULL)
> @@ -2373,7 +2377,7 @@ gfc_find_derived_vtab (gfc_symbol *derived)
>  	      else
>  		{
>  		  /* Construct default initialization variable.  */
> -		  sprintf (name, "__def_init_%s", tname);
> +		  name = xasprintf ("__def_init_%s", tname);
>  		  gfc_get_symbol (name, ns, &def_init);
>  		  def_init->attr.target = 1;
>  		  def_init->attr.artificial = 1;
> @@ -2406,7 +2410,7 @@ gfc_find_derived_vtab (gfc_symbol *derived)
>  		  ns->contained = sub_ns;
>  		  sub_ns->resolved = 1;
>  		  /* Set up procedure symbol.  */
> -		  sprintf (name, "__copy_%s", tname);
> +		  name = xasprintf ("__copy_%s", tname);
>  		  gfc_get_symbol (name, sub_ns, &copy);
>  		  sub_ns->proc_name = copy;
>  		  copy->attr.flavor = FL_PROCEDURE;
> @@ -2483,7 +2487,7 @@ gfc_find_derived_vtab (gfc_symbol *derived)
>  		  ns->contained = sub_ns;
>  		  sub_ns->resolved = 1;
>  		  /* Set up procedure symbol.  */
> -		  sprintf (name, "__deallocate_%s", tname);
> +		  name = xasprintf ("__deallocate_%s", tname);
>  		  gfc_get_symbol (name, sub_ns, &dealloc);
>  		  sub_ns->proc_name = dealloc;
>  		  dealloc->attr.flavor = FL_PROCEDURE;
> @@ -2532,6 +2536,7 @@ have_vtype:
>  	  vtab->ts.u.derived = vtype;
>  	  vtab->value = gfc_default_initializer (&vtab->ts);
>  	}
> +      free (name);
>      }
>  
>    found_sym = vtab;
> @@ -2623,13 +2628,14 @@ find_intrinsic_vtab (gfc_typespec *ts)
>  
>    if (ns)
>      {
> -      char name[GFC_MAX_SYMBOL_LEN+1], tname[GFC_MAX_SYMBOL_LEN+1];
> -
> +      char tname[GFC_MAX_SYMBOL_LEN+1];
> +      char *name;
> +      
>        /* Encode all types as TYPENAME_KIND_ including especially character
>  	 arrays, whose length is now consistently stored in the _len component
>  	 of the class-variable.  */
>        sprintf (tname, "%s_%d_", gfc_basic_typename (ts->type), ts->kind);
> -      sprintf (name, "__vtab_%s", tname);
> +      name = xasprintf ("__vtab_%s", tname);
>  
>        /* Look for the vtab symbol in the top-level namespace only.  */
>        gfc_find_symbol (name, ns, 0, &vtab);
> @@ -2646,7 +2652,7 @@ find_intrinsic_vtab (gfc_typespec *ts)
>  	  vtab->attr.vtab = 1;
>  	  vtab->attr.access = ACCESS_PUBLIC;
>  	  gfc_set_sym_referenced (vtab);
> -	  sprintf (name, "__vtype_%s", tname);
> +	  name = xasprintf ("__vtype_%s", tname);
>  
>  	  gfc_find_symbol (name, ns, 0, &vtype);
>  	  if (vtype == NULL)
> @@ -2722,12 +2728,12 @@ find_intrinsic_vtab (gfc_typespec *ts)
>  	      c->tb->ppc = 1;
>  
>  	      if (ts->type != BT_CHARACTER)
> -		sprintf (name, "__copy_%s", tname);
> +		name = xasprintf ("__copy_%s", tname);
>  	      else
>  		{
>  		  /* __copy is always the same for characters.
>  		     Check to see if copy function already exists.  */
> -		  sprintf (name, "__copy_character_%d", ts->kind);
> +		  name = xasprintf ("__copy_character_%d", ts->kind);
>  		  contained = ns->contained;
>  		  for (; contained; contained = contained->sibling)
>  		    if (contained->proc_name
> @@ -2796,6 +2802,7 @@ find_intrinsic_vtab (gfc_typespec *ts)
>  	  vtab->ts.u.derived = vtype;
>  	  vtab->value = gfc_default_initializer (&vtab->ts);
>  	}
> +      free (name);
>      }
>  
>    found_sym = vtab;
> diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
> index 353a46e..eb6a87a 100644
> --- a/gcc/gimple-fold.c
> +++ b/gcc/gimple-fold.c
> @@ -1323,6 +1323,19 @@ get_range_strlen (tree arg, tree length[2], bitmap *visited, int type,
>  		 the array could have zero length.  */
>  	      *minlen = ssize_int (0);
>  	    }
> +
> +	  if (VAR_P (arg) 
> +	      && TREE_CODE (TREE_TYPE (arg)) == ARRAY_TYPE)
> +	    {
> +	      val = TYPE_SIZE_UNIT (TREE_TYPE (arg));
> +	      if (!val || TREE_CODE (val) != INTEGER_CST || integer_zerop (val))
> +		return false;
> +	      val = wide_int_to_tree (TREE_TYPE (val), 
> +				      wi::sub(wi::to_wide (val), 1));
> +	      /* Set the minimum size to zero since the string in
> +		 the array could have zero length.  */
> +	      *minlen = ssize_int (0);
> +	    }
>  	}
>  
>        if (!val)
> diff --git a/gcc/testsuite/gcc.dg/pr79538.c b/gcc/testsuite/gcc.dg/pr79538.c
> new file mode 100644
> index 0000000..c5a631e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr79538.c
> @@ -0,0 +1,23 @@
> +/* PR middle-end/79538 - missing -Wformat-overflow with %s and non-member array arguments
> +   { dg-do compile }
> +   { dg-options "-O2 -Wformat-overflow" } */
> +
> +char a3[3];
> +char a4[4];
> +char d[3];
> +
> +void g (int i)
> +{
> +  const char *s = i < 0 ? a3 : a4;
> +  __builtin_sprintf (d, "%s", s);      /* { dg-warning ".__builtin_sprintf. may write a terminating nul past the end of the destination" } */
> +  return;
> +}
> +
> +void f ()
> +{
> +  char des[3];
> +  char src[] = "abcd";
> +  __builtin_sprintf (des, "%s", src); /* { dg-warning "directive writing up to 4 bytes into a region of size 3" } */
> +  return;
> +}
> +
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]