PR tree-optimization/87096 - Optimised snprintf is not POSIX conformant
authorMartin Sebor <msebor@redhat.com>
Fri, 14 Dec 2018 22:38:08 +0000 (22:38 +0000)
committerMartin Sebor <msebor@gcc.gnu.org>
Fri, 14 Dec 2018 22:38:08 +0000 (15:38 -0700)
gcc/ChangeLog:

PR rtl-optimization/87096
* gimple-ssa-sprintf.c (sprintf_dom_walker::handle_gimple_call): Avoid
folding calls whose bound may exceed INT_MAX.  Diagnose bound ranges
that exceed the limit.

gcc/testsuite/ChangeLog:

PR tree-optimization/87096
* gcc.dg/tree-ssa/builtin-snprintf-4.c: New test.

From-SVN: r267157

gcc/ChangeLog
gcc/gimple-ssa-sprintf.c
gcc/testsuite/ChangeLog
gcc/testsuite/gcc.dg/tree-ssa/builtin-snprintf-4.c [new file with mode: 0644]

index 68f8f6d4193eb856fd6ea9a3806c5339891ce29f..6d0f1d50c11766585dcc936fdaa7d60f02b216ec 100644 (file)
@@ -1,3 +1,10 @@
+2018-12-14  Martin Sebor  <msebor@redhat.com>
+
+       PR rtl-optimization/87096
+       * gimple-ssa-sprintf.c (sprintf_dom_walker::handle_gimple_call): Avoid
+       folding calls whose bound may exceed INT_MAX.  Diagnose bound ranges
+       that exceed the limit.
+
 2018-12-14  Martin Sebor  <msebor@redhat.com>
 
        PR web/79738
index 00485d9a0735a3ca6aae48a6010dddcc353d558c..52286a6da1cfe4f54bd319eb22f0f3e589d3fad8 100644 (file)
@@ -3990,6 +3990,7 @@ sprintf_dom_walker::handle_gimple_call (gimple_stmt_iterator *gsi)
   /* True when the destination size is constant as opposed to the lower
      or upper bound of a range.  */
   bool dstsize_cst_p = true;
+  bool posunder4k = true;
 
   if (idx_dstsize == UINT_MAX)
     {
@@ -4022,11 +4023,20 @@ sprintf_dom_walker::handle_gimple_call (gimple_stmt_iterator *gsi)
                            "specified bound %wu exceeds maximum object size "
                            "%wu",
                            dstsize, target_size_max () / 2);
+             /* POSIX requires snprintf to fail if DSTSIZE is greater
+                than INT_MAX.  Even though not all POSIX implementations
+                conform to the requirement, avoid folding in this case.  */
+             posunder4k = false;
            }
          else if (dstsize > target_int_max ())
-           warning_at (gimple_location (info.callstmt), info.warnopt (),
-                       "specified bound %wu exceeds %<INT_MAX%>",
-                       dstsize);
+           {
+             warning_at (gimple_location (info.callstmt), info.warnopt (),
+                         "specified bound %wu exceeds %<INT_MAX%>",
+                         dstsize);
+             /* POSIX requires snprintf to fail if DSTSIZE is greater
+                than INT_MAX.  Avoid folding in that case.  */
+             posunder4k = false;
+           }
        }
       else if (TREE_CODE (size) == SSA_NAME)
        {
@@ -4035,9 +4045,29 @@ sprintf_dom_walker::handle_gimple_call (gimple_stmt_iterator *gsi)
             of them at level 2.  */
          value_range *vr = evrp_range_analyzer.get_value_range (size);
          if (range_int_cst_p (vr))
-           dstsize = (warn_level < 2
-                      ? TREE_INT_CST_LOW (vr->max ())
-                      : TREE_INT_CST_LOW (vr->min ()));
+           {
+             unsigned HOST_WIDE_INT minsize = TREE_INT_CST_LOW (vr->min ());
+             unsigned HOST_WIDE_INT maxsize = TREE_INT_CST_LOW (vr->max ());
+             dstsize = warn_level < 2 ? maxsize : minsize;
+
+             if (minsize > target_int_max ())
+               warning_at (gimple_location (info.callstmt), info.warnopt (),
+                           "specified bound range [%wu, %wu] exceeds "
+                           "%<INT_MAX%>",
+                           minsize, maxsize);
+
+             /* POSIX requires snprintf to fail if DSTSIZE is greater
+                than INT_MAX.  Avoid folding if that's possible.  */
+             if (maxsize > target_int_max ())
+               posunder4k = false;
+           }
+         else if (vr->varying_p ())
+           {
+             /* POSIX requires snprintf to fail if DSTSIZE is greater
+                than INT_MAX.  Since SIZE's range is unknown, avoid
+                folding.  */
+             posunder4k = false;
+           }
 
          /* The destination size is not constant.  If the function is
             bounded (e.g., snprintf) a lower bound of zero doesn't
@@ -4122,7 +4152,7 @@ sprintf_dom_walker::handle_gimple_call (gimple_stmt_iterator *gsi)
      directive.  Clear POSUNDER4K for the former set of functions and set
      it to true for the latter (it can only be cleared later, but it is
      never set to true again).  */
-  res.posunder4k = dstptr;
+  res.posunder4k = posunder4k && dstptr;
 
   bool success = compute_format_length (info, &res);
   if (res.warned)
index 9c24e473e5fe7da36477e3ffe4c4b34bf6f8c612..caa4f37db78b684e79cc44310d37c4410c3c6a08 100644 (file)
@@ -1,3 +1,8 @@
+2018-12-14  Martin Sebor  <msebor@redhat.com>
+
+       PR tree-optimization/87096
+       * gcc.dg/tree-ssa/builtin-snprintf-4.c: New test.
+
 2018-12-14  Alexandre Oliva <aoliva@redhat.com>
 
        PR c++/87814
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-snprintf-4.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-snprintf-4.c
new file mode 100644 (file)
index 0000000..5a61608
--- /dev/null
@@ -0,0 +1,156 @@
+/* PR tree-optimization/87096 - "Optimised" snprintf is not POSIX conformant
+   Verify that calls to snprintf with size in excess of INT_MAX are not
+   treated as successful.
+   It would be valid for GCC to fold some of these calls to a negative
+   value provided it also arranged to set errno to EOVERFLOW.  If that
+   is ever implemented this test will need to be adjusted.
+   { dg-do compile }
+   { dg-options "-O2 -Wall -fdump-tree-optimized -ftrack-macro-expansion=0" } */
+
+#include "../range.h"
+
+typedef __builtin_va_list va_list;
+
+extern int snprintf (char*, size_t, const char*, ...);
+extern int vsnprintf (char*, size_t, const char*, va_list);
+
+#define CAT(x, y) x ## y
+#define CONCAT(x, y) CAT (x, y)
+#define FAILNAME(name) CONCAT (call_ ## name ##_on_line_, __LINE__)
+
+#define FAIL(name) do {                                \
+    extern void FAILNAME (name) (void);                \
+    FAILNAME (name)();                         \
+  } while (0)
+
+/* Macro to emit a call to function named
+     call_in_true_branch_not_eliminated_on_line_NNN()
+   for each expression that's expected to fold to false but that
+   GCC does not fold.  The dg-final scan-tree-dump-time directive
+   at the bottom of the test verifies that no such call appears
+   in output.  */
+#define ELIM(expr)                                                     \
+  if ((expr)) FAIL (in_true_branch_not_eliminated); else (void)0
+
+/* Macro to emit a call to a function named
+     call_made_in_{true,false}_branch_on_line_NNN()
+   for each call that's expected to be retained.  The dg-final
+   scan-tree-dump-time directive at the bottom of the test verifies
+   that the expected number of both kinds of calls appears in output
+   (a pair for each line with the invocation of the KEEP() macro.  */
+#define KEEP(expr)                             \
+  if (expr)                                    \
+    FAIL (made_in_true_branch);                        \
+  else                                         \
+    FAIL (made_in_false_branch)
+
+extern void sink (int, ...);
+#define sink(...) sink (0, __VA_ARGS__)
+
+#define WARN(N, expr)                          \
+  do {                                         \
+    char a[N];                                 \
+    expr;                                      \
+    sink (a);                                  \
+  } while (0)
+
+
+static const size_t imax = __INT_MAX__;
+static const size_t imaxp1 = imax + 1;
+
+static const size_t dmax = __PTRDIFF_MAX__;
+static const size_t dmaxp1 = dmax + 1;
+
+static const size_t szmax = __SIZE_MAX__;
+static const size_t szmaxm1 = __SIZE_MAX__ - 1;
+
+
+void test_size_cst (char **d)
+{
+  ELIM (0 > snprintf (*d++, imax, "%s", ""));
+
+  KEEP (0 > snprintf (*d++, imaxp1, "%s", ""));   /* { dg-warning "\\\[-Wformat-truncation=]" } */
+
+  KEEP (0 > snprintf (*d++, dmax, "%s", ""));     /* { dg-warning "\\\[-Wformat-truncation=]" } */
+  KEEP (0 > snprintf (*d++, dmaxp1, "%s", ""));   /* { dg-warning "\\\[-Wformat-truncation=]" } */
+  KEEP (0 > snprintf (*d++, szmaxm1, "%s", ""));  /* { dg-warning "\\\[-Wformat-truncation=]" } */
+  KEEP (0 > snprintf (*d++, szmax, "%s", ""));    /* { dg-warning "\\\[-Wformat-truncation=]" } */
+}
+
+
+void test_size_cst_va (char **d, va_list va)
+{
+  ELIM (0 > vsnprintf (*d++, imax, " ", va));
+
+  KEEP (0 > vsnprintf (*d++, imaxp1, " ", va));   /* { dg-warning "\\\[-Wformat-truncation=]" } */
+
+  KEEP (0 > vsnprintf (*d++, dmax, " ", va));     /* { dg-warning "\\\[-Wformat-truncation=]" } */
+  KEEP (0 > vsnprintf (*d++, dmaxp1, " ", va));   /* { dg-warning "\\\[-Wformat-truncation=]" } */
+  KEEP (0 > vsnprintf (*d++, szmaxm1, " ", va));  /* { dg-warning "\\\[-Wformat-truncation=]" } */
+  KEEP (0 > vsnprintf (*d++, szmax, " ", va));    /* { dg-warning "\\\[-Wformat-truncation=]" } */
+}
+
+
+void test_size_range (char **d)
+{
+  size_t r = UR (imax - 1, imax);
+  ELIM (0 > snprintf (*d++, r, "%s", ""));
+
+  r = UR (imax, imax + 1);
+  KEEP (0 > snprintf (*d++, r, "%s", ""));
+
+  r = UR (imaxp1, imaxp1 + 1);
+  KEEP (0 > snprintf (*d++, r, "%s", ""));        /* { dg-warning "specified bound range \\\[\[0-9\]+, \[0-9\]+] exceeds .INT_MAX." } */
+
+  r = UR (dmax, dmaxp1);
+  KEEP (0 > snprintf (*d++, r, "%s", ""));        /* { dg-warning "\\\[-Wformat-truncation=]" } */
+
+  r = UR (dmaxp1, dmaxp1 + 1);
+  KEEP (0 > snprintf (*d++, r, "%s", ""));        /* { dg-warning "\\\[-Wformat-truncation=]" } */
+
+  r = UR (szmaxm1, szmax);
+  KEEP (0 > snprintf (*d++, r, "%s", ""));        /* { dg-warning "\\\[-Wformat-truncation=]" } */
+}
+
+
+void test_size_range_va (char **d, va_list va)
+{
+  size_t r = UR (imax - 1, imax);
+  ELIM (0 > vsnprintf (*d++, r, " ", va));
+
+  r = UR (imax, imax + 1);
+  KEEP (0 > vsnprintf (*d++, r, " ", va));
+
+  r = UR (imaxp1, imaxp1 + 1);
+  KEEP (0 > vsnprintf (*d++, r, " ", va));        /* { dg-warning "specified bound range \\\[\[0-9\]+, \[0-9\]+] exceeds .INT_MAX." } */
+
+  r = UR (dmax, dmaxp1);
+  KEEP (0 > vsnprintf (*d++, r, " ", va));        /* { dg-warning "\\\[-Wformat-truncation=]" } */
+
+  r = UR (dmaxp1, dmaxp1 + 1);
+  KEEP (0 > vsnprintf (*d++, r, " ", va));        /* { dg-warning "\\\[-Wformat-truncation=]" } */
+
+  r = UR (szmaxm1, szmax);
+  KEEP (0 > vsnprintf (*d++, r, " ", va));        /* { dg-warning "\\\[-Wformat-truncation=]" } */
+}
+
+
+void test_size_varying (char **d, size_t n)
+{
+  KEEP (0 > snprintf (*d++, n, "%s", ""));
+
+  n += 1;
+  KEEP (0 > snprintf (*d++, n, "%s", ""));
+}
+
+
+void test_size_varying_va (char **d, size_t n, va_list va)
+{
+  KEEP (0 > vsnprintf (*d++, n, " ", va));
+
+  n += 1;
+  KEEP (0 > vsnprintf (*d++, n, " ", va));
+}
+
+/* { dg-final { scan-tree-dump-times " = snprintf" 12 "optimized"} }
+   { dg-final { scan-tree-dump-times " = vsnprintf" 12 "optimized"} } */