Bug 83431 - -Wformat-truncation may incorrectly report truncation
Summary: -Wformat-truncation may incorrectly report truncation
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 8.0
: P3 normal
Target Milestone: 10.0
Assignee: Martin Sebor
URL:
Keywords: diagnostic, missed-optimization, patch
Depends on:
Blocks:
 
Reported: 2017-12-14 18:32 UTC by Daniel Fruzynski
Modified: 2019-08-27 18:02 UTC (History)
5 users (show)

See Also:
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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Fruzynski 2017-12-14 18:32:06 UTC
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)
Comment 1 Martin Sebor 2017-12-14 22:05:03 UTC
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;

}
Comment 2 Martin Sebor 2017-12-14 22:06:18 UTC
This is GCC 9 material.
Comment 3 Matteo Croce 2019-05-20 18:12:59 UTC
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
Comment 4 Martin Sebor 2019-05-20 19:06:47 UTC
I'm working on the integration of the two passes for GCC 10.
Comment 5 Martin Sebor 2019-06-11 23:43:23 UTC
Initial patch: https://gcc.gnu.org/ml/gcc-patches/2019-06/msg00668.html
Comment 6 Martin Sebor 2019-08-26 18:30:25 UTC
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
Comment 7 Martin Sebor 2019-08-26 18:43:29 UTC
Done in r274933.  The change will not be backported.
Comment 8 Martin Sebor 2019-08-27 16:18:58 UTC
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