PR tree-optimization/78512 - [7 Regression] r242674 miscompiles Linux kernel
authorMartin Sebor <msebor@redhat.com>
Tue, 29 Nov 2016 21:08:02 +0000 (21:08 +0000)
committerMartin Sebor <msebor@gcc.gnu.org>
Tue, 29 Nov 2016 21:08:02 +0000 (14:08 -0700)
gcc/ChangeLog:

PR tree-optimization/78512
* config/linux.h (TARGET_PRINTF_POINTER_FORMAT): Remove.
* config/rs6000/linux.h: Same.
* config/rs6000/linux64.h: Same.
* config/sol2.h: Same.
* config/sol2.c (solaris_printf_pointer_format): Remove.
* doc/tm.texi.in (TARGET_PRINTF_POINTER_FORMAT): Remove.
* doc/tm.texi: Regenerate.
* gimple-ssa-sprintf.c (format_pointer): Rempove.
(pass_sprintf_length::compute_format_length): Return bool.
(pass_sprintf_length::handle_gimple_call): Adjust.
* target.def (printf_pointer_format): Remove.
* targhooks.c (default_printf_pointer_format): Remove.
(linux_printf_pointer_format): Same.
* targhooks.h (default_printf_pointer_format): Remove.
(linux_printf_pointer_format, solaris_printf_pointer_format): Same.

gcc/testsuite/ChangeLog:

PR tree-optimization/78512
* gcc.dg/tree-ssa/builtin-sprintf-6.c: Add test cases.
* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Remove test cases.

From-SVN: r242975

15 files changed:
gcc/ChangeLog
gcc/config/linux.h
gcc/config/rs6000/linux.h
gcc/config/rs6000/linux64.h
gcc/config/sol2.c
gcc/config/sol2.h
gcc/doc/tm.texi
gcc/doc/tm.texi.in
gcc/gimple-ssa-sprintf.c
gcc/target.def
gcc/targhooks.c
gcc/targhooks.h
gcc/testsuite/ChangeLog
gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-6.c
gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c

index f9bcdbd9e49f97b696c405b410256d14da2d6777..256d121c77dd52b5a73031b1b31eef34a24aca53 100644 (file)
@@ -1,3 +1,22 @@
+2016-11-29  Martin Sebor  <msebor@redhat.com>
+
+       PR tree-optimization/78512
+       * config/linux.h (TARGET_PRINTF_POINTER_FORMAT): Remove.
+       * config/rs6000/linux.h: Same.
+       * config/rs6000/linux64.h: Same.
+       * config/sol2.h: Same.
+       * config/sol2.c (solaris_printf_pointer_format): Remove.
+       * doc/tm.texi.in (TARGET_PRINTF_POINTER_FORMAT): Remove.
+       * doc/tm.texi: Regenerate.
+       * gimple-ssa-sprintf.c (format_pointer): Rempove.
+       (pass_sprintf_length::compute_format_length): Return bool.
+       (pass_sprintf_length::handle_gimple_call): Adjust.
+       * target.def (printf_pointer_format): Remove.
+       * targhooks.c (default_printf_pointer_format): Remove.
+       (linux_printf_pointer_format): Same.
+       * targhooks.h (default_printf_pointer_format): Remove.
+       (linux_printf_pointer_format, solaris_printf_pointer_format): Same.
+
 2016-11-29  Uros Bizjak  <ubizjak@gmail.com>
 
        * config/i386/sse.md (UNSPEC_MASKOP): Move from i386.md.
index 7211da2008978f75cca3c639dcd92e9dc9d3a481..9aeeb948f55040986aac7bfcedeb1f3e82cea738 100644 (file)
@@ -208,8 +208,3 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 # define TARGET_LIBC_HAS_FUNCTION linux_libc_has_function
 
 #endif
-
-/* The format string to which "%p" corresponds (same in Glibc and
-   uClibc.  */
-#undef TARGET_PRINTF_POINTER_FORMAT
-#define TARGET_PRINTF_POINTER_FORMAT linux_printf_pointer_format
index a28e17f966c3402f5da33c9a0d69bb3b02d27bb5..ac9296d79ec8edc92c570a7d39855e03d79431be 100644 (file)
   || (TARGET_GLIBC_MAJOR == 2 && TARGET_GLIBC_MINOR >= 19)
 #define RS6000_GLIBC_ATOMIC_FENV 1
 #endif
-
-/* The format string to which "%p" corresponds.  */
-#undef TARGET_PRINTF_POINTER_FORMAT
-#define TARGET_PRINTF_POINTER_FORMAT linux_printf_pointer_format
index 7de51ea81a409df111851b0d62eefd72d69c2b18..0101ec0ac698fad979b6d0c55abd763a7074c7ab 100644 (file)
@@ -640,7 +640,3 @@ extern int dot_symbols;
    enabling the __float128 keyword.  */
 #undef TARGET_FLOAT128_ENABLE_TYPE
 #define TARGET_FLOAT128_ENABLE_TYPE 1
-
-/* The format string to which "%p" corresponds.  */
-#undef TARGET_PRINTF_POINTER_FORMAT
-#define TARGET_PRINTF_POINTER_FORMAT linux_printf_pointer_format
index fcab9de054c60c3335ea4e96460f8cbde1a75ba9..97f92e6c91f731131d8c6e0c34e5c65de68c8a24 100644 (file)
@@ -31,9 +31,6 @@ along with GCC; see the file COPYING3.  If not see
 #include "varasm.h"
 #include "output.h"
 
-#undef TARGET_PRINTF_POINTER_FORMAT
-#define TARGET_PRINTF_POINTER_FORMAT solaris_printf_pointer_format
-
 tree solaris_pending_aligns, solaris_pending_inits, solaris_pending_finis;
 
 /* Attach any pending attributes for DECL to the list in *ATTRIBUTES.
@@ -301,14 +298,3 @@ solaris_override_options (void)
   if (!HAVE_LD_EH_FRAME_CIEV3 && !global_options_set.x_dwarf_version)
     dwarf_version = 2;
 }
-
-/* Solaris libc formats pointers as if by "%zx" with the pound ('#')
-   format flag having the same meaning as in the integer directive.  */
-
-const char*
-solaris_printf_pointer_format (tree, const char **flags)
-{
-  *flags = "#";
-
-  return "%zx";
-}
index 6f0270891f552e435552dfc10cd94793cde03db8..50f2b383a1b7007c1d7654fbcbea56c1395a7d0d 100644 (file)
@@ -440,10 +440,6 @@ along with GCC; see the file COPYING3.  If not see
 #undef TARGET_LIBC_HAS_FUNCTION
 #define TARGET_LIBC_HAS_FUNCTION default_libc_has_function
 
-/* The format string to which "%p" corresponds.  */
-#undef TARGET_LIBC_PRINTF_POINTER_FORMAT
-#define TARGET_LIBC_PRINTF_POINTER_FORMAT solaris_libc_printf_pointer_format
-
 extern GTY(()) tree solaris_pending_aligns;
 extern GTY(()) tree solaris_pending_inits;
 extern GTY(()) tree solaris_pending_finis;
index 7559c12210723875d44cc6904fae032f844ff62b..cdf5f482f346638de3da4a0e85d5c1f02a905fd9 100644 (file)
@@ -5422,10 +5422,6 @@ In either case, it remains possible to select code-generation for the alternate
 scheme, by means of compiler command line switches.
 @end defmac
 
-@deftypefn {Target Hook} {const char*} TARGET_PRINTF_POINTER_FORMAT (tree, const char **@var{flags})
-Determine the target @code{printf} implementation format string that the most closely corresponds to the @code{%p} format directive.  The object pointed to by the @var{flags} is set to a string consisting of recognized format flags such as the @code{'#'} character.
-@end deftypefn
-
 @node Addressing Modes
 @section Addressing Modes
 @cindex addressing modes
index bc6d3cbce029cbc63aa71d4331483aff9c0f7bec..bbf53c966ee580d43c14a37e57d96f943f761969 100644 (file)
@@ -4106,8 +4106,6 @@ In either case, it remains possible to select code-generation for the alternate
 scheme, by means of compiler command line switches.
 @end defmac
 
-@hook TARGET_PRINTF_POINTER_FORMAT
-
 @node Addressing Modes
 @section Addressing Modes
 @cindex addressing modes
index 43bc560665dba8a962c428c11425c3b2ef5efa66..732bc42a7673afc6edbf074aec89f7bc96d62b45 100644 (file)
@@ -72,7 +72,6 @@ along with GCC; see the file COPYING3.  If not see
 
 #include "realmpfr.h"
 #include "target.h"
-#include "targhooks.h"
 
 #include "cpplib.h"
 #include "input.h"
@@ -126,7 +125,7 @@ public:
   void handle_gimple_call (gimple_stmt_iterator*);
 
   struct call_info;
-  void compute_format_length (const call_info &, format_result *);
+  bool compute_format_length (const call_info &, format_result *);
 };
 
 bool
@@ -759,83 +758,12 @@ build_intmax_type_nodes (tree *pintmax, tree *puintmax)
     }
 }
 
-static fmtresult
-format_integer (const conversion_spec &, tree);
-
-/* Return a range representing the minimum and maximum number of bytes
-   that the conversion specification SPEC will write on output for the
-   pointer argument ARG when non-null.  ARG may be null (for vararg
-   functions).  */
-
-static fmtresult
-format_pointer (const conversion_spec &spec, tree arg)
-{
-  fmtresult res;
-
-  /* Determine the target's integer format corresponding to "%p".  */
-  const char *flags;
-  const char *pfmt = targetm.printf_pointer_format (arg, &flags);
-  if (!pfmt)
-    {
-      /* The format couldn't be determined.  */
-      res.range.min = res.range.max = HOST_WIDE_INT_M1U;
-      return res;
-    }
-
-  if (pfmt [0] == '%')
-    {
-      /* Format the pointer using the integer format string.  */
-      conversion_spec pspec = spec;
-
-      /* Clear flags that are not listed as recognized.  */
-      for (const char *pf = "+ #0"; *pf; ++pf)
-       {
-         if (!strchr (flags, *pf))
-           pspec.clear_flag (*pf);
-       }
-
-      /* Set flags that are specified in the format string.  */
-      bool flag_p = true;
-      do
-       {
-         switch (*++pfmt)
-           {
-           case '+': case ' ': case '#': case '0':
-             pspec.set_flag (*pfmt);
-             break;
-           default:
-             flag_p = false;
-           }
-       }
-      while (flag_p);
-
-      /* Set the appropriate length modifier taking care to clear
-       the one that may be set (Glibc's %p accepts but ignores all
-       the integer length modifiers).  */
-      switch (*pfmt)
-       {
-       case 'l': pspec.modifier = FMT_LEN_l; ++pfmt; break;
-       case 't': pspec.modifier = FMT_LEN_t; ++pfmt; break;
-       case 'z': pspec.modifier = FMT_LEN_z; ++pfmt; break;
-       default: pspec.modifier = FMT_LEN_none;
-       }
-
-      pspec.force_flags = 1;
-      pspec.specifier = *pfmt++;
-      gcc_assert (*pfmt == '\0');
-      return format_integer (pspec, arg);
-    }
-
-  /* The format is a plain string such as Glibc's "(nil)".  */
-  res.range.min = res.range.max = strlen (pfmt);
-  return res;
-}
-
 /* Set *PWIDTH and *PPREC according to the width and precision specified
    in SPEC.  Each is set to HOST_WIDE_INT_MIN when the corresponding
    field is specified but unknown, to zero for width and -1 for precision,
    respectively when it's not specified, or to a non-negative value
    corresponding to the known value.  */
+
 static void
 get_width_and_precision (const conversion_spec &spec,
                         HOST_WIDE_INT *pwidth, HOST_WIDE_INT *pprec)
@@ -867,7 +795,6 @@ get_width_and_precision (const conversion_spec &spec,
   *pprec = prec;
 }
 
-
 /* Return a range representing the minimum and maximum number of bytes
    that the conversion specification SPEC will write on output for the
    integer argument ARG when non-null.  ARG may be null (for vararg
@@ -2257,9 +2184,12 @@ add_bytes (const pass_sprintf_length::call_info &info,
 
 /* Compute the length of the output resulting from the call to a formatted
    output function described by INFO and store the result of the call in
-   *RES.  Issue warnings for detected past the end writes.  */
+   *RES.  Issue warnings for detected past the end writes.  Return true
+   if the complete format string has been processed and *RES can be relied
+   on, false otherwise (e.g., when a unknown or unhandled directive was seen
+   that caused the processing to be terminated early).  */
 
-void
+bool
 pass_sprintf_length::compute_format_length (const call_info &info,
                                            format_result *res)
 {
@@ -2299,7 +2229,7 @@ pass_sprintf_length::compute_format_length (const call_info &info,
       if (0 && *pf == 0)
        {
          /* Incomplete directive.  */
-         return;
+         return false;
        }
 
       conversion_spec spec = conversion_spec ();
@@ -2322,10 +2252,10 @@ pass_sprintf_length::compute_format_length (const call_info &info,
        {
          /* Similarly to the block above, this could be either a POSIX
             positional argument or a width, depending on what follows.  */
-         if (argno < gimple_call_num_args (info.callstmt))
-           spec.star_width = gimple_call_arg (info.callstmt, argno++);
-         else
-           return;
+         if (gimple_call_num_args (info.callstmt) <= argno)
+           return false;
+
+         spec.star_width = gimple_call_arg (info.callstmt, argno++);
          ++pf;
        }
 
@@ -2344,7 +2274,7 @@ pass_sprintf_length::compute_format_length (const call_info &info,
          if (dollar == 0
              || dollar == info.argidx
              || dollar > gimple_call_num_args (info.callstmt))
-           return;
+           return false;
 
          --dollar;
 
@@ -2411,7 +2341,7 @@ pass_sprintf_length::compute_format_length (const call_info &info,
                 estimate the upper bound on the size of the output
                 based on the number of digits it probably isn't worth
                 continuing.  */
-             return;
+             return false;
            }
        }
 
@@ -2520,11 +2450,14 @@ pass_sprintf_length::compute_format_length (const call_info &info,
          break;
 
        case 'p':
-         spec.fmtfunc = format_pointer;
-         break;
+         /* The %p output is implementation-defined.  It's possible
+            to determine this format but due to extensions (especially
+            those of the Linux kernel -- see bug 78512) the first %p
+            in the format string disables any further processing.  */
+         return false;
 
        case 'n':
-         return;
+         break;
 
        case 'c':
        case 'S':
@@ -2533,7 +2466,8 @@ pass_sprintf_length::compute_format_length (const call_info &info,
          break;
 
        default:
-         return;
+         /* Unknown conversion specification.  */
+         return false;
        }
 
       spec.specifier = *pf++;
@@ -2552,6 +2486,9 @@ pass_sprintf_length::compute_format_length (const call_info &info,
 
       ::format_directive (info, res, dir, dirlen, spec, arg);
     }
+
+  /* Complete format string was processed (with or without warnings).  */
+  return true;
 }
 
 /* Return the size of the object referenced by the expression DEST if
@@ -2893,13 +2830,15 @@ pass_sprintf_length::handle_gimple_call (gimple_stmt_iterator *gsi)
   /* The result is the number of bytes output by the formatted function,
      including the terminating NUL.  */
   format_result res = format_result ();
-  compute_format_length (info, &res);
+
+  bool success = compute_format_length (info, &res);
 
   /* When optimizing and the printf return value optimization is enabled,
      attempt to substitute the computed result for the return value of
      the call.  Avoid this optimization when -frounding-math is in effect
      and the format string contains a floating point directive.  */
-  if (optimize > 0
+  if (success
+      && optimize > 0
       && flag_printf_return_value
       && (!flag_rounding_math || !res.floating))
     try_substitute_return_value (gsi, info, res);
index 417cd0256c5c036937a2a713e4a85dc703f60c7d..ac3470ee6f200742e349a7495bf3060820242298 100644 (file)
@@ -3381,12 +3381,6 @@ greater than 128 and a multiple of 32.",
  machine_mode, (int n, bool extended),
  default_floatn_mode)
 
-DEFHOOK
-(printf_pointer_format,
- "Determine the target @code{printf} implementation format string that the most closely corresponds to the @code{%p} format directive.  The object pointed to by the @var{flags} is set to a string consisting of recognized format flags such as the @code{'#'} character.",
- const char*, (tree, const char **flags),
- default_printf_pointer_format)
-
 /* Compute cost of moving data from a register of class FROM to one of
    TO, using MODE.  */
 DEFHOOK
index a80b30172995377e98fd7086fdcd0b408d5042e1..5d3e91ef3adbb11ebabc0af5310eb7fc2d189f6d 100644 (file)
@@ -1512,36 +1512,6 @@ no_c99_libc_has_function (enum function_class fn_class ATTRIBUTE_UNUSED)
   return false;
 }
 
-/* Return the format string to which "%p" corresponds.  By default,
-   assume it corresponds to the C99 "%zx" format and set *FLAGS to
-   the empty string to indicate that format flags have no effect.
-   An example of an implementation that matches this description
-   is AIX.  */
-
-const char*
-default_printf_pointer_format (tree, const char **flags)
-{
-  *flags = "";
-
-  return "%zx";
-}
-
-/* For Glibc and uClibc targets also define the hook here because
-   otherwise it would have to be duplicated in each target's .c file
-   (such as in bfin/bfin.c and c6x/c6x.c, etc.)
-   Glibc and uClibc format pointers as if by "%zx" except for the null
-   pointer which outputs "(nil)".  It ignores the pound ('#') format
-   flag but interprets the space and plus flags the same as in the integer
-   directive.  */
-
-const char*
-linux_printf_pointer_format (tree arg, const char **flags)
-{
-  *flags = " +";
-
-  return arg && integer_zerop (arg) ? "(nil)" : "%#zx";
-}
-
 tree
 default_builtin_tm_load_store (tree ARG_UNUSED (type))
 {
index e00da602ec76d8101ecd529dd939262c6265cf5b..3a9271f379faf35f21712b18d280b848e4946602 100644 (file)
@@ -191,10 +191,6 @@ extern bool default_libc_has_function (enum function_class);
 extern bool no_c99_libc_has_function (enum function_class);
 extern bool gnu_libc_has_function (enum function_class);
 
-extern const char* default_printf_pointer_format (tree, const char **);
-extern const char* linux_printf_pointer_format (tree, const char **);
-extern const char* solaris_printf_pointer_format (tree, const char **);
-
 extern tree default_builtin_tm_load_store (tree);
 
 extern int default_memory_move_cost (machine_mode, reg_class_t, bool);
index c86c345055ef33122901fbf4639a8dec83f7a266..982d47cd2644779f4cfcc4080111e962f6c71f2c 100644 (file)
@@ -1,3 +1,9 @@
+2016-11-29  Martin Sebor  <msebor@redhat.com>
+
+       PR tree-optimization/78512
+       * gcc.dg/tree-ssa/builtin-sprintf-6.c: Add test cases.
+       * gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Remove test cases.
+
 2016-11-29  Uros Bizjak  <ubizjak@gmail.com>
 
        * gcc.target/i386/avx512f-kmovw-1.c (avx512f_test):
index 375fc094bcf83d43520c4f6767dac2c547f87c8f..4c412344c8aa256b92d5ac90759f12536932b7d6 100644 (file)
@@ -1,9 +1,11 @@
 /* PR middle-end/78476 - snprintf(0, 0, ...) with known arguments not
    optimized away
+   PR middle-end/78512 - r242674 miscompiles Linux kernel
    A negative test complementing builtin-sprintf-5.c to verify that calls
    to the function that do not return a constant are not optimized away.
+   Test also verifies that unknown directives prevent the optimization.
    { dg-compile }
-   { dg-options "-O2 -fdump-tree-optimized" }
+   { dg-options "-O2 -Wformat -fdump-tree-optimized" }
    { dg-require-effective-target int32plus } */
 
 #define CONCAT(a, b) a ## b
@@ -48,10 +50,25 @@ void test_arg_int (int width, int prec, int i, int n)
 
   T ("%i", R (1, 10));
 
+  T ("%'i", 1234567);
+
   for (i = -n; i != n; ++i)
     T ("%*x", n, i);
 }
 
+/* Support for %p was removed in response to PR middle-end/78512 due
+   to the Linux kernel relying on GCC builtins while at the same time
+   providing a large number of extensions to the %p directive that
+   interfere with the optimization.  Verify that %p disables it.  */
+
+void test_arg_ptr (int width, int prec, int i)
+{
+  T ("%p", (void*)0);
+  T ("p=%p", (void*)0);
+  T ("%s=%p", "p=", (void*)0);
+  T ("%i%p", 123, (void*)0);
+}
+
 void test_arg_string (int width, int prec, const char *s)
 {
   T ("%-s", s);
@@ -69,5 +86,24 @@ void test_arg_string (int width, int prec, const char *s)
   T ("%*.*s", width, prec, "123");
 }
 
+void test_invalid_directive (void)
+{
+  T ("%");        /* { dg-warning "spurious trailing .%." } */
+  T ("abc%");     /* { dg-warning "spurious trailing .%." } */
+
+  T ("%2$i");     /* { dg-warning "operand number out of range" } */
+  T ("abc%2$i");  /* { dg-warning "operand number out of range" } */
+
+  T ("%=i", 0);   /* { dg-warning "unknown conversion type character .=." } */
+  /* { dg-warning "too many arguments" "" { target *-*-* } .-1 } */
+
+  T ("%*i", "", 0); /* { dg-warning "field width specifier .\\*. expects argument of type .int." } */
+  T ("%.*i", "", 0); /* { dg-warning "field precision specifier .\\.\\*. expects argument of type .int." } */
+  T ("%.*.i", 0);   /* { dg-warning "unknown conversion type character .\\.." } */
+  T ("%Q");       /* { dg-warning "unknown conversion type character .Q." } */
+  T ("abc%Q");    /* { dg-warning "unknown conversion type character .Q." } */
+}
 
-/* { dg-final { scan-tree-dump-times "snprintf" 27 "optimized"} } */
+/* Use 'grep "^ *T (" builtin-sprintf-6.c  | wc -l' to determine
+   the count for the directive below.
+   { dg-final { scan-tree-dump-times "snprintf" 42 "optimized"} } */
index 7937149cf36b2ef2bdda187117d3949679d6e0e3..4b0813effe6506920066973b5ed322952ffe5ba1 100644 (file)
@@ -99,30 +99,6 @@ void test_sprintf_c_const (void)
   T (-1, "%*cX", INT_MAX,     '1'); /* { dg-warning "output of \[0-9\]+ bytes causes result to exceed .INT_MAX." } */
 }
 
-/* Exercise the "%p" directive with constant arguments.  */
-
-void test_sprintf_p_const (void)
-{
-  /* GLIBC and uClibc format null pointers as "(nil)".  Sane implementations
-     format null pointers as 0 or 0x0 and so the following will only be
-     diagnosed on the former targets.  */
-  T (5, "%p",     (void*)0);
-  /* { dg-warning "nul past the end" "(nil)" { target *-linux-gnu *-*-uclinux } .-1 } */
-
-  /* The exact output for %p is unspecified by C.  Two formats are known:
-     same as %tx (for example AIX) and same as %#tx (for example Solaris).  */
-  T (0, "%p",     (void*)0x1);    /* { dg-warning ".%p. directive writing . bytes? into a region of size 0" } */
-  T (1, "%p",     (void*)0x12);   /* { dg-warning ".%p. directive writing . bytes? into a region of size 1" } */
-  T (2, "%p",     (void*)0x123);  /* { dg-warning ".%p. directive writing . bytes? into a region of size 2" } */
-
-  /* GLIBC and uClibc treat the ' ' flag with the "%p" directive the same
-     as with signed integer conversions (i.e., it prepends a space).  Other
-     known implementations ignore it.  */
-  T (6, "% p",    (void*)0x234);  /* { dg-warning ". . flag used with .%p." } */
-  /* { dg-warning "nul past the end" "Glibc %p" { target *-linux-gnu } .-1 } */
-  /* { dg-warning "nul past the end" "Generic %p" { target *-*-uclinux } .-2 } */
-}
-
 /* Verify that no warning is issued for calls that write into a flexible
    array member whose size isn't known.  Also verify that calls that use
    a flexible array member as an argument to the "%s" directive do not