| Summary: | -Wformat-truncation may incorrectly report truncation | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Daniel Fruzynski <bugzilla> |
| Component: | c++ | Assignee: | Martin Sebor <msebor> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | daniel.kruegler, dimhen, msebor, rootkit85, webrown.cpp |
| Priority: | P3 | Keywords: | diagnostic, missed-optimization, patch |
| Version: | 8.0 | ||
| Target Milestone: | 10.0 | ||
| See Also: |
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90917 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91567 |
||
| Host: | Target: | ||
| Build: | Known to work: | 10.0 | |
| Known to fail: | 8.3.0, 9.2.0 | Last reconfirmed: | 2017-12-14 00:00:00 |
That's right. String lengths are computed by and available only within the strlen pass, while sprintf/snprintf calls are processed by the sprintf pass. The two passes should probably be integrated somehow to benefit from each other's analysis. The example below shows where the integration would benefit in the strlen pass.
$ cat d.c && gcc -O2 -S -Wall -fdump-tree-optimized=/dev/stdout d.c
void f (char *d)
{
__builtin_sprintf (d, "%s", "123");
if (__builtin_strlen (d) != 3) // folded
__builtin_abort (); // eliminated
}
void g (char *d)
{
int n = __builtin_sprintf (d, "%i", 123);
if (n != 3) // folded
__builtin_abort (); // eliminated
}
void h (char *d)
{
__builtin_sprintf (d, "%i", 123);
if (__builtin_strlen (d) != 3) // not folded
__builtin_abort (); // not eliminated
}
;; Function f (f, funcdef_no=0, decl_uid=1892, cgraph_uid=0, symbol_order=0)
f (char * d)
{
<bb 2> [local count: 1073741825]:
__builtin_memcpy (d_3(D), "123", 4); [tail call]
return;
}
;; Function g (g, funcdef_no=1, decl_uid=1895, cgraph_uid=1, symbol_order=1)
g (char * d)
{
<bb 2> [local count: 1073741825]:
__builtin_sprintf (d_2(D), "%i", 123); [tail call]
return;
}
;; Function h (h, funcdef_no=2, decl_uid=1899, cgraph_uid=2, symbol_order=2)
h (char * d)
{
long unsigned int _1;
<bb 2> [local count: 1073741825]:
__builtin_sprintf (d_3(D), "%i", 123);
_1 = __builtin_strlen (d_3(D));
if (_1 != 3)
goto <bb 3>; [0.00%]
else
goto <bb 4>; [99.96%]
<bb 3> [count: 0]:
__builtin_abort ();
<bb 4> [local count: 1073312327]:
return;
}
This is GCC 9 material. I can reproduce it with this snippet:
void f()
{
const char *dir = "a";
const char file[50] = "b";
char buf[4];
snprintf(buf, sizeof(buf), "%s/%s", dir, file);
}
$ gcc -Wall -c buf.c
buf.c: In function ‘f’:
buf.c:8:33: warning: ‘%s’ directive output may be truncated writing up to 49 bytes into a region of size 3 [-Wformat-truncation=]
8 | snprintf(buf, sizeof(buf), "%s/%s", dir, file);
| ^~ ~~~~~~~~~~~
buf.c:8:2: note: ‘snprintf’ output 2 or more bytes (assuming 51) into a destination of size 4
8 | snprintf(buf, sizeof(buf), "%s/%s", dir, file);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
$ gcc -v
Using built-in specs.
COLLECT_GCC=/usr/bin/gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/9/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-redhat-linux
Configured with: ../configure --enable-bootstrap --enable-languages=c,c++,fortran,objc,obj-c++,ada,go,d,lto --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared --enable-threads=posix --enable-checking=release --enable-multilib --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-gcc-major-version-only --with-linker-hash-style=gnu --enable-plugin --enable-initfini-array --with-isl --enable-offload-targets=nvptx-none --without-cuda-driver --enable-gnu-indirect-function --enable-cet --with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux
Thread model: posix
gcc version 9.1.1 20190503 (Red Hat 9.1.1-1) (GCC)
Regards,
Matteo Croce
I'm working on the integration of the two passes for GCC 10. Initial patch: https://gcc.gnu.org/ml/gcc-patches/2019-06/msg00668.html Author: msebor Date: Mon Aug 26 18:29:45 2019 New Revision: 274933 URL: https://gcc.gnu.org/viewcvs?rev=274933&root=gcc&view=rev Log: PR tree-optimization/83431 - -Wformat-truncation may incorrectly report truncation gcc/ChangeLog: PR c++/83431 * gimple-ssa-sprintf.c (pass_data_sprintf_length): Remove object. (sprintf_dom_walker): Remove class. (get_int_range): Make argument const. (directive::fmtfunc, directive::set_precision): Same. (format_none): Same. (build_intmax_type_nodes): Same. (adjust_range_for_overflow): Same. (format_floating): Same. (format_character): Same. (format_string): Same. (format_plain): Same. (get_int_range): Cast away constness. (format_integer): Same. (get_string_length): Call get_range_strlen_dynamic. Handle null lendata.maxbound. (should_warn_p): Adjust argument scope qualifier. (maybe_warn): Same. (format_directive): Same. (parse_directive): Same. (is_call_safe): Same. (try_substitute_return_value): Same. (sprintf_dom_walker::handle_printf_call): Rename... (handle_printf_call): ...to this. Initialize target to host charmap here instead of in pass_sprintf_length::execute. (struct call_info): Make global. (sprintf_dom_walker::compute_format_length): Make global. (sprintf_dom_walker::handle_gimple_call): Same. * passes.def (pass_sprintf_length): Replace with pass_strlen. * print-rtl.c (print_pattern): Reduce the number of spaces to avoid -Wformat-truncation. * tree-pass.h (make_pass_warn_printf): New function. * tree-ssa-strlen.c (strlen_optimize): New variable. (get_string_length): Add comments. (get_range_strlen_dynamic): New function. (check_and_optimize_call): New function. (handle_integral_assign): New function. (strlen_check_and_optimize_stmt): Factor code out into strlen_check_and_optimize_call and handle_integral_assign. (strlen_dom_walker::evrp): New member. (strlen_dom_walker::before_dom_children): Use evrp member. (strlen_dom_walker::after_dom_children): Use evrp member. (printf_strlen_execute): New function. (pass_strlen::gate): Update to handle printf calls. (dump_strlen_info): New function. (pass_data_warn_printf): New variable. (pass_warn_printf): New class. * tree-ssa-strlen.h (get_range_strlen_dynamic): Declare. (handle_printf_call): Same. * tree-vrp.c (value_range_base::type): Adjust assertion. * vr-values.c (vr_values::update_value_range): Use type of the first argument rather than the second. gcc/testsuite/ChangeLog: PR c++/83431 * gcc.dg/strlenopt-63.c: New test. * gcc.dg/pr79538.c: Adjust text of expected warning. * gcc.dg/pr81292-1.c: Adjust pass name. * gcc.dg/pr81292-2.c: Same. * gcc.dg/pr81703.c: Same. * gcc.dg/strcmpopt_2.c: Same. * gcc.dg/strcmpopt_3.c: Same. * gcc.dg/strcmpopt_4.c: Same. * gcc.dg/strlenopt-1.c: Same. * gcc.dg/strlenopt-10.c: Same. * gcc.dg/strlenopt-11.c: Same. * gcc.dg/strlenopt-13.c: Same. * gcc.dg/strlenopt-14g.c: Same. * gcc.dg/strlenopt-14gf.c: Same. * gcc.dg/strlenopt-15.c: Same. * gcc.dg/strlenopt-16g.c: Same. * gcc.dg/strlenopt-17g.c: Same. * gcc.dg/strlenopt-18g.c: Same. * gcc.dg/strlenopt-19.c: Same. * gcc.dg/strlenopt-1f.c: Same. * gcc.dg/strlenopt-2.c: Same. * gcc.dg/strlenopt-20.c: Same. * gcc.dg/strlenopt-21.c: Same. * gcc.dg/strlenopt-22.c: Same. * gcc.dg/strlenopt-22g.c: Same. * gcc.dg/strlenopt-24.c: Same. * gcc.dg/strlenopt-25.c: Same. * gcc.dg/strlenopt-26.c: Same. * gcc.dg/strlenopt-27.c: Same. * gcc.dg/strlenopt-28.c: Same. * gcc.dg/strlenopt-29.c: Same. * gcc.dg/strlenopt-2f.c: Same. * gcc.dg/strlenopt-3.c: Same. * gcc.dg/strlenopt-30.c: Same. * gcc.dg/strlenopt-31g.c: Same. * gcc.dg/strlenopt-32.c: Same. * gcc.dg/strlenopt-33.c: Same. * gcc.dg/strlenopt-33g.c: Same. * gcc.dg/strlenopt-34.c: Same. * gcc.dg/strlenopt-35.c: Same. * gcc.dg/strlenopt-4.c: Same. * gcc.dg/strlenopt-48.c: Same. * gcc.dg/strlenopt-49.c: Same. * gcc.dg/strlenopt-4g.c: Same. * gcc.dg/strlenopt-4gf.c: Same. * gcc.dg/strlenopt-5.c: Same. * gcc.dg/strlenopt-50.c: Same. * gcc.dg/strlenopt-51.c: Same. * gcc.dg/strlenopt-52.c: Same. * gcc.dg/strlenopt-53.c: Same. * gcc.dg/strlenopt-54.c: Same. * gcc.dg/strlenopt-55.c: Same. * gcc.dg/strlenopt-56.c: Same. * gcc.dg/strlenopt-6.c: Same. * gcc.dg/strlenopt-61.c: Same. * gcc.dg/strlenopt-7.c: Same. * gcc.dg/strlenopt-8.c: Same. * gcc.dg/strlenopt-9.c: Same. * gcc.dg/strlenopt.h (snprintf, snprintf): Declare. * gcc.dg/tree-ssa/builtin-snprintf-6.c: New test. * gcc.dg/tree-ssa/builtin-snprintf-7.c: New test. * gcc.dg/tree-ssa/builtin-snprintf-8.c: New test. * gcc.dg/tree-ssa/builtin-snprintf-9.c: New test. * gcc.dg/tree-ssa/builtin-sprintf-warn-21.c: New test. * gcc.dg/tree-ssa/dump-4.c: New test. * gcc.dg/tree-ssa/pr83501.c: Adjust pass name. Added: trunk/gcc/testsuite/gcc.dg/strlenopt-68.c trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-snprintf-6.c trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-snprintf-7.c trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-snprintf-8.c trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-snprintf-9.c trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-snprintf-warn-5.c trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-21.c trunk/gcc/testsuite/gcc.dg/tree-ssa/dump-4.c Modified: trunk/gcc/ChangeLog trunk/gcc/gimple-ssa-sprintf.c trunk/gcc/passes.def trunk/gcc/print-rtl.c trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.dg/pr79538.c trunk/gcc/testsuite/gcc.dg/pr81292-1.c trunk/gcc/testsuite/gcc.dg/pr81292-2.c trunk/gcc/testsuite/gcc.dg/pr81703.c trunk/gcc/testsuite/gcc.dg/strcmpopt_2.c trunk/gcc/testsuite/gcc.dg/strcmpopt_3.c trunk/gcc/testsuite/gcc.dg/strcmpopt_4.c trunk/gcc/testsuite/gcc.dg/strlenopt-1.c trunk/gcc/testsuite/gcc.dg/strlenopt-10.c trunk/gcc/testsuite/gcc.dg/strlenopt-11.c trunk/gcc/testsuite/gcc.dg/strlenopt-13.c trunk/gcc/testsuite/gcc.dg/strlenopt-14g.c trunk/gcc/testsuite/gcc.dg/strlenopt-14gf.c trunk/gcc/testsuite/gcc.dg/strlenopt-15.c trunk/gcc/testsuite/gcc.dg/strlenopt-16g.c trunk/gcc/testsuite/gcc.dg/strlenopt-17g.c trunk/gcc/testsuite/gcc.dg/strlenopt-18g.c trunk/gcc/testsuite/gcc.dg/strlenopt-19.c trunk/gcc/testsuite/gcc.dg/strlenopt-1f.c trunk/gcc/testsuite/gcc.dg/strlenopt-2.c trunk/gcc/testsuite/gcc.dg/strlenopt-20.c trunk/gcc/testsuite/gcc.dg/strlenopt-21.c trunk/gcc/testsuite/gcc.dg/strlenopt-22.c trunk/gcc/testsuite/gcc.dg/strlenopt-22g.c trunk/gcc/testsuite/gcc.dg/strlenopt-24.c trunk/gcc/testsuite/gcc.dg/strlenopt-25.c trunk/gcc/testsuite/gcc.dg/strlenopt-26.c trunk/gcc/testsuite/gcc.dg/strlenopt-27.c trunk/gcc/testsuite/gcc.dg/strlenopt-28.c trunk/gcc/testsuite/gcc.dg/strlenopt-29.c trunk/gcc/testsuite/gcc.dg/strlenopt-2f.c trunk/gcc/testsuite/gcc.dg/strlenopt-3.c trunk/gcc/testsuite/gcc.dg/strlenopt-30.c trunk/gcc/testsuite/gcc.dg/strlenopt-31g.c trunk/gcc/testsuite/gcc.dg/strlenopt-32.c trunk/gcc/testsuite/gcc.dg/strlenopt-33.c trunk/gcc/testsuite/gcc.dg/strlenopt-33g.c trunk/gcc/testsuite/gcc.dg/strlenopt-34.c trunk/gcc/testsuite/gcc.dg/strlenopt-35.c trunk/gcc/testsuite/gcc.dg/strlenopt-4.c trunk/gcc/testsuite/gcc.dg/strlenopt-48.c trunk/gcc/testsuite/gcc.dg/strlenopt-49.c trunk/gcc/testsuite/gcc.dg/strlenopt-4g.c trunk/gcc/testsuite/gcc.dg/strlenopt-4gf.c trunk/gcc/testsuite/gcc.dg/strlenopt-5.c trunk/gcc/testsuite/gcc.dg/strlenopt-50.c trunk/gcc/testsuite/gcc.dg/strlenopt-51.c trunk/gcc/testsuite/gcc.dg/strlenopt-52.c trunk/gcc/testsuite/gcc.dg/strlenopt-53.c trunk/gcc/testsuite/gcc.dg/strlenopt-54.c trunk/gcc/testsuite/gcc.dg/strlenopt-55.c trunk/gcc/testsuite/gcc.dg/strlenopt-56.c trunk/gcc/testsuite/gcc.dg/strlenopt-6.c trunk/gcc/testsuite/gcc.dg/strlenopt-61.c trunk/gcc/testsuite/gcc.dg/strlenopt-7.c trunk/gcc/testsuite/gcc.dg/strlenopt-9.c trunk/gcc/testsuite/gcc.dg/strlenopt.h trunk/gcc/testsuite/gcc.dg/tree-ssa/pr83501.c trunk/gcc/testsuite/gcc.dg/tree-ssa/strlen-2.c trunk/gcc/tree-pass.h trunk/gcc/tree-ssa-strlen.c trunk/gcc/tree-ssa-strlen.h trunk/gcc/tree-vrp.c trunk/gcc/vr-values.c Author: msebor Date: Tue Aug 27 16:18:27 2019 New Revision: 274961 URL: https://gcc.gnu.org/viewcvs?rev=274961&root=gcc&view=rev Log: gcc/testsuite/ChangeLog: PR c++/83431 PR testsuite/91562 * gcc.dg/strlenopt-8.c: Adjust pass/dump name. Modified: trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.dg/strlenopt-8.c |
This looks like another missing optimization - -Wformat-truncation does not take into account that there is "if" which checks that truncation will not happen. [code] #include <stdio.h> #include <string.h> struct S { char str[20]; char out[10]; }; void test(S* s) { if (strlen(s->str) < sizeof(s->out) - 2) snprintf(s->out, sizeof(s->out), "[%s]", s->str); } [/code] [out] $ g++ -c -o test.o test.cc -O2 -Wall test.cc: In function ‘void test(S*)’: test.cc:10:6: warning: ‘%s’ directive output may be truncated writing up to 19 bytes into a region of size 9 [-Wformat-truncation=] void test(S* s) ^~~~ test.cc:13:17: note: ‘snprintf’ output between 3 and 22 bytes into a destination of size 10 snprintf(s->out, sizeof(s->out), "[%s]", s->str); ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [/out] g++ --version g++ (GCC) 8.0.0 20171210 (experimental)