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]

[PATCH] correct an ILP32/LP64 bug in sprintf warning (PR 91567)


The recent sprintf+strlen integration doesn't handle unbounded
string lengths entirely correctly for ILP32 targets and causes
-Wformat-overflow false positives in some common cases, including
during GCC bootstrap targeting such systems  The attached patch
fixes that mistake.  (I think this code could be cleaned up and
simplified some more but in the interest of unblocking the ILP32
bootstrap and Glibc builds I haven't taken the time to do that.)
The patch also adjusts down the maximum strlen result set by EVRP
to PTRDIFF_MAX - 2, to match what the strlen pass does.

The strlen maximum would ideally be computed in terms of
max_object_size() (for which there would ideally be a --param
setting), and checked the same way to avoid off-by-one mistakes
between subsystems and their clients.  I have not made this change
here but added a FIXME comment mentioning it.  I plan to add such
a parameter and use it in max_object_size() in a future change.

Testing with an ILP32 compiler also ran into the known limitation
of the strlen pass being unable to determine the length of array
members of local aggregates (PR 83543) initialized using
the braced-list syntax.  gcc.dg/tree-ssa/builtin-snprintf-6.c
fails a few cases as a result.    I've xfailed the assertions
for targets other than x86_64 where it isn't an issue.

Martin
PR tree-optimization/91567 - Spurious -Wformat-overflow warnings building glibc (32-bit only)

gcc/ChangeLog:

	PR tree-optimization/91567
	* gimple-ssa-sprintf.c (get_string_length): Handle more forms of lengths
	of unknown strings.
	* vr-values.c (vr_values::extract_range_basic): Set strlen upper bound
	to PTRDIFF_MAX - 2.

gcc/testsuite/ChangeLog:

	PR tree-optimization/91567
	* gcc.dg/tree-ssa/builtin-snprintf-6.c: Xfail a subset of assertions
	on targets other than x86_64 to work around PR 83543.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-22.c: New test.

Index: gcc/gimple-ssa-sprintf.c
===================================================================
--- gcc/gimple-ssa-sprintf.c	(revision 274960)
+++ gcc/gimple-ssa-sprintf.c	(working copy)
@@ -1994,11 +1994,22 @@ get_string_length (tree str, unsigned eltsize, con
      or it's SIZE_MAX otherwise.  */
 
   /* Return the default result when nothing is known about the string.  */
-  if (lendata.maxbound
-      && integer_all_onesp (lendata.maxbound)
-      && integer_all_onesp (lendata.maxlen))
-    return fmtresult ();
+  if (lendata.maxbound)
+    {
+      if (integer_all_onesp (lendata.maxbound)
+      	  && integer_all_onesp (lendata.maxlen))
+      	return fmtresult ();
 
+      if (!tree_fits_uhwi_p (lendata.maxbound)
+	  || !tree_fits_uhwi_p (lendata.maxlen))
+      	return fmtresult ();
+
+      unsigned HOST_WIDE_INT lenmax = tree_to_uhwi (max_object_size ()) - 2;
+      if (lenmax <= tree_to_uhwi (lendata.maxbound)
+	  && lenmax <= tree_to_uhwi (lendata.maxlen))
+	return fmtresult ();
+    }
+
   HOST_WIDE_INT min
     = (tree_fits_uhwi_p (lendata.minlen)
        ? tree_to_uhwi (lendata.minlen)
Index: gcc/vr-values.c
===================================================================
--- gcc/vr-values.c	(revision 274960)
+++ gcc/vr-values.c	(working copy)
@@ -1319,7 +1319,12 @@ vr_values::extract_range_basic (value_range *vr, g
 		tree max = vrp_val_max (ptrdiff_type_node);
 		wide_int wmax = wi::to_wide (max, TYPE_PRECISION (TREE_TYPE (max)));
 		tree range_min = build_zero_cst (type);
-		tree range_max = wide_int_to_tree (type, wmax - 1);
+		/* To account for the terminating NUL, the maximum length
+		   is one less than the maximum array size, which in turn
+		   is one  less than PTRDIFF_MAX (or SIZE_MAX where it's
+		   smaller than the former type).
+		   FIXME: Use max_object_size() - 1 here.  */
+		tree range_max = wide_int_to_tree (type, wmax - 2);
 		vr->set (VR_RANGE, range_min, range_max);
 		return;
 	      }
Index: gcc/testsuite/gcc.dg/tree-ssa/builtin-snprintf-6.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/builtin-snprintf-6.c	(revision 274960)
+++ gcc/testsuite/gcc.dg/tree-ssa/builtin-snprintf-6.c	(working copy)
@@ -65,6 +65,10 @@ void test_assign_init_list (void)
   T (5, ARGS ({ 1, 2, 3, 4, 5, 6, 0 }), "s=%.*s", 3, &a[2]);
 }
 
+#if __x86_64__
+
+/* Enabled only on x86_64 to work around PR 83543.  */
+
 #undef T
 #define T(expect, init, fmt, ...)			\
   do {							\
@@ -87,7 +91,10 @@ void test_assign_aggregate (void)
   T (5, "123456", "s=%.*s", 3, &s.a[2]);
 }
 
+/* { dg-final { scan-tree-dump-times "Function test_assign_aggregate" 1 "optimized" { xfail { ! x86_64-*-* } } } } */
 
+#endif   /* x86_64 */
+
 #undef T
 #define T(expect, init, fmt, ...)			\
   do {							\
Index: gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-22.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-22.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-22.c	(working copy)
@@ -0,0 +1,58 @@
+/* PR tree-optimization/91567 - Spurious -Wformat-overflow warnings building
+   glibc (32-bit only)
+   { dg-do compile }
+   { dg-options "-O2 -Wall -ftrack-macro-expansion=0" } */
+
+typedef __SIZE_TYPE__ size_t;
+
+extern int sprintf (char*, const char*, ...);
+extern size_t strlen (const char*);
+
+void f (char *);
+
+void g (char *s1, char *s2)
+{
+  char b[1025];
+  size_t n = __builtin_strlen (s1), d = __builtin_strlen (s2);
+  if (n + d + 1 >= 1025)
+    return;
+
+  sprintf (b, "%s.%s", s1, s2);     // { dg-bogus "\\\[-Wformat-overflow" }
+
+  f (b);
+}
+
+/* Extracted from gcc/c-cppbuiltin.c.  */
+
+void cpp_define (char*);
+
+static void
+builtin_define_type_minmax (const char *min_macro, const char *max_macro,
+			    void *type)
+{
+  extern const char *suffix;
+  char *buf;
+
+  if (type)
+    {
+      buf = (char *) __builtin_alloca (__builtin_strlen (min_macro) + 2
+				       + __builtin_strlen (suffix) + 1);
+      sprintf (buf, "%s=0%s", min_macro, suffix);      // { dg-bogus "\\\[-Wformat-overflow" }
+    }
+  else
+    {
+      buf = (char *) __builtin_alloca (__builtin_strlen (min_macro) + 3
+				       + __builtin_strlen (max_macro) + 6);
+      sprintf (buf, "%s=(-%s - 1)", min_macro, max_macro);  // { dg-bogus "\\\[-Wformat-overflow" }
+    }
+
+  cpp_define (buf);
+}
+
+void
+c_cpp_builtins (void *type)
+{
+
+  builtin_define_type_minmax ("__WCHAR_MIN__", "__WCHAR_MAX__", type);
+  builtin_define_type_minmax ("__WINT_MIN__", "__WINT_MAX__", type);
+}

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