From 3e6837c2a56f47c6b3156bbc12a9e89611412f83 Mon Sep 17 00:00:00 2001 From: Martin Sebor Date: Fri, 14 Dec 2018 22:38:08 +0000 Subject: [PATCH] PR tree-optimization/87096 - Optimised snprintf is not POSIX conformant 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 | 7 + gcc/gimple-ssa-sprintf.c | 44 ++++- gcc/testsuite/ChangeLog | 5 + .../gcc.dg/tree-ssa/builtin-snprintf-4.c | 156 ++++++++++++++++++ 4 files changed, 205 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/builtin-snprintf-4.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 68f8f6d4193..6d0f1d50c11 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,10 @@ +2018-12-14 Martin Sebor + + 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 PR web/79738 diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c index 00485d9a073..52286a6da1c 100644 --- a/gcc/gimple-ssa-sprintf.c +++ b/gcc/gimple-ssa-sprintf.c @@ -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 %", - dstsize); + { + warning_at (gimple_location (info.callstmt), info.warnopt (), + "specified bound %wu exceeds %", + 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 " + "%", + 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) diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 9c24e473e5f..caa4f37db78 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2018-12-14 Martin Sebor + + PR tree-optimization/87096 + * gcc.dg/tree-ssa/builtin-snprintf-4.c: New test. + 2018-12-14 Alexandre Oliva 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 index 00000000000..5a6160881e3 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-snprintf-4.c @@ -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"} } */ -- 2.30.2