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] use zero as the lower bound for a signed-unsigned range (PR 79327)


-  T (2, "%#hho",        a);     /* { dg-warning "nul past the end" } */
-  T (2, "%#hhx",        a);     /* { dg-warning ".%#hhx. directive
writing between 3 and . bytes into a region of size 2" } */
+  T (2, "%#hho",        a);
+  T (2, "%#hhx",        a);

On reflection, this isn't quite the right fix.  We want to both set
the correct range and warn because the call will likely overflow.
This is an example of why the likely/unlikely counters have been
introduced.  By setting min = 1 and likely = 2 for the %#hho and
3 for the %#hhx we get the desired result.

Attached is a simple patch that removes the vestigial setting of
the minimum counter while preserving the warnings above by using
the likely counter.

I had overlooked this when I introduced the likely counter and so
in the corner cases of "%#o" and "%#x" with a non-constant argument
that could be zero, the minimum counter would be set to 2 and 3
respectively rather than 1 (because zero is formatted without
the '0' or '0x' base prefix).

Martin
gcc/ChangeLog:

	* gimple-ssa-sprintf.c (format_integer): Adjust the likely counter
	to assume an unknown argument that may be zero is non-zero.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/builtin-sprintf-warn-13.c: New test.

diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 9e099f0..84dd3671 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -1242,6 +1242,10 @@ format_integer (const directive &dir, tree arg)
        of the format string by returning [-1, -1].  */
     return fmtresult ();
 
+  /* The adjustment to add to the MIN counter when computing the LIKELY
+     counter for arguments with unknown values.  */
+  unsigned likely_adjust = 0;
+
   fmtresult res;
 
   /* Using either the range the non-constant argument is in, or its
@@ -1273,6 +1277,15 @@ format_integer (const directive &dir, tree arg)
 
 	  res.argmin = argmin;
 	  res.argmax = argmax;
+
+	  /* Set the adjustment for an argument whose range includes
+	     zero since that doesn't include the octal or hexadecimal
+	     base prefix.  */
+	  wide_int wzero = wi::zero (wi::get_precision (min));
+	  if (maybebase
+	      && wi::le_p (min, wzero, SIGNED)
+	      && wi::le_p (wzero, max, SIGNED))
+	    likely_adjust = base == 8 ? 1 : base == 16 ? 2 : 0;
 	}
       else if (range_type == VR_ANTI_RANGE)
 	{
@@ -1311,11 +1324,14 @@ format_integer (const directive &dir, tree arg)
 	 or one whose value range cannot be determined, create a T_MIN
 	 constant if the argument's type is signed and T_MAX otherwise,
 	 and use those to compute the range of bytes that the directive
-	 can output.  When precision may be zero, use zero as the minimum
-	 since it results in no bytes on output (unless width is specified
-	 to be greater than 0).  */
-      bool zero = dir.prec[0] <= 0 && dir.prec[1] >= 0;
-      argmin = build_int_cst (argtype, !zero);
+	 can output.  */
+      argmin = integer_zero_node;
+
+      /* Set the adjustment for an argument whose range includes
+	 zero since that doesn't include the octal or hexadecimal
+	 base prefix.  */
+      if (maybebase)
+	likely_adjust = base == 8 ? 1 : base == 16 ? 2 : 0;
 
       int typeprec = TYPE_PRECISION (dirtype);
       int argprec = TYPE_PRECISION (argtype);
@@ -1391,7 +1407,11 @@ format_integer (const directive &dir, tree arg)
       res.range.min = tmp;
     }
 
-  res.range.likely = res.knownrange ? res.range.max : res.range.min;
+  /* Add the adjustment for an argument whose range includes zero
+     since it doesn't include the octal or hexadecimal base prefix.  */
+  res.range.likely
+    = res.knownrange ? res.range.max : res.range.min + likely_adjust;
+
   res.range.unlikely = res.range.max;
   res.adjust_for_width_or_precision (dir.width, dirtype, base,
 				     (sign | maybebase) + (base == 16));
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-13.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-13.c
new file mode 100644
index 0000000..deba705
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-13.c
@@ -0,0 +1,180 @@
+/* { dg-do compile }
+   { dg-options "-O2 -Wall -Wformat-overflow=1 -ftrack-macro-expansion=0" }
+   { dg-require-effective-target int32plus } */
+
+typedef __SIZE_TYPE__  size_t;
+typedef __WCHAR_TYPE__ wchar_t;
+
+#define INT_MAX __INT_MAX__
+#define INT_MIN (-INT_MAX - 1)
+
+/* When debugging, define LINE to the line number of the test case to exercise
+   and avoid exercising any of the others.  The buffer and objsize macros
+   below make use of LINE to avoid warnings for other lines.  */
+#ifndef LINE
+# define LINE 0
+#endif
+
+void sink (char*, char*);
+
+int dummy_sprintf (char*, const char*, ...);
+
+char buffer [256];
+extern char *ptr;
+
+int int_range (int min, int max)
+{
+  extern int int_value (void);
+  int n = int_value ();
+  return n < min || max < n ? min : n;
+}
+
+unsigned uint_range (unsigned min, unsigned max)
+{
+  extern unsigned uint_value (void);
+  unsigned n = uint_value ();
+  return n < min || max < n ? min : n;
+}
+
+/* Evaluate to an array of SIZE characters when non-negative, or to
+   a pointer to an unknown object otherwise.  */
+#define buffer(size)					\
+  ((0 <= size) ? buffer + sizeof buffer - (size) : ptr)
+
+/* Helper to expand function to either __builtin_f or dummy_f to
+   make debugging GCC easy.  */
+#define FUNC(f)							\
+  ((!LINE || LINE == __LINE__) ? __builtin_ ## f : dummy_ ## f)
+
+/* Macro to verify that calls to __builtin_sprintf (i.e., with no size
+   argument) issue diagnostics by correctly determining the size of
+   the destination buffer.  */
+#define T(size, ...)						\
+  (FUNC (sprintf) (buffer (size),  __VA_ARGS__),		\
+   sink (buffer, ptr))
+
+/* Return a signed integer in the range [MIN, MAX].  */
+#define R(min, max)  int_range (min, max)
+
+/* Return a unsigned integer in the range [MIN, MAX].  */
+#define U(min, max)  uint_range (min, max)
+
+/* Verify warnings and ranges for certain overflow.  */
+void test_min_overflow (int i)
+{
+  T (0, "%#hho", i);            /* { dg-warning "between 1 and 4 bytes" } */
+  T (0, "%#hho", R (-1,  0));   /* { dg-warning "between 1 and 4 bytes" } */
+  T (0, "%#hho", R (-1,  1));   /* { dg-warning "between 1 and 4 bytes" } */
+  T (0, "%#hho", R ( 0,  1));   /* { dg-warning "between 1 and 2 bytes" } */
+  T (0, "%#hho", R ( 1,  2));   /* { dg-warning "writing 2 bytes" } */
+
+  T (0, "%#ho",  i);            /* { dg-warning "between 1 and 7 bytes" } */
+  T (0, "%#ho",  R (-1,  0));   /* { dg-warning "between 1 and 7 bytes" } */
+  T (0, "%#ho",  R (-1,  1));   /* { dg-warning "between 1 and 7 bytes" } */
+  T (0, "%#ho",  R ( 0,  1));   /* { dg-warning "between 1 and 2 bytes" } */
+  T (0, "%#ho",  R ( 1,  2));   /* { dg-warning "writing 2 bytes" } */
+
+  T (0, "%#o",   i);            /* { dg-warning "between 1 and 12 bytes" } */
+  T (0, "%#o",   R (-1,  0));   /* { dg-warning "between 1 and 12 bytes" } */
+  T (0, "%#o",   R (-1,  1));   /* { dg-warning "between 1 and 12 bytes" } */
+  T (0, "%#o",   R ( 0,  1));   /* { dg-warning "between 1 and 2 bytes" } */
+  T (0, "%#o",   R ( 1,  2));   /* { dg-warning "writing 2 bytes" } */
+
+  T (0, "%#hhx", i);            /* { dg-warning "between 1 and 4 bytes" } */
+  T (0, "%#hhx", R (-1,  0));   /* { dg-warning "between 1 and 4 bytes" } */
+  T (0, "%#hhx", R (-1,  1));   /* { dg-warning "between 1 and 4 bytes" } */
+  T (0, "%#hhx", R ( 0,  1));   /* { dg-warning "between 1 and 3 bytes" } */
+  T (0, "%#hhx", R ( 1,  2));   /* { dg-warning "writing 3 bytes" } */
+
+  T (0, "%#hx", i);             /* { dg-warning "between 1 and 6 bytes" } */
+  T (0, "%#hx", R (-1,  0));    /* { dg-warning "between 1 and 6 bytes" } */
+  T (0, "%#hx", R (-1,  1));    /* { dg-warning "between 1 and 6 bytes" } */
+  T (0, "%#hx", R ( 0,  1));    /* { dg-warning "between 1 and 3 bytes" } */
+  T (0, "%#hx", R ( 1,  2));    /* { dg-warning "writing 3 bytes" } */
+
+  T (0, "%#x",   i);            /* { dg-warning "between 1 and 10 bytes" } */
+  T (0, "%#x",   R (-1,  0));   /* { dg-warning "between 1 and 10 bytes" } */
+  T (0, "%#x",   R (-1,  1));   /* { dg-warning "between 1 and 10 bytes" } */
+  T (0, "%#x",   R ( 0,  1));   /* { dg-warning "between 1 and 3 bytes" } */
+  T (0, "%#x",   R ( 1,  2));   /* { dg-warning "writing 3 bytes" } */
+}
+
+/* Verify warnings and ranges for likely overflow.  */
+void test_likely_overflow (int i)
+{
+  T (2, "%#hho", i);          /* { dg-warning "may write a terminating nul" } */
+  T (2, "%#hho", R (-1,  0)); /* { dg-warning "may write a terminating nul" } */
+  T (2, "%#hho", R (-1,  1)); /* { dg-warning "may write a terminating nul" } */
+  T (2, "%#hho", R ( 0,  1)); /* { dg-warning "may write a terminating nul" } */
+  T (2, "%#hho", R ( 1,  2)); /* { dg-warning "writing a terminating nul" } */
+
+  T (2, "%#ho",  i);          /* { dg-warning "may write a terminating nul" } */
+  T (2, "%#ho",  R (-1,  0)); /* { dg-warning "may write a terminating nul" } */
+  T (2, "%#ho",  R (-1,  1)); /* { dg-warning "may write a terminating nul" } */
+  T (2, "%#ho",  R ( 0,  1)); /* { dg-warning "may write a terminating nul" } */
+  T (2, "%#ho",  R ( 1,  2)); /* { dg-warning "writing a terminating nul" } */
+
+  T (2, "%#o",   i);          /* { dg-warning "may write a terminating nul" } */
+  T (2, "%#o",   R (-1,  0)); /* { dg-warning "may write a terminating nul" } */
+  T (2, "%#o",   R (-1,  1)); /* { dg-warning "may write a terminating nul" } */
+  T (2, "%#o",   R ( 0,  1)); /* { dg-warning "may write a terminating nul" } */
+  T (2, "%#o",   R ( 1,  2)); /* { dg-warning "writing a terminating nul" } */
+
+  T (2, "%#hhx", i);          /* { dg-warning "between 1 and 4 bytes" } */
+  T (2, "%#hhx", R (-1,  0)); /* { dg-warning "between 1 and 4 bytes" } */
+  T (2, "%#hhx", R (-1,  1)); /* { dg-warning "between 1 and 4 bytes" } */
+  T (2, "%#hhx", R ( 0,  1)); /* { dg-warning "between 1 and 3 bytes" } */
+  T (2, "%#hhx", R ( 1,  2)); /* { dg-warning "writing 3 bytes" } */
+
+  T (2, "%#hx", i);           /* { dg-warning "between 1 and 6 bytes" } */
+  T (2, "%#hx", R (-1,  0));  /* { dg-warning "between 1 and 6 bytes" } */
+  T (2, "%#hx", R (-1,  1));  /* { dg-warning "between 1 and 6 bytes" } */
+  T (2, "%#hx", R ( 0,  1));  /* { dg-warning "between 1 and 3 bytes" } */
+  T (2, "%#hx", R ( 1,  2));  /* { dg-warning "writing 3 bytes" } */
+
+  T (2, "%#x",   i);          /* { dg-warning "between 1 and 10 bytes" } */
+  T (2, "%#x",   R (-1,  0)); /* { dg-warning "between 1 and 10 bytes" } */
+  T (2, "%#x",   R (-1,  1)); /* { dg-warning "between 1 and 10 bytes" } */
+  T (2, "%#x",   R ( 0,  1)); /* { dg-warning "between 1 and 3 bytes" } */
+  T (2, "%#x",   R ( 1,  2)); /* { dg-warning "writing 3 bytes" } */
+}
+
+/* Verify warnings likely overflow due to the terminating nul.  */
+void test_likely_nul_overflow (int i)
+{
+  T (3, "%#hho", i);
+  T (3, "%#hho", R (-1,  0));
+  T (3, "%#hho", R (-1,  1));
+  T (3, "%#hho", R ( 0,  1));
+  T (3, "%#hho", R ( 1,  2));
+
+  T (3, "%#ho",  i);
+  T (3, "%#ho",  R (-1,  0));
+  T (3, "%#ho",  R (-1,  1));
+  T (3, "%#ho",  R ( 0,  1));
+  T (3, "%#ho",  R ( 1,  2));
+
+  T (3, "%#o",   i);
+  T (3, "%#o",   R (-1,  0));
+  T (3, "%#o",   R (-1,  1));
+  T (3, "%#o",   R ( 0,  1));
+  T (3, "%#o",   R ( 1,  2));
+
+  T (3, "%#hhx", i);          /* { dg-warning "may write a terminating nul" } */
+  T (3, "%#hhx", R (-1,  0)); /* { dg-warning "may write a terminating nul" } */
+  T (3, "%#hhx", R (-1,  1)); /* { dg-warning "may write a terminating nul" } */
+  T (3, "%#hhx", R ( 0,  1)); /* { dg-warning "may write a terminating nul" } */
+  T (3, "%#hhx", R ( 1,  2)); /* { dg-warning "writing a terminating nul" } */
+
+  T (3, "%#hx", i);           /* { dg-warning "may write a terminating nul" } */
+  T (3, "%#hx", R (-1,  0));  /* { dg-warning "may write a terminating nul" } */
+  T (3, "%#hx", R (-1,  1));  /* { dg-warning "may write a terminating nul" } */
+  T (3, "%#hx", R ( 0,  1));  /* { dg-warning "may write a terminating nul" } */
+  T (3, "%#hx", R ( 1,  2));  /* { dg-warning "writing a terminating nul" } */
+
+  T (3, "%#x",   i);          /* { dg-warning "may write a terminating nul" } */
+  T (3, "%#x",   R (-1,  0)); /* { dg-warning "may write a terminating nul" } */
+  T (3, "%#x",   R (-1,  1)); /* { dg-warning "may write a terminating nul" } */
+  T (3, "%#x",   R ( 0,  1)); /* { dg-warning "may write a terminating nul" } */
+  T (3, "%#x",   R ( 1,  2)); /* { dg-warning "writing a terminating nul" } */
+}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
index 9955326..2b5f7dd 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
@@ -1152,7 +1152,7 @@ void test_sprintf_chk_hh_nonconst (int w, int p, int a)
   T (2, "% hhx",        a);     /* { dg-warning ". . flag used with .%x." } */
 
   T (2, "%#hho",        a);     /* { dg-warning "nul past the end" } */
-  T (2, "%#hhx",        a);     /* { dg-warning ".%#hhx. directive writing between 3 and . bytes into a region of size 2" } */
+  T (2, "%#hhx",        a);     /* { dg-warning ".%#hhx. directive writing between 1 and . bytes into a region of size 2" } */
 
   T (3, "%0hhd",        a);
   T (3, "%1hhd",        a);
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr79327-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr79327-2.c
new file mode 100644
index 0000000..2a2c297
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr79327-2.c
@@ -0,0 +1,129 @@
+/* { dg-compile }
+   { dg-options "-O2 -fdump-tree-optimized" } */
+
+#define CAT(s, n)   s ## n
+#define FAIL(line)  CAT (failure_on_line_, line)
+
+/* Emit a call to a function named failure_on_line_NNN when EXPR is false.  */
+#define ASSERT(expr)				\
+  do {						\
+    extern void FAIL (__LINE__)(void);		\
+    if (!(expr)) FAIL (__LINE__)();		\
+  } while (0)
+
+#define KEEP(line)  CAT (keep_call_on_line_, line)
+
+/* Emit a call to a function named keep_call_on_line_NNN when EXPR is true.
+   Used to verify that the expression need not be the only one that holds.  */
+#define ASSERT_MAYBE(expr)			\
+  do {						\
+    extern void KEEP (__LINE__)(void);		\
+    if (expr) KEEP (__LINE__)();		\
+  } while (0)
+
+int test_hho (int i)
+{
+  int n = __builtin_snprintf (0, 0, "%#hho", i);
+
+  ASSERT (0 < n && n < 5);
+
+  ASSERT_MAYBE (1 == n);
+  ASSERT_MAYBE (2 == n);
+  ASSERT_MAYBE (3 == n);
+  ASSERT_MAYBE (4 == n);
+
+  return n;
+}
+
+int test_ho (int i)
+{
+  int n = __builtin_snprintf (0, 0, "%#ho", i);
+
+  ASSERT (0 < n && n < 8);
+
+  ASSERT_MAYBE (1 == n);
+  ASSERT_MAYBE (2 == n);
+  ASSERT_MAYBE (3 == n);
+  ASSERT_MAYBE (4 == n);
+  ASSERT_MAYBE (5 == n);
+  ASSERT_MAYBE (6 == n);
+  ASSERT_MAYBE (7 == n);
+
+  return n;
+}
+
+int test_o (int i)
+{
+  int n = __builtin_snprintf (0, 0, "%#o", i);
+
+  ASSERT (0 < n && n < 13);
+
+  ASSERT_MAYBE (1 == n);
+  ASSERT_MAYBE (2 == n);
+  ASSERT_MAYBE (3 == n);
+  ASSERT_MAYBE (4 == n);
+  ASSERT_MAYBE (5 == n);
+  ASSERT_MAYBE (6 == n);
+  ASSERT_MAYBE (7 == n);
+  ASSERT_MAYBE (8 == n);
+  ASSERT_MAYBE (9 == n);
+  ASSERT_MAYBE (10 == n);
+  ASSERT_MAYBE (11 == n);
+  ASSERT_MAYBE (12 == n);
+
+  return n;
+}
+
+
+int test_hhx (int i)
+{
+  int n = __builtin_snprintf (0, 0, "%#hhx", i);
+
+  ASSERT (0 < n && n < 5);
+
+  ASSERT_MAYBE (1 == n);
+  ASSERT_MAYBE (2 == n);
+  ASSERT_MAYBE (3 == n);
+  ASSERT_MAYBE (4 == n);
+
+  return n;
+}
+
+int test_hx (int i)
+{
+  int n = __builtin_snprintf (0, 0, "%#hx", i);
+
+  ASSERT (0 < n && n < 7);
+
+  ASSERT_MAYBE (1 == n);
+  ASSERT_MAYBE (2 == n);
+  ASSERT_MAYBE (3 == n);
+  ASSERT_MAYBE (4 == n);
+  ASSERT_MAYBE (5 == n);
+  ASSERT_MAYBE (6 == n);
+
+  return n;
+}
+
+int test_x (int i)
+{
+  int n = __builtin_snprintf (0, 0, "%#x", i);
+
+  ASSERT (0 < n && n < 11);
+
+  ASSERT_MAYBE (1 == n);
+  ASSERT_MAYBE (2 == n);
+  ASSERT_MAYBE (3 == n);
+  ASSERT_MAYBE (4 == n);
+  ASSERT_MAYBE (5 == n);
+  ASSERT_MAYBE (6 == n);
+  ASSERT_MAYBE (7 == n);
+  ASSERT_MAYBE (8 == n);
+  ASSERT_MAYBE (9 == n);
+  ASSERT_MAYBE (10 == n);
+
+  return n;
+}
+
+/* { dg-final { scan-tree-dump-not "failure_on_line" "optimized"} }
+   { dg-final { scan-tree-dump-times "keep_call_on_line" 43 "optimized"} } */

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