Re: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)

[Rainer Orth <ro at CeBiTec dot Uni-Bielefeld dot DE>]

Hi Martin,

>> as it happens, I'd already started bootstraps with your patch before
>> your mail arrived :-)
>
> Thanks for your help getting to the bottom of this!
>
>>
>> We're left with
>>
>> FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for excess errors)
>> FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-4.c (test for excess errors)
>>
>> for 32 bit and
>>
>> FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-4.c (test for excess errors)
>>
>> for 64 bit on both i386-pc-solaris2.12 and sparc-sun-solaris2.12.
>>
>> In the 32-bit builtin-sprintf-warn-1.c case, there are many instances of
>>
>> Excess errors:
>> /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c:224:3:
>> warning: format '%lc' expects argument of type 'wint_t', but argument 5
>> has type 'int' [-Wformat=]
>
> I've built the sparc-sun-solaris2.12 toolchain and reproduced these
> warnings.  They are vestiges of those I saw and some of which I fixed
> before.  The problem is that %lc expects a wint_t argument which on
> this target is an alias for long in but the argument of 0 has type
> int.  The warning is coming out of the -Wformat checker which doesn't
> seem to care that int and long have the same size.  I've committed
> r240758 that should fix the remaining warnings of this kind but long
> term I think GCC should change to avoid warning in this case (Clang
> doesn't).
>
>>
>> while the second is
>>
>> Excess errors:
>> /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-4.c:15:23:
>> warning: writing a terminating nul past the end of the destination
>> [-Wformat-length=]/vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-4.c:30:21:
>> warning: writing format character '4' at offset 3 past the end of the
>> destination [-Wformat-length=]
>> /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-4.c:46:21:
>> warning: writing format character '4' at offset 3 past the end of the
>> destination [-Wformat-length=]
>> /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-4.c:61:25:
>> warning: writing a terminating nul past the end of the destination
>> [-Wformat-length=]
>> /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-4.c:74:22:
>> warning: '%-s' directive writing 4 bytes into a region of size 1
>> [-Wformat-length=]
>>
>> I've no idea yet why in the first error message two different messages
>> are joined into one line.  Probably something with DejaGnu mangling the
>> output...
>
> I've reproduced this as well and it took me a while to see the
> problem.  It turns out that the target specifier I used in the
> test (*-*-*-*) happened to match my native target
> x86_64-pc-linux-gnu but not sparc-sun-solaris2.12.  Let me fix
> that in the next patch.  Hopefully with that all the remaining
> failures should clear up.
>
> Thanks again for your help and patience!

No worries: I've refreshed your patch on top of Thomas Preud'homme's for
PR testsuite/77710 and found that one more bit is needed to fix this
completely.  32-bit Solaris shows three more warnings:

/vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c:1355:3: warning: format '%lc' expects argument of type 'wint_t', but argument 6 has type 'int' [-Wformat=]
/vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c:1356:3: warning: format '%lc' expects argument of type 'wint_t', but argument 6 has type 'int' [-Wformat=]
/vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c:1357:3: warning: format '%lc' expects argument of type 'wint_t', but argument 6 has type 'int' [-Wformat=]

Fixed as follows:

# HG changeset patch
# Parent  1aaf616a61b8ea3ecff9313e059a1e85571cdde1
[testsuite] Fix 32-bit gcc.dg/tree-ssa/builtin-sprintf-warn-1.c on Solaris

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
@@ -1352,9 +1352,9 @@ void test_snprintf_chk_c_const (void)
   T (3, "%c_%c", '1', '2');      /* { dg-warning "output truncated" } */
 
   /* Wide characters.  */
-  T (0, "%lc",  0);
-  T (1, "%lc",  0);
-  T (2, "%lc",  0);
+  T (0, "%lc",  (wint_t)0);
+  T (1, "%lc",  (wint_t)0);
+  T (2, "%lc",  (wint_t)0);
 
   /* The following could result in as few as a single byte and in as many
      as MB_CUR_MAX, but since the MB_CUR_MAX value is a runtime property

With this one and your refreshed patch, all failures are gone now for
i386-pc-solaris2.12, sparc-sun-solaris2.12, and x86_64-pc-linux-gnu.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University