Hi,

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79538 
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79538>
missing -Wformat-overflow with %s and non-member array arguments

-Wformat-overflow uses the routine "get_range_strlen" to decide the
maximum string length, however, currently "get_range_strlen" misses
the handling of non-member arrays.

Adding the handling of non-member array resolves the issue.
Adding test case pr79538.c into gcc.dg.

During gcc bootstrap, 2 source files (c-family/c-cppbuiltin.c,
fortran/class.c) were detected new warnings by -Wformat-overflow
due to the new handling of non-member array in "get_range_strlen".
in order to avoid these new warnings and continue with bootstrap,
updating these 2 files to avoid the warnings.

in c-family/c-cppbuiltin.c, the warning is following:

../../latest_gcc_2/gcc/c-family/c-cppbuiltin.c:1627:15: note:
‘sprintf’ output 2 or more bytes (assuming 257) into a destination
of size 256
      sprintf (buf1, "%s=%s", macro, buf2);
      ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
in the above, buf1 and buf2 are declared as:
char buf1[256], buf2[256];

i.e, buf1 and buf2 have same size. adjusting the size of buf1 and
buf2 resolves the warning.

fortran/class.c has the similar issue as above. Instead of adjusting
size of the buffers, replacing sprintf with xasprintf.

bootstraped and tested on both X86 and aarch64. no regression.

Okay for trunk?

thanks.

Qing

===========================================================

gcc/ChangeLog

2017-11-30  Qing Zhao  <qing.z...@oracle.com <mailto:qing.z...@oracle.com>>

      PR middle_end/79538
      * gimple-fold.c (get_range_strlen): Add the handling of non-member array.

gcc/fortran/ChangeLog

2017-11-30  Qing Zhao  <qing.z...@oracle.com <mailto:qing.z...@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-11-30  Qing Zhao  <qing.z...@oracle.com <mailto:qing.z...@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-11-30  Qing Zhao  <qing.z...@oracle.com <mailto:qing.z...@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..fef1969 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 || integer_zerop (val))
+                return false;
+              val = fold_build2 (MINUS_EXPR, TREE_TYPE (val), val,
+                                 integer_one_node);
+              /* 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;
+}
+
-- 
1.9.1

Reply via email to