+2017-05-14 Martin Sebor <msebor@redhat.com>
+
+ PR middle-end/77671
+ * gimple-fold.c (gimple_fold_builtin_sprintf): Make extern.
+ (gimple_fold_builtin_snprintf): Same.
+ * gimple-fold.h (gimple_fold_builtin_sprintf): Declare.
+ (gimple_fold_builtin_snprintf): Same.
+ * gimple-ssa-sprintf.c (get_format_string): Correct the detection
+ of character types.
+ (is_call_safe): New function.
+ (try_substitute_return_value): Call it.
+ (try_simplify_call): New function.
+ (pass_sprintf_length::handle_gimple_call): Call it.
+
2017-05-14 Martin Sebor <msebor@redhat.com>
PR middle-end/80669
ORIG may be null if this is a 2-argument call. We don't attempt to
simplify calls with more than 3 arguments.
- Return NULL_TREE if no simplification was possible, otherwise return the
- simplified form of the call as a tree. If IGNORED is true, it means that
- the caller does not use the returned value of the function. */
+ Return true if simplification was possible, otherwise false. */
-static bool
+bool
gimple_fold_builtin_sprintf (gimple_stmt_iterator *gsi)
{
gimple *stmt = gsi_stmt (*gsi);
FMT, and ORIG. ORIG may be null if this is a 3-argument call. We don't
attempt to simplify calls with more than 4 arguments.
- Return NULL_TREE if no simplification was possible, otherwise return the
- simplified form of the call as a tree. If IGNORED is true, it means that
- the caller does not use the returned value of the function. */
+ Return true if simplification was possible, otherwise false. */
-static bool
+bool
gimple_fold_builtin_snprintf (gimple_stmt_iterator *gsi)
{
gcall *stmt = as_a <gcall *> (gsi_stmt (*gsi));
case BUILT_IN_SNPRINTF_CHK:
case BUILT_IN_VSNPRINTF_CHK:
return gimple_fold_builtin_snprintf_chk (gsi, fcode);
- case BUILT_IN_SNPRINTF:
- return gimple_fold_builtin_snprintf (gsi);
- case BUILT_IN_SPRINTF:
- return gimple_fold_builtin_sprintf (gsi);
+
case BUILT_IN_FPRINTF:
case BUILT_IN_FPRINTF_UNLOCKED:
case BUILT_IN_VFPRINTF:
unsigned HOST_WIDE_INT,
bool *can_refer = NULL);
extern tree gimple_fold_indirect_ref (tree);
+extern bool gimple_fold_builtin_sprintf (gimple_stmt_iterator *);
+extern bool gimple_fold_builtin_snprintf (gimple_stmt_iterator *);
extern bool arith_code_with_undefined_signed_overflow (tree_code);
extern gimple_seq rewrite_to_defined_overflow (gimple *);
if (TREE_CODE (format) != STRING_CST)
return NULL;
- if (TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (format))) != char_type_node)
+ tree type = TREE_TYPE (format);
+
+ if (GET_MODE_CLASS (TYPE_MODE (TREE_TYPE (type))) != MODE_INT
+ || GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (type))) != 1)
{
/* Wide format string. */
return NULL;
return HOST_WIDE_INT_M1U;
}
-/* Given a suitable result RES of a call to a formatted output function
- described by INFO, substitute the result for the return value of
- the call. The result is suitable if the number of bytes it represents
- is known and exact. A result that isn't suitable for substitution may
- have its range set to the range of return values, if that is known.
- Return true if the call is removed and gsi_next should not be performed
- in the caller. */
+/* Return true if the call described by INFO with result RES safe to
+ optimize (i.e., no undefined behavior), and set RETVAL to the range
+ of its return values. */
static bool
-try_substitute_return_value (gimple_stmt_iterator *gsi,
- const pass_sprintf_length::call_info &info,
- const format_result &res)
+is_call_safe (const pass_sprintf_length::call_info &info,
+ const format_result &res, bool under4k,
+ unsigned HOST_WIDE_INT retval[2])
{
- tree lhs = gimple_get_lhs (info.callstmt);
-
- /* Set to true when the entire call has been removed. */
- bool removed = false;
+ if (under4k && !res.under4k)
+ return false;
/* The minimum return value. */
- unsigned HOST_WIDE_INT minretval = res.range.min;
+ retval[0] = res.range.min;
/* The maximum return value is in most cases bounded by RES.RANGE.MAX
but in cases involving multibyte characters could be as large as
RES.RANGE.UNLIKELY. */
- unsigned HOST_WIDE_INT maxretval
+ retval[1]
= res.range.unlikely < res.range.max ? res.range.max : res.range.unlikely;
/* Adjust the number of bytes which includes the terminating nul
a valid range before the adjustment below is [0, INT_MAX + 1]
(the functions only return negative values on error or undefined
behavior). */
- if (minretval <= target_int_max () + 1)
- --minretval;
- if (maxretval <= target_int_max () + 1)
- --maxretval;
+ if (retval[0] <= target_int_max () + 1)
+ --retval[0];
+ if (retval[1] <= target_int_max () + 1)
+ --retval[1];
/* Avoid the return value optimization when the behavior of the call
is undefined either because any directive may have produced 4K or
the output overflows the destination object (but leave it enabled
when the function is bounded because then the behavior is well-
defined). */
- if (res.under4k
- && minretval == maxretval
- && (info.bounded || minretval < info.objsize)
- && minretval <= target_int_max ()
+ if (retval[0] == retval[1]
+ && (info.bounded || retval[0] < info.objsize)
+ && retval[0] <= target_int_max ())
+ return true;
+
+ if ((info.bounded || retval[1] < info.objsize)
+ && (retval[0] < target_int_max ()
+ && retval[1] < target_int_max ()))
+ return true;
+
+ if (!under4k && (info.bounded || retval[0] < info.objsize))
+ return true;
+
+ return false;
+}
+
+/* Given a suitable result RES of a call to a formatted output function
+ described by INFO, substitute the result for the return value of
+ the call. The result is suitable if the number of bytes it represents
+ is known and exact. A result that isn't suitable for substitution may
+ have its range set to the range of return values, if that is known.
+ Return true if the call is removed and gsi_next should not be performed
+ in the caller. */
+
+static bool
+try_substitute_return_value (gimple_stmt_iterator *gsi,
+ const pass_sprintf_length::call_info &info,
+ const format_result &res)
+{
+ tree lhs = gimple_get_lhs (info.callstmt);
+
+ /* Set to true when the entire call has been removed. */
+ bool removed = false;
+
+ /* The minimum and maximum return value. */
+ unsigned HOST_WIDE_INT retval[2];
+ bool safe = is_call_safe (info, res, true, retval);
+
+ if (safe
+ && retval[0] == retval[1]
/* Not prepared to handle possibly throwing calls here; they shouldn't
appear in non-artificial testcases, except when the __*_chk routines
are badly declared. */
&& !stmt_ends_bb_p (info.callstmt))
{
- tree cst = build_int_cst (integer_type_node, minretval);
+ tree cst = build_int_cst (integer_type_node, retval[0]);
if (lhs == NULL_TREE
&& info.nowrite)
{
bool setrange = false;
- if ((info.bounded || maxretval < info.objsize)
- && res.under4k
- && (minretval < target_int_max ()
- && maxretval < target_int_max ()))
+ if (safe
+ && (info.bounded || retval[1] < info.objsize)
+ && (retval[0] < target_int_max ()
+ && retval[1] < target_int_max ()))
{
/* If the result is in a valid range bounded by the size of
the destination set it so that it can be used for subsequent
optimizations. */
int prec = TYPE_PRECISION (integer_type_node);
- wide_int min = wi::shwi (minretval, prec);
- wide_int max = wi::shwi (maxretval, prec);
+ wide_int min = wi::shwi (retval[0], prec);
+ wide_int max = wi::shwi (retval[1], prec);
set_range_info (lhs, VR_RANGE, min, max);
setrange = true;
if (dump_file)
{
const char *inbounds
- = (minretval < info.objsize
- ? (maxretval < info.objsize
+ = (retval[0] < info.objsize
+ ? (retval[1] < info.objsize
? "in" : "potentially out-of")
: "out-of");
const char *what = setrange ? "Setting" : "Discarding";
- if (minretval != maxretval)
+ if (retval[0] != retval[1])
fprintf (dump_file,
" %s %s-bounds return value range [%llu, %llu].\n",
what, inbounds,
- (unsigned long long)minretval,
- (unsigned long long)maxretval);
+ (unsigned long long)retval[0],
+ (unsigned long long)retval[1]);
else
fprintf (dump_file, " %s %s-bounds return value %llu.\n",
- what, inbounds, (unsigned long long)minretval);
+ what, inbounds, (unsigned long long)retval[0]);
}
}
return removed;
}
+/* Try to simplify a s{,n}printf call described by INFO with result
+ RES by replacing it with a simpler and presumably more efficient
+ call (such as strcpy). */
+
+static bool
+try_simplify_call (gimple_stmt_iterator *gsi,
+ const pass_sprintf_length::call_info &info,
+ const format_result &res)
+{
+ unsigned HOST_WIDE_INT dummy[2];
+ if (!is_call_safe (info, res, info.retval_used (), dummy))
+ return false;
+
+ switch (info.fncode)
+ {
+ case BUILT_IN_SNPRINTF:
+ return gimple_fold_builtin_snprintf (gsi);
+
+ case BUILT_IN_SPRINTF:
+ return gimple_fold_builtin_sprintf (gsi);
+
+ default:
+ ;
+ }
+
+ return false;
+}
+
/* Determine if a GIMPLE CALL is to one of the sprintf-like built-in
functions and if so, handle it. Return true if the call is removed
and gsi_next should not be performed in the caller. */
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 (success
- && optimize > 0
- && flag_printf_return_value
- && (!flag_rounding_math || !res.floating))
- return try_substitute_return_value (gsi, info, res);
+ bool call_removed = false;
+ if (success && optimize > 0)
+ {
+ /* Save a copy of the iterator pointing at the call. The iterator
+ may change to point past the call in try_substitute_return_value
+ but the original value is needed in try_simplify_call. */
+ gimple_stmt_iterator gsi_call = *gsi;
- return false;
+ if (flag_printf_return_value
+ && (!flag_rounding_math || !res.floating))
+ call_removed = try_substitute_return_value (gsi, info, res);
+
+ if (!call_removed)
+ try_simplify_call (&gsi_call, info, res);
+ }
+
+ return call_removed;
}
/* Execute the pass for function FUN. */
+2017-05-14 Martin Sebor <msebor@redhat.com>
+
+ PR middle-end/77671
+ * gcc.dg/tree-ssa/builtin-sprintf-7.c: New test.
+ * gcc.dg/tree-ssa/builtin-sprintf-8.c: New test.
+ * gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Adjust.
+ * gcc.dg/tree-ssa/builtin-sprintf-warn-2.c: Adjust.
+ * gcc.dg/tree-ssa/builtin-sprintf-warn-3.c: Adjust.
+
2017-05-14 Martin Sebor <msebor@redhat.com>
PR middle-end/80669
--- /dev/null
+/* PR tree-optimization/77671 - missing -Wformat-overflow warning
+ on sprintf overflow with "%s"
+ { dg-compile }
+ { dg-options "-O2 -Wformat -Wno-format-zero-length -fdump-tree-optimized" } */
+
+void sink (char*);
+
+extern char buffer[];
+
+/* String exactly 4100 characters long (plus the terminating NUL). */
+extern const char s4100[4101];
+
+void test_sprintf (const char *s)
+{
+#define IGN(...) __builtin_sprintf (buffer, __VA_ARGS__); sink (buffer)
+
+ /* Each of the following calls is expected to be transformed into
+ one of memcpy or strcpy. */
+ IGN ("");
+ IGN ("a");
+ IGN ("ab");
+ /* FIXME: Transform to strcpy/memcpy. */
+ /* IGN (s4100 + 5); */
+
+ IGN ("%s", "");
+ IGN ("%s", "a");
+ IGN ("%s", "ab");
+
+ IGN ("%s", s4100 + 5);
+
+ /* FIXME: This can be transformed into strcpy. */
+ /* IGN (s); */
+ IGN ("%s", s);
+}
+
+
+void test_snprintf (void)
+{
+#undef IGN
+#define IGN(N, ...) __builtin_snprintf (buffer, N, __VA_ARGS__); sink (buffer)
+
+ /* Each of the following calls is expected to be transformed into
+ one of memcpy or strcpy. */
+ IGN (1, "");
+ IGN (2, "1");
+ IGN (8, "1234567");
+
+ /* FIXME: Transform to strcpy/memcpy. */
+ /* IGN (4096, s4100 + 5); */
+
+ IGN (1, "%s", "");
+ IGN (2, "%s", "1");
+ IGN (8, "%s", "1234567");
+
+ IGN (4096, "%s", s4100 + 5);
+}
+
+#if 0 /* FIXME: Implement vs{,n}printf optimization. */
+
+void test_vsprintf (__builtin_va_list va)
+{
+#undef IGN
+#define IGN(fmt) __builtin_vsprintf (buffer, fmt, va); sink (buffer)
+
+ /* Each of the following calls is expected to be transformed into
+ one of memcpy or strcpy. */
+ IGN ("");
+ IGN ("a");
+ IGN ("ab");
+ IGN (s4100 + 5);
+
+ IGN ("%s");
+}
+
+void test_vsnprintf (__builtin_va_list va)
+{
+#undef IGN
+#define IGN(N, fmt) __builtin_vsnprintf (buffer, N, fmt, va); sink (buffer)
+
+ /* Each of the following calls is expected to be transformed into
+ one of memcpy or strcpy. */
+ IGN ( 1, "");
+ IGN ( 2, "1");
+ IGN ( 8, "1234567");
+ IGN (4096, s4100 + 5);
+}
+
+#endif
+
+/* { dg-final { scan-tree-dump-not "builtin_sprintf" "optimized" } }
+ { dg-final { scan-tree-dump-not "builtin_snprintf" "optimized" } }
+ { dg-final { scan-tree-dump-not "builtin_vsprintf" "optimized" } }
+ { dg-final { scan-tree-dump-not "builtin_vsnprintf" "optimized" } } */
+
+#define S10 "0123456789"
+#define S100 S10 S10 S10 S10 S10 S10 S10 S10 S10 S10
+#define S1000 S100 S100 S100 S100 S100 S100 S100 S100 S100 S100
+
+const char s4100[4101] = S1000 S1000 S1000 S1000 S100;
--- /dev/null
+/* PR tree-optimization/77671 - missing -Wformat-overflow warning
+ on sprintf overflow with "%s"
+
+ Negative test verifying that sprintf family calls that must not
+ be transformed into calls to other functions (such as memcpy)
+ are preserved.
+
+ { dg-compile }
+ { dg-options "-O2 -Wformat -Wno-format-truncation -Wno-format-zero-length -fdump-tree-optimized" } */
+
+void sink (char*, ...);
+
+extern char buffer[];
+
+/* String exactly 4100 characters long (plus the terminating NUL). */
+extern const char s4100[4101];
+
+void test_sprintf (const char *s)
+{
+ /* Macros to test the function call while eignoring and using
+ the return value, respectively. */
+#define IGN(...) __builtin_sprintf (buffer, __VA_ARGS__), sink (buffer)
+#define USE(...) sink (buffer, __builtin_sprintf (buffer, __VA_ARGS__))
+
+ /* All of the following calls to sprintf must be emitted (and not
+ transformed into memcpy, strcpy, or similar). */
+
+ /* Verify that a sprintf call with output in excess of the maximum
+ of 4095 bytes is not transformed into memcpy/strcpy when its
+ return value is used (the call may fail with EOVERFLOW but
+ the error is only detectable when the function's negative return
+ value indicates errno is valid ). */
+ USE (s4100);
+
+ USE ("%s", s4100);
+
+ /* Same as above but with string of unknown length (which could
+ be in excess of 4K long). */
+ USE (s);
+ USE ("%s", s);
+}
+
+
+void test_snprintf (void)
+{
+#undef IGN
+#define IGN(N, ...) __builtin_snprintf (buffer, N, __VA_ARGS__); sink (buffer)
+
+ /* All of the following calls to sprintf must be emitted (and not
+ transformed into memcpy, strcpy, or similar). */
+
+ /* Truncated output could be optimized into strncpy (not done yet). */
+ IGN (1, "123");
+ IGN (1, s4100);
+
+ IGN (1, "%s", "123");
+ IGN (1, "%s", s4100);
+
+ /* Output in excess of the maximum of 4095 bytes. */
+ IGN (4097, s4100);
+
+ IGN (4097, "%s", s4100);
+}
+
+
+void test_vsprintf (__builtin_va_list va)
+{
+#undef IGN
+#define IGN(fmt) __builtin_vsprintf (buffer, fmt, va); sink (buffer)
+
+ /* All of the following calls to vsprintf must be emitted (and not
+ transformed into memcpy, strcpy, or similar). */
+
+ /* Output in excess of the maximum of 4095 bytes. */
+ IGN (s4100);
+}
+
+
+void test_vsnprintf (__builtin_va_list va)
+{
+#undef IGN
+#define IGN(N, fmt) __builtin_vsnprintf (buffer, N, fmt, va); sink (buffer)
+
+ /* All of the following calls to vsnprintf must be emitted (and not
+ transformed into memcpy, strcpy, or similar). */
+
+ /* Truncated output could be optimized into strncpy (not done yet). */
+ IGN (1, "123");
+ IGN (1, s4100);
+
+ /* Output in excess of the maximum of 4095 bytes. */
+ IGN (4097, s4100);
+}
+
+/* { dg-final { scan-tree-dump-times "builtin_sprintf" 4 "optimized" } }
+ { dg-final { scan-tree-dump-times "builtin_snprintf" 6 "optimized" } }
+ { dg-final { scan-tree-dump-times "builtin_vsprintf" 1 "optimized" } }
+ { dg-final { scan-tree-dump-times "builtin_vsnprintf" 3 "optimized" } } */
+
+#define S10 "0123456789"
+#define S100 S10 S10 S10 S10 S10 S10 S10 S10 S10 S10
+#define S1000 S100 S100 S100 S100 S100 S100 S100 S100 S100 S100
+
+const char s4100[4101] = S1000 S1000 S1000 S1000 S100;
void test_sprintf_chk_s_nonconst (int w, int p, const char *s)
{
T (-1, "%s", s);
- T ( 0, "%s", s); /* { dg-warning "writing a terminating nul" } */
+ T ( 0, "%-s", s); /* { dg-warning "writing a terminating nul" } */
T ( 1, "%s", s);
T (-1, "%.0s", s);
T ( 1, "%.0s", s);
void test_s_nonconst (int w, int p, const char *s, const wchar_t *ws,
struct Arrays *a)
{
- T (0, "%s", s); /* { dg-warning "into a region" "sprintf transformed into strcpy" { xfail *-*-* } } */
- T (1, "%s", s); /* { dg-warning "nul past the end" "sprintf transformed into strcpy" { xfail *-*-* } } */
+ T (0, "%s", s); /* { dg-warning "into a region" } */
+ T (1, "%s", s); /* { dg-warning "nul past the end" } */
T (1, "%1s", s); /* { dg-warning "writing a terminating nul" } */
T (1, "%.0s", s);
T (1, "%.1s", s); /* { dg-warning "may write a terminating nul" } */
T (1, "%.1ls", ws); /* { dg-warning "may write a terminating nul" } */
T (1, "%ls", ws); /* { dg-warning "may write a terminating nul" } */
- /* Verify that the size of the array is used in lieu of its length.
- The minus sign disables GCC's sprintf to strcpy transformation.
- In the case below, the length of s->a1 can be at most zero, so
- the call should not be diagnosed. */
- T (1, "%-s", a->a1);
+ /* Verify that the size of the array is used in lieu of its length. */
+ T (1, "%s", a->a1);
/* In the following test, since the length of the strings isn't known,
their type (the array) is used to bound the maximum length to 1,
- which means the "%-s" directive would not overflow the buffer,
+ which means the "%s" directive would not overflow the buffer,
but it would leave no room for the terminating nul. */
- T (1, "%-s", a->a2); /* { dg-warning "may write a terminating nul" } */
+ T (1, "%s", a->a2); /* { dg-warning "may write a terminating nul" } */
/* Unlike in the test above, since the length of the string is bounded
- by the array type to at most 2, the "^-s" directive is diagnosed firts,
+ by the array type to at most 2, the "%s" directive is diagnosed firts,
preventing the diagnostic about the terminatinb nul. */
- T (1, "%-s", a->a3); /* { dg-warning "directive writing up to 2 bytes" } */
+ T (1, "%s", a->a3); /* { dg-warning "directive writing up to 2 bytes" } */
/* The length of a zero length array and flexible array member is
unknown and at leve 2 assumed to be at least 1. */
- T (1, "%-s", a->a0); /* { dg-warning "may write a terminating nul" } */
- T (1, "%-s", a->ax); /* { dg-warning "may write a terminating nul" } */
+ T (1, "%s", a->a0); /* { dg-warning "may write a terminating nul" } */
+ T (1, "%s", a->ax); /* { dg-warning "may write a terminating nul" } */
- T (2, "%-s", a->a0);
- T (2, "%-s", a->ax);
+ T (2, "%s", a->a0);
+ T (2, "%s", a->ax);
}
/* Exercise buffer overflow detection with non-const integer arguments. */
{
#define x x ()
- T (1, "%s", x ? "" : "1"); /* { dg-warning "nul past the end" } */
- T (1, "%s", x ? "1" : ""); /* { dg-warning "nul past the end" } */
- T (1, "%s", x ? s : "1"); /* { dg-warning "nul past the end" } */
- T (1, "%s", x ? "1" : s); /* { dg-warning "nul past the end" } */
+ T (1, "%-s", x ? "" : "1"); /* { dg-warning "nul past the end" } */
+ T (1, "%-s", x ? "1" : ""); /* { dg-warning "nul past the end" } */
+ T (1, "%-s", x ? s : "1"); /* { dg-warning "nul past the end" } */
+ T (1, "%-s", x ? "1" : s); /* { dg-warning "nul past the end" } */
/* When neither string is known no warning should be issued at level 1
since their lenghts are assumed to be zero. */