The call to __builtin___sprintf_chk in the following program overflows the destination. The overflow is detected at runtime and is clearly detectable at compile-time, but it's not diagnosed as it should be. $ cat t.c && gcc -Wall -Wextra -Wpedantic -Wrestrict -fdump-tree-optimized=/dev/stdout t.c && ./a.out char d[1]; int main (int argc, char *argv[]) { const char *s = argc < 0 ? "123" : "456"; int n = __builtin___sprintf_chk (d, 0, sizeof d, "%s", s); __builtin_printf ("%i: \"%.*s\"\n", n, n, d); } t.c: In function ‘main’: t.c:3:27: warning: unused parameter ‘argv’ [-Wunused-parameter] int main (int argc, char *argv[]) ^~~~ ;; Function main (main, funcdef_no=0, decl_uid=1797, cgraph_uid=0, symbol_order=1) main (int argc, char * * argv) { int n; const char * s; int D.1806; char[4] * iftmp.0; char[4] * iftmp.0_1; char[4] * iftmp.0_3; char[4] * iftmp.0_4; int _10; <bb 2> [0.00%]: if (argc_2(D) < 0) goto <bb 3>; [0.00%] else goto <bb 4>; [0.00%] <bb 3> [0.00%]: iftmp.0_4 = "123"; goto <bb 5>; [0.00%] <bb 4> [0.00%]: iftmp.0_3 = "456"; <bb 5> [0.00%]: # iftmp.0_1 = PHI <iftmp.0_4(3), iftmp.0_3(4)> s_5 = iftmp.0_1; n_8 = __builtin___sprintf_chk (&d, 0, 1, "%s", s_5); __builtin_printf ("%i: \"%.*s\"\n", n_8, n_8, &d); _10 = 0; <L3> [0.00%]: return _10; } *** buffer overflow detected ***: ./a.out terminated ======= Backtrace: ========= /lib64/libc.so.6(+0x77de5)[0x7f0975f73de5] /lib64/libc.so.6(__fortify_fail+0x37)[0x7f0976010167] /lib64/libc.so.6(+0x1122f0)[0x7f097600e2f0] /lib64/libc.so.6(+0x111859)[0x7f097600d859] /lib64/libc.so.6(_IO_default_xsputn+0x80)[0x7f0975f77bc0] /lib64/libc.so.6(_IO_vfprintf+0xaa6)[0x7f0975f48336] /lib64/libc.so.6(__vsprintf_chk+0x8c)[0x7f097600d8ec] /lib64/libc.so.6(__sprintf_chk+0x7d)[0x7f097600d83d] ./a.out[0x4005fb] /lib64/libc.so.6(__libc_start_main+0xf0)[0x7f0975f1c580] ./a.out[0x4004c9] ======= Memory map: ======== 00400000-00401000 r-xp 00000000 fd:03 8129719 /home/msebor/build/tmp/a.out 00600000-00601000 r--p 00000000 fd:03 8129719 /home/msebor/build/tmp/a.out 00601000-00602000 rw-p 00001000 fd:03 8129719 /home/msebor/build/tmp/a.out 00a07000-00a28000 rw-p 00000000 00:00 0 [heap] 7f0975ce5000-7f0975cfb000 r-xp 00000000 fd:00 4091 /usr/lib64/libgcc_s-5.3.1-20160406.so.1 7f0975cfb000-7f0975efa000 ---p 00016000 fd:00 4091 /usr/lib64/libgcc_s-5.3.1-20160406.so.1 7f0975efa000-7f0975efb000 r--p 00015000 fd:00 4091 /usr/lib64/libgcc_s-5.3.1-20160406.so.1 7f0975efb000-7f0975efc000 rw-p 00016000 fd:00 4091 /usr/lib64/libgcc_s-5.3.1-20160406.so.1 7f0975efc000-7f09760b3000 r-xp 00000000 fd:00 11035 /usr/lib64/libc-2.22.so 7f09760b3000-7f09762b3000 ---p 001b7000 fd:00 11035 /usr/lib64/libc-2.22.so 7f09762b3000-7f09762b7000 r--p 001b7000 fd:00 11035 /usr/lib64/libc-2.22.so 7f09762b7000-7f09762b9000 rw-p 001bb000 fd:00 11035 /usr/lib64/libc-2.22.so 7f09762b9000-7f09762bd000 rw-p 00000000 00:00 0 7f09762bd000-7f09762de000 r-xp 00000000 fd:00 73564 /usr/lib64/ld-2.22.so 7f09764c1000-7f09764c4000 rw-p 00000000 00:00 0 7f09764db000-7f09764dd000 rw-p 00000000 00:00 0 7f09764dd000-7f09764de000 r--p 00020000 fd:00 73564 /usr/lib64/ld-2.22.so 7f09764de000-7f09764df000 rw-p 00021000 fd:00 73564 /usr/lib64/ld-2.22.so 7f09764df000-7f09764e0000 rw-p 00000000 00:00 0 7ffd4b095000-7ffd4b0b6000 rw-p 00000000 00:00 0 [stack] 7ffd4b1a3000-7ffd4b1a5000 r--p 00000000 00:00 0 [vvar] 7ffd4b1a5000-7ffd4b1a7000 r-xp 00000000 00:00 0 [vdso] ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vsyscall] Aborted (core dumped)
The root cause is that the maybe_emit_sprintf_chk_warning function in builtins.c uses c_strlen() to compute the length of the string argument to the "%s" directive. The function should instead leave the length computation to the check_sizes() function which has the smarts to figure out the (minimum) length even from a non-literal argument. The following untested patch makes this change and allows the test case to be diagnosed. @ -9734,33 +9793,26 @@ maybe_emit_sprintf_chk_warning (tree exp, enum built_in_function fcode) if (!init_target_chars ()) return; + tree arg; /* If the format doesn't contain % args or %%, we know its size. */ if (strchr (fmt_str, target_percent) == 0) - len = build_int_cstu (size_type_node, strlen (fmt_str)); + arg = build_int_cstu (size_type_node, strlen (fmt_str)); /* If the format is "%s" and first ... argument is a string literal, we know it too. */ else if (fcode == BUILT_IN_SPRINTF_CHK && strcmp (fmt_str, target_percent_s) == 0) { - tree arg; - if (nargs < 5) return; arg = CALL_EXPR_ARG (exp, 4); if (! POINTER_TYPE_P (TREE_TYPE (arg))) return; - - len = c_strlen (arg, 1); - if (!len || ! tree_fits_uhwi_p (len)) - return; } else return; - /* Add one for the terminating nul. */ - len = fold_build2 (PLUS_EXPR, TREE_TYPE (len), len, size_one_node); check_sizes (OPT_Wstringop_overflow_, - exp, /*size=*/NULL_TREE, /*maxlen=*/NULL_TREE, len, size); + exp, /*size=*/NULL_TREE, /*maxlen=*/NULL_TREE, arg, size); } /* Emit warning if a free is called with address of a variable. */
GCC 10 issues: pr80074.c: In function ‘main’: pr80074.c:7:53: warning: ‘%s’ directive writing 3 bytes into a region of size 1 [-Wformat-overflow=] 7 | int n = __builtin___sprintf_chk (d, 0, sizeof d, "%s", s); | ^~ pr80074.c:7:11: note: ‘__builtin___sprintf_chk’ output 4 bytes into a destination of size 1 7 | int n = __builtin___sprintf_chk (d, 0, sizeof d, "%s", s); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ This was implemented in r240298: commit 88d0c3f0a1448e71dcf49c2f34909ec8d7ce348f Author: Martin Sebor <msebor@redhat.com> Date: Wed Sep 21 01:39:27 2016 +0000 PR middle-end/49905 - Better sanity checking on sprintf src & dest to gcc/ChangeLog: PR middle-end/49905 * Makefile.in (OBJS): Add gimple-ssa-sprintf.o. * config/linux.h (TARGET_PRINTF_POINTER_FORMAT): Redefine. * config/linux.c (gnu_libc_printf_pointer_format): New function. * config/sol2.h (TARGET_PRINTF_POINTER_FORMAT): Same. * config/sol2.c (solaris_printf_pointer_format): New function. * doc/invoke.texi (-Wformat-length, -fprintf-return-value): New options. * doc/tm.texi.in (TARGET_PRINTF_POINTER_FORMAT): Document. * doc/tm.texi: Regenerate. * gimple-fold.h (get_range_strlen): New function. (get_maxval_strlen): Declare existing function. * gimple-fold.c (get_range_strlen): Add arguments and compute both maximum and minimum. (get_range_strlen): Define overload. (get_maxval_strlen): Adjust. * gimple-ssa-sprintf.c: New file and pass. * passes.def (pass_sprintf_length): Add new pass. * targhooks.h (default_printf_pointer_format): Declare new function. (gnu_libc_printf_pointer_format): Same. (solaris_libc_printf_pointer_format): Same. * targhooks.c (default_printf_pointer_format): Define new function. * tree-pass.h (make_pass_sprintf_length): Declare new function. * print-tree.c: Increase buffer size. gcc/c-family/ChangeLog: PR middle-end/49905 * c.opt: Add -Wformat-length and -fprintf-return-value. gcc/testsuite/ChangeLog: PR middle-end/49905 * gcc.dg/builtin-stringop-chk-1.c: Adjust. * gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: New test. * gcc.dg/tree-ssa/builtin-sprintf-warn-2.c: New test. * gcc.dg/tree-ssa/builtin-sprintf-warn-3.c: New test. * gcc.dg/tree-ssa/builtin-sprintf-warn-4.c: New test. * gcc.dg/tree-ssa/builtin-sprintf.c: New test. * gcc.dg/tree-ssa/builtin-sprintf-2.c: New test. From-SVN: r240298