PR middle-end/77671 - missing -Wformat-overflow warning on sprintf overflow with %s

gcc/ChangeLog:

	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.

gcc/testsuite/ChangeLog:

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.

From-SVN: r248035
This commit is contained in:
Martin Sebor 2017-05-14 17:50:28 +00:00 committed by Martin Sebor
parent c6c0251911
commit a104bd8859
10 changed files with 370 additions and 81 deletions

View File

@ -1,3 +1,17 @@
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

View File

@ -2670,11 +2670,9 @@ gimple_fold_builtin_sprintf_chk (gimple_stmt_iterator *gsi,
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);
@ -2795,11 +2793,9 @@ gimple_fold_builtin_sprintf (gimple_stmt_iterator *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));
@ -3384,10 +3380,7 @@ gimple_fold_builtin (gimple_stmt_iterator *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:

View File

@ -54,6 +54,8 @@ extern tree gimple_get_virt_method_for_vtable (HOST_WIDE_INT, tree,
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 *);

View File

@ -526,7 +526,10 @@ get_format_string (tree format, location_t *ploc)
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;
@ -3511,6 +3514,60 @@ get_destination_size (tree dest)
return HOST_WIDE_INT_M1U;
}
/* 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
is_call_safe (const pass_sprintf_length::call_info &info,
const format_result &res, bool under4k,
unsigned HOST_WIDE_INT retval[2])
{
if (under4k && !res.under4k)
return false;
/* The minimum return value. */
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. */
retval[1]
= res.range.unlikely < res.range.max ? res.range.max : res.range.unlikely;
/* Adjust the number of bytes which includes the terminating nul
to reflect the return value of the function which does not.
Because the valid range of the function is [INT_MIN, INT_MAX],
a valid range before the adjustment below is [0, INT_MAX + 1]
(the functions only return negative values on error or undefined
behavior). */
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
more of output, or the return value exceeds INT_MAX, or because
the output overflows the destination object (but leave it enabled
when the function is bounded because then the behavior is well-
defined). */
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
@ -3529,42 +3586,18 @@ try_substitute_return_value (gimple_stmt_iterator *gsi,
/* Set to true when the entire call has been removed. */
bool removed = false;
/* The minimum return value. */
unsigned HOST_WIDE_INT minretval = res.range.min;
/* The minimum and maximum return value. */
unsigned HOST_WIDE_INT retval[2];
bool safe = is_call_safe (info, res, true, retval);
/* 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
= res.range.unlikely < res.range.max ? res.range.max : res.range.unlikely;
/* Adjust the number of bytes which includes the terminating nul
to reflect the return value of the function which does not.
Because the valid range of the function is [INT_MIN, INT_MAX],
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;
/* Avoid the return value optimization when the behavior of the call
is undefined either because any directive may have produced 4K or
more of output, or the return value exceeds INT_MAX, or because
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 (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)
@ -3612,18 +3645,18 @@ try_substitute_return_value (gimple_stmt_iterator *gsi,
{
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;
@ -3632,21 +3665,21 @@ try_substitute_return_value (gimple_stmt_iterator *gsi,
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]);
}
}
@ -3656,6 +3689,34 @@ try_substitute_return_value (gimple_stmt_iterator *gsi,
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. */
@ -3907,13 +3968,23 @@ pass_sprintf_length::handle_gimple_call (gimple_stmt_iterator *gsi)
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. */

View File

@ -1,3 +1,12 @@
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

View File

@ -0,0 +1,99 @@
/* 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;

View File

@ -0,0 +1,104 @@
/* 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;

View File

@ -1068,7 +1068,7 @@ void test_sprintf_chk_e_const (void)
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);

View File

@ -94,8 +94,8 @@ struct Arrays {
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" } */
@ -112,30 +112,27 @@ void test_s_nonconst (int w, int p, const char *s, const wchar_t *ws,
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. */

View File

@ -57,10 +57,10 @@ void test_sprintf_chk_string (const char *s, const char *t)
{
#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. */