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

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:

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

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:

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

On Thu, 6 Oct 2016, Martin Sebor wrote: > > The problem is that extracting via an integer of the opposite signedness > > is *not* defined unless the argument has a value representable in both > > types - both as a matter of ISO C rules on variadic functions, and as a > > practical matter of what

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

On 10/05/2016 05:54 PM, Joseph Myers wrote: On Wed, 5 Oct 2016, Martin Sebor wrote: On 10/05/2016 05:11 PM, Joseph Myers wrote: On Wed, 5 Oct 2016, Martin Sebor wrote: may have been subjected to. Issuing a warning for a safe piece of code only on the basis that there might be some other

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

On Wed, 5 Oct 2016, Martin Sebor wrote: > On 10/05/2016 05:11 PM, Joseph Myers wrote: > > On Wed, 5 Oct 2016, Martin Sebor wrote: > > > > > may have been subjected to. Issuing a warning for a safe piece > > > of code only on the basis that there might be some other piece > > > of code in the

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

On 10/05/2016 05:11 PM, Joseph Myers wrote: On Wed, 5 Oct 2016, Martin Sebor wrote: may have been subjected to. Issuing a warning for a safe piece of code only on the basis that there might be some other piece of code in the program that does something wrong makes no sense. Suggesting to the

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

On Wed, 5 Oct 2016, Martin Sebor wrote: > may have been subjected to. Issuing a warning for a safe piece > of code only on the basis that there might be some other piece > of code in the program that does something wrong makes no sense. > Suggesting to the user to change the safe piece of code

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

The problem is e.g. code calling getwchar and not checking for WEOF. 0xu does not fit in a 32-bit signed variable unless your code is confusing signed and unsigned freely. And even if you consider it OK to check after conversion to wchar_t rather than before, the lack of a wint_t

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

On Wed, 5 Oct 2016, Martin Sebor wrote: > > Or it could show up a logical error where the code should have had a > > wint_t variable and checked for WEOF somewhere but just assumed things > > would fit in wchar_t > > WEOF that fits in a wint_t also fits in a wchar_t when the two types > have the

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

On 10/05/2016 10:27 AM, Joseph Myers wrote: On Wed, 5 Oct 2016, Martin Sebor wrote: The warning would only helpful if there was a target where wchar_t didn't promote to a type with the same precision as wint_t, or in Or it could show up a logical error where the code should have had a wint_t

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

On Wed, 5 Oct 2016, Martin Sebor wrote: > The warning would only helpful if there was a target where wchar_t > didn't promote to a type with the same precision as wint_t, or in Or it could show up a logical error where the code should have had a wint_t variable and checked for WEOF somewhere

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

On 10/05/2016 09:40 AM, Joseph Myers wrote: On Tue, 4 Oct 2016, Martin Sebor wrote: Well, typically cases where one of long and int is passed and the other is expected, but they have the same size, are bugs waiting to happen when the code is built on a 64-bit system. That is, they *should*

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

On Wed, 5 Oct 2016, Jakub Jelinek wrote: > On Wed, Oct 05, 2016 at 03:40:18PM +, Joseph Myers wrote: > > On Tue, 4 Oct 2016, Martin Sebor wrote: > > > > > > Well, typically cases where one of long and int is passed and the other > > > > is > > > > expected, but they have the same size, are

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

On Wed, Oct 05, 2016 at 03:40:18PM +, Joseph Myers wrote: > On Tue, 4 Oct 2016, Martin Sebor wrote: > > > > Well, typically cases where one of long and int is passed and the other is > > > expected, but they have the same size, are bugs waiting to happen when the > > > code is built on a

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

On Tue, 4 Oct 2016, Martin Sebor wrote: > > Well, typically cases where one of long and int is passed and the other is > > expected, but they have the same size, are bugs waiting to happen when the > > code is built on a 64-bit system. That is, they *should* warn. > > Typically, yes. In the

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

On 10/04/2016 06:21 PM, Joseph Myers wrote: On Tue, 4 Oct 2016, Martin Sebor wrote: 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

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

On Tue, 4 Oct 2016, Martin Sebor wrote: > 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

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

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

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

Hi Martin, +FAIL: gcc.dg/tree-ssa/builtin-sprintf.c execution test FAIL: test_a_double:364: "%a" expected result for "0x0.0p+0" doesn't match function call return value: 20 != 6 FAIL: test_a_double:365: "%a" expected result for "0x1.0p+0"

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

+FAIL: gcc.dg/tree-ssa/builtin-sprintf.c execution test FAIL: test_a_double:364: "%a" expected result for "0x0.0p+0" doesn't match function call return value: 20 != 6 FAIL: test_a_double:365: "%a" expected result for "0x1.0p+0" doesn't match function call return value: 20

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

On 09/24/2016 10:39 AM, Andreas Schwab wrote: I'm still seeing these failures on m68k: FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for warnings, line 358) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for warnings, line 1222) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c

Re: [BUILDROBOT] tic6x-uclinux: undefined reference to `gnu_libc_printf_pointer_format(tree_node*, char const**)' (was: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905))

On Tue, Sep 27, 2016 at 12:08:46AM +0200, Jan-Benedict Glaw wrote: > Hi Martin, > > On Thu, 2016-09-08 13:03:12 -0600, Martin Sebor wrote: > > Attached is another update to the patch to address the last round > > of comments and suggestions, most notably to: > [...] > > with

[BUILDROBOT] tic6x-uclinux: undefined reference to `gnu_libc_printf_pointer_format(tree_node*, char const**)' (was: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905))

Hi Martin, On Thu, 2016-09-08 13:03:12 -0600, Martin Sebor wrote: > Attached is another update to the patch to address the last round > of comments and suggestions, most notably to: [...] with the currently committed version, the tic6x-uclinux target fails, see ie.

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

I'm still seeing these failures on m68k: FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for warnings, line 358) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for warnings, line 1222) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for warnings, line 1272) FAIL:

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

Hi Martin, > On 09/22/2016 06:14 AM, Rainer Orth wrote: >> Hi Martin, >> >>> your patch broke bootstrap with MPFR 2.4.2, which is still the >>> recommended (or perhaps minimal) version according to install.texi: >> [...] >>> The following patch (together with your other one to fix ILP32 targets)

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

On 09/22/2016 06:14 AM, Rainer Orth wrote: Hi Martin, your patch broke bootstrap with MPFR 2.4.2, which is still the recommended (or perhaps minimal) version according to install.texi: [...] The following patch (together with your other one to fix ILP32 targets) allows a

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

your patch broke bootstrap with MPFR 2.4.2, which is still the recommended (or perhaps minimal) version according to install.texi: /vol/gcc/src/hg/trunk/local/gcc/gimple-ssa-sprintf.c: In function 'int {anonymous}::format_floating_max(tree, char)':

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

Hi Martin, > your patch broke bootstrap with MPFR 2.4.2, which is still the > recommended (or perhaps minimal) version according to install.texi: [...] > The following patch (together with your other one to fix ILP32 targets) > allows a sparc-sun-solaris2.12 bootstrap to continue. I'm going to >

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

Hi Martin, >> Another nit, if I may: FWIW I'm not in love with the wording of the >> messages. Sorry to bikeshed, but how about: >> warning: buffer overflow will occur when writing terminating NUL >> and: >> note: formatted output of 2 bytes into a destination of size 1 >> or somesuch. > > I

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

On Wed, 21 Sep 2016, Martin Sebor wrote: >> Is it possible that is related to your warning patches? > Yes, this is likely the same bug as mentioned in comment #6 on > pr77676. The bug in the comment ILP32-specific and only tangentially > related to the PR itself. I'm testing the patch that's

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

On Wed, Sep 21, 2016 at 01:55:33PM -0600, Martin Sebor wrote: > On 09/21/2016 01:40 PM, Gerald Pfeifer wrote: > >I noticed the following bootstrap failure on i?86-unknown-freebsd > >that started in the last 24 hours: > > > >/scratch/tmp/gerald/gcc-HEAD/gcc/vec.c: In member function ‘void >

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

On 09/21/2016 01:40 PM, Gerald Pfeifer wrote: I noticed the following bootstrap failure on i?86-unknown-freebsd that started in the last 24 hours: /scratch/tmp/gerald/gcc-HEAD/gcc/vec.c: In member function ‘void vec_usage::dump(mem_location*, mem_usage&) const’:

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

I noticed the following bootstrap failure on i?86-unknown-freebsd that started in the last 24 hours: /scratch/tmp/gerald/gcc-HEAD/gcc/vec.c: In member function ‘void vec_usage::dump(mem_location*, mem_usage&) const’: /scratch/tmp/gerald/gcc-HEAD/gcc/vec.c:79:3: error: ‘%s’ directive writing

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

On 09/21/2016 03:59 PM, Szabolcs Nagy wrote: > On 21/09/16 15:37, Alexander Monakov wrote: >> On Wed, 21 Sep 2016, Martin Sebor wrote: >>> On 09/21/2016 01:11 AM, Markus Trippelsdorf wrote: The patch uses "nul" instead of "null" throughout. >>> >>> Yes, that's intentional. NUL and null are

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

On 09/21/2016 01:11 AM, Markus Trippelsdorf wrote: On 2016.09.11 at 20:03 -0600, Martin Sebor wrote: On 09/08/2016 04:10 PM, Joseph Myers wrote: On Thu, 8 Sep 2016, Martin Sebor wrote: PR middle-end/49905 - Better sanity checking on sprintf src & dest to produce warning for dodgy

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

On 09/21/2016 08:37 AM, Alexander Monakov wrote: On Wed, 21 Sep 2016, Martin Sebor wrote: On 09/21/2016 01:11 AM, Markus Trippelsdorf wrote: The patch uses "nul" instead of "null" throughout. Yes, that's intentional. NUL and null are alternate spellings for the same character. I went with

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

On 21/09/16 15:37, Alexander Monakov wrote: > On Wed, 21 Sep 2016, Martin Sebor wrote: >> On 09/21/2016 01:11 AM, Markus Trippelsdorf wrote: >>> The patch uses "nul" instead of "null" throughout. >> >> Yes, that's intentional. NUL and null are alternate spellings for >> the same character. I went

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

On Wed, 21 Sep 2016, Martin Sebor wrote: > On 09/21/2016 01:11 AM, Markus Trippelsdorf wrote: > > The patch uses "nul" instead of "null" throughout. > > Yes, that's intentional. NUL and null are alternate spellings for > the same character. I went with nul to distinguish it from the null >

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

On 09/21/2016 01:11 AM, Markus Trippelsdorf wrote: On 2016.09.11 at 20:03 -0600, Martin Sebor wrote: On 09/08/2016 04:10 PM, Joseph Myers wrote: On Thu, 8 Sep 2016, Martin Sebor wrote: PR middle-end/49905 - Better sanity checking on sprintf src & dest to produce warning for dodgy

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

On 09/21/2016 04:36 AM, Christophe Lyon wrote: Hi Martin, On 21 September 2016 at 09:11, Markus Trippelsdorf wrote: On 2016.09.11 at 20:03 -0600, Martin Sebor wrote: On 09/08/2016 04:10 PM, Joseph Myers wrote: On Thu, 8 Sep 2016, Martin Sebor wrote: PR

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

Hi Martin, On 21 September 2016 at 09:11, Markus Trippelsdorf wrote: > On 2016.09.11 at 20:03 -0600, Martin Sebor wrote: >> On 09/08/2016 04:10 PM, Joseph Myers wrote: >> > On Thu, 8 Sep 2016, Martin Sebor wrote: > >> PR middle-end/49905 - Better sanity checking on

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

On 2016.09.11 at 20:03 -0600, Martin Sebor wrote: > On 09/08/2016 04:10 PM, Joseph Myers wrote: > > On Thu, 8 Sep 2016, Martin Sebor wrote: > PR middle-end/49905 - Better sanity checking on sprintf src & dest to > produce warning for dodgy code The patch uses "nul" instead of "null"

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

On 09/20/2016 01:38 PM, David Malcolm wrote: On Tue, 2016-09-20 at 13:35 -0600, Martin Sebor wrote: [...] +#if 0 ... +#else ... +#endif Please remove the #if 0'd code. Sorry about that. This was left over from my effort to persuade the substring_loc class to underline just the last

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

On Tue, 2016-09-20 at 13:35 -0600, Martin Sebor wrote: [...] > > > +#if 0 > ... > > > +#else > ... > > > +#endif > > Please remove the #if 0'd code. > > Sorry about that. This was left over from my effort to persuade > the substring_loc class to underline just the last quote of the > format

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

+ /* The minimum and maximum length. The MAXLEN pointer stays unchanged + but MINLEN may be cleared during the execution of the function. */ + tree *minlen = length; + tree* const maxlen = length + 1; Check formatting here. I'm not sure if the formatting standards explicitly state how

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

David, in the way of feedback, I spent hours debugging the simple multiline test I added. It seems that DejaGnu is exquisitely sensitive to whitespaces in the multiline output. I appreciate that spacing is essential to lining up the caret and the tildes with the text of the program but tests

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

On Sun, 2016-09-11 at 20:03 -0600, Martin Sebor wrote: > On 09/08/2016 04:10 PM, Joseph Myers wrote: > > On Thu, 8 Sep 2016, Martin Sebor wrote: > > > > > diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in > > > index da133a4..4607495 100644 > > > --- a/gcc/doc/tm.texi.in > > > +++

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

On 09/11/2016 08:03 PM, Martin Sebor wrote: Attached is revision 8 of the patch (hopefully) adjusted as requested above, and with a simple test with of the multiline diagnostic directives suggested by David. This revision also enables the -fprintf-return-value option by default. The libgomp

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

On 09/08/2016 01:03 PM, Martin Sebor wrote: Attached is another update to the patch to address the last round of comments and suggestions, most notably to: * implement the checking of the implementation limit of 4,095 on the output of a single directive to allow for the Glibc failure

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

On Mon, 12 Sep 2016, Martin Sebor wrote: > On 09/12/2016 04:08 PM, Joseph Myers wrote: > > On Sun, 11 Sep 2016, Martin Sebor wrote: > > > > > Attached is revision 8 of the patch (hopefully) adjusted as > > > requested above, and with a simple test with of the multiline > > > > The ChangeLog

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

On 09/12/2016 04:08 PM, Joseph Myers wrote: On Sun, 11 Sep 2016, Martin Sebor wrote: Attached is revision 8 of the patch (hopefully) adjusted as requested above, and with a simple test with of the multiline The ChangeLog appears not to have been updated (at least, it doesn't mention the

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

On Sun, 11 Sep 2016, Martin Sebor wrote: > Attached is revision 8 of the patch (hopefully) adjusted as > requested above, and with a simple test with of the multiline The ChangeLog appears not to have been updated (at least, it doesn't mention the changes to target.def). -- Joseph S. Myers

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

Patch #1. All the fixes to static buffer sizes that were inspired by your warning. These are all approved and can go in immediately. Attached is this patch. Hi, this patch changed the file gcc/go/gofrontend/expressions.cc. As described in gcc/go/gofrontend/README, the files in the

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

On Fri, Aug 19, 2016 at 8:29 AM, Martin Sebor wrote: >> My biggest concern with this iteration is the tight integration between >> the optimization and warning. We generally avoid that kind of tight >> integration such that enabling the warning does not affect the >>

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

On Thu, 8 Sep 2016, Martin Sebor wrote: > diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in > index da133a4..4607495 100644 > --- a/gcc/doc/tm.texi.in > +++ b/gcc/doc/tm.texi.in > @@ -4081,6 +4081,13 @@ In either case, it remains possible to select > code-generation for the alternate >

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

On 09/08/2016 01:45 PM, David Malcolm wrote: On Thu, 2016-09-08 at 13:03 -0600, Martin Sebor wrote: Attached is another update to the patch to address the last round of comments and suggestions, most notably to: * implement the checking of the implementation limit of 4,095 on the

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

On Thu, 2016-09-08 at 13:03 -0600, Martin Sebor wrote: > Attached is another update to the patch to address the last round > of comments and suggestions, most notably to: > > * implement the checking of the implementation limit of 4,095 on > the output of a single directive to allow for

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

On 09/08/2016 01:21 PM, Jeff Law wrote: On 08/24/2016 10:40 AM, Martin Sebor wrote: On 08/23/2016 05:00 PM, Joseph Myers wrote: Some observations: * Does -fprintf-return-value allow for the possibility of snprintf failing because of a memory allocation failure and so returning -1 when GCC

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

On 08/24/2016 10:40 AM, Martin Sebor wrote: On 08/23/2016 05:00 PM, Joseph Myers wrote: Some observations: * Does -fprintf-return-value allow for the possibility of snprintf failing because of a memory allocation failure and so returning -1 when GCC computed bounds on what it could return if

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

On 08/24/2016 05:14 PM, Manuel López-Ibáñez wrote: I agree. The challenge is that not all the bits this depends on (the g_string_concat_db and parse_in globals defined in the front end) are available in the middle end. I've been talking to David Malcolm about how best to factor things out of

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

On Wed, 24 Aug 2016, Martin Sebor wrote: > Since it's presumably a Glibc bug (bug 2509?) is this something > you believe the optimization needs to worry about? If so, can > you confirm that only Glibc versions 2.4 and prior are affected > and 2.5 is not? The code may not need to care about it,

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

>> --Wno-format-contains-nul -Wno-format-extra-args -Wformat-nonliteral @gol >> +-Wno-format-contains-nul -Wno-format-extra-args -Wformat-length=1 @gol >> >> Most options that take levels are documented as: >> >> -Wshift-overflow -Wshift-overflow=@var{n} >> -Wstrict-overflow

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

On 08/24/2016 04:03 PM, Joseph Myers wrote: On Wed, 24 Aug 2016, Martin Sebor wrote: No. I recall having seen Glibc fail with ENOMEM years ago when formatting a floating point number to a very large precision but I haven't seen any implementation fail. I haven't yet looked to see if the

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

On Wed, 24 Aug 2016, Martin Sebor wrote: > No. I recall having seen Glibc fail with ENOMEM years ago when > formatting a floating point number to a very large precision but > I haven't seen any implementation fail. I haven't yet looked to > see if the Glibc failure can still happen. My reading

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

On 08/24/2016 06:40 PM, Martin Sebor wrote: On 08/23/2016 05:00 PM, Joseph Myers wrote: Some observations: * Does -fprintf-return-value allow for the possibility of snprintf failing because of a memory allocation failure and so returning -1 when GCC computed bounds on what it could return if

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

On 08/23/2016 05:00 PM, Joseph Myers wrote: Some observations: * Does -fprintf-return-value allow for the possibility of snprintf failing because of a memory allocation failure and so returning -1 when GCC computed bounds on what it could return if successful? No. I recall having seen Glibc

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

On 23 August 2016 at 22:56, Martin Sebor wrote: > Attached is the latest patch. Some suggestions: --Wno-format-contains-nul -Wno-format-extra-args -Wformat-nonliteral @gol +-Wno-format-contains-nul -Wno-format-extra-args -Wformat-length=1 @gol Most options that take levels

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

Some observations: * Does -fprintf-return-value allow for the possibility of snprintf failing because of a memory allocation failure and so returning -1 when GCC computed bounds on what it could return if successful? * It looks like you convert to (signed/unsigned) char for %hh formats, etc.

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

On 08/20/2016 05:18 AM, Trevor Saunders wrote: Patch #5 and beyond: Further optimization work. As one of the next steps I'd like to make this feature available to user-defined sprintf-like functions decorated with attribute format. To do that, I'm thinking of adding either a fourth (optional)

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

> > > Patch #5 and beyond: Further optimization work. > > > > As one of the next steps I'd like to make this feature available > > to user-defined sprintf-like functions decorated with attribute > > format. To do that, I'm thinking of adding either a fourth > > (optional) argument to attribute

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

On 08/19/2016 09:29 AM, Martin Sebor wrote: My biggest concern with this iteration is the tight integration between the optimization and warning. We generally avoid that kind of tight integration such that enabling the warning does not affect the optimization and vice-versa. So ISTM you have

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

My biggest concern with this iteration is the tight integration between the optimization and warning. We generally avoid that kind of tight integration such that enabling the warning does not affect the optimization and vice-versa. So ISTM you have to do the analysis if the optimization or

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

On 08/11/2016 08:13 PM, Martin Sebor wrote: Attached is an updated patch with changes addressing most of the suggestions and comments I received. The main changes/improvements are the following: * New option, -fprintf-return-value, enables the sprintf return value optimization. It's

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

On 08/12/2016 09:48 AM, Joseph Myers wrote: On Thu, 11 Aug 2016, Martin Sebor wrote: * New target hooks remove hardcoding target-specific assumptions about libc implementation-specific details (%p format and printf floating point rounding mode). But the rounding mode may vary at

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

On Thu, 11 Aug 2016, Martin Sebor wrote: > * New target hooks remove hardcoding target-specific assumptions > about libc implementation-specific details (%p format and printf > floating point rounding mode). But the rounding mode may vary at runtime; optimally the conversions should

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

On 07/22/2016 04:12 PM, Jeff Law wrote: Working through the new pass... Overall it looks pretty good. There's a certain level of trust I'll extend WRT getting the low level details right -- a thorough testsuite obviously helps there. In the latest patch where I add the return value

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

On 07/29/2016 04:50 PM, Joseph Myers wrote: On Mon, 18 Jul 2016, Martin Sebor wrote: + /* Number of exponent digits or -1 when unknown. */ + int expdigs = -1; + /* 1 when ARG < 0, 0 when ARG >= 0, -1 when unknown. */ + int negative = -1; + /* Log10 of EXPDIGS. */ + int logexpdigs = 2;

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

On 08/04/2016 03:13 PM, Martin Sebor wrote: On 07/21/2016 03:48 PM, Jeff Law wrote: Sorry about the delay in responding... I saw a few places in GCC itself where you increased buffer sizes. Were any of those level 1 failures? Yes. With optimization, even level 1 uses range information

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

On 07/21/2016 03:48 PM, Jeff Law wrote: Sorry about the delay in responding... I saw a few places in GCC itself where you increased buffer sizes. Were any of those level 1 failures? Yes. With optimization, even level 1 uses range information when it's available. The idea is that when

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

On Mon, 18 Jul 2016, Martin Sebor wrote: > + /* Number of exponent digits or -1 when unknown. */ > + int expdigs = -1; > + /* 1 when ARG < 0, 0 when ARG >= 0, -1 when unknown. */ > + int negative = -1; > + /* Log10 of EXPDIGS. */ > + int logexpdigs = 2; > + > + const double log10_2 =

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

Working through the new pass... Overall it looks pretty good. There's a certain level of trust I'll extend WRT getting the low level details right -- a thorough testsuite obviously helps there. +const pass_data pass_data_sprintf_length = { + GIMPLE_PASS,// pass type +

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

On Mon, Jul 18, 2016 at 03:59:11PM -0600, Martin Sebor wrote: > + /* Try to use __builtin_object_size although it rarely returns > + a useful result even for straighforward cases. */ > + tree ost = warn_format_length < 2 > +? integer_zero_node : build_int_cst (size_type_node, 2); > +

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

On 07/21/2016 03:57 PM, Martin Sebor wrote: The pass only runs once. Without optimization, it's invoked early, does its thing, and is never invoked again. With optimization, it's also invoked early but returns without doing anything, only to to invoked again much later on to do real work. I

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

To that end, the attached patch adds the checker under its own new pass. The pass runs very early without optimization, and relatively late with it to benefit from the passes above. With optimization the pass also can (but doesn't yet) fold the return value from these functions into a constant.

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

I saw a few places in GCC itself where you increased buffer sizes. Were any of those level 1 failures? In c.opt: +C ObjC C++ ObjC++ Warning Alias (Wformat-length=, 1, 0) Can't this take on values 0, 1, 2? And if so, is the "1" above correct? In invoke.texi we have:

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

On 07/18/2016 03:59 PM, Martin Sebor wrote: Unfortunately, linking with C_COMMON_OBJS isn't enough and linking with C_OBJS doesn't work because of multiple definitions for symbols like gt_ggc_mx_language_function. I don't know this part of of GCC but it seems that each front end gets a set of

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

On 01/07/16 19:15, Martin Sebor wrote: + /* Differentiate between an exact and inexact buffer overflow +or truncation. */ + const char *fmtstr; + if (res->number_chars < 0) + fmtstr = info->bounded + ? "output may

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

On Tue, 2016-07-12 at 16:35 -0600, Martin Sebor wrote: > On 07/12/2016 10:12 AM, Manuel López-Ibáñez wrote: > > On 12/07/16 16:59, Martin Sebor wrote: > > > You're probably right. I suspect I have a tendency to overuse > > > the quotes (e.g, the -Wplacement-new warning also quotes the > > >

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

On Tue, Jul 12, 2016 at 04:35:50PM -0600, Martin Sebor wrote: > Based on the gcc.pot file it does look like quoted numbers are > far less common than unquoted ones (just 10 messages where they > are quoted vs 528 unquoted). > > I've added this as a guideline to the Wiki and assuming no one >

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

On 07/12/2016 10:12 AM, Manuel López-Ibáñez wrote: On 12/07/16 16:59, Martin Sebor wrote: You're probably right. I suspect I have a tendency to overuse the quotes (e.g, the -Wplacement-new warning also quotes the sizes). If there aren't yet (I vague recall coming across something on the GCC

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

On 07/12/2016 06:32 AM, Bernd Schmidt wrote: On 07/01/2016 08:15 PM, Martin Sebor wrote: The attached patch enhances compile-time checking for buffer overflow and output truncation in non-trivial calls to the sprintf family of functions under a new option -Wformat-length=[12]. This initial

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

On 12/07/16 16:59, Martin Sebor wrote: You're probably right. I suspect I have a tendency to overuse the quotes (e.g, the -Wplacement-new warning also quotes the sizes). If there aren't yet (I vague recall coming across something on the GCC Wiki but can't find it now), it would be helpful to

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

Does it warn for the code that Florian actually posted? I tried it with a recent (unpatched) build of trunk and got no warning (with -O2 -Wall -D_FORTIFY_SOURCE=2), but it strikes me that we ought to warn if someone passes about the above code (for the uninitialized format string, at least; I

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

On Tue, 2016-07-12 at 08:45 -0600, Martin Sebor wrote: > On 07/12/2016 04:01 AM, Florian Weimer wrote: > > On 07/12/2016 11:54 AM, Jakub Jelinek wrote: > > > On Tue, Jul 12, 2016 at 11:51:50AM +0200, Florian Weimer wrote: > > > > On 07/01/2016 08:15 PM, Martin Sebor wrote: > > > > > The attached

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

On 07/12/2016 04:01 AM, Florian Weimer wrote: On 07/12/2016 11:54 AM, Jakub Jelinek wrote: On Tue, Jul 12, 2016 at 11:51:50AM +0200, Florian Weimer wrote: On 07/01/2016 08:15 PM, Martin Sebor wrote: The attached patch enhances compile-time checking for buffer overflow and output truncation in

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

On 07/01/2016 08:15 PM, Martin Sebor wrote: The attached patch enhances compile-time checking for buffer overflow and output truncation in non-trivial calls to the sprintf family of functions under a new option -Wformat-length=[12]. This initial patch handles printf directives with string,

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

On 07/12/2016 11:54 AM, Jakub Jelinek wrote: On Tue, Jul 12, 2016 at 11:51:50AM +0200, Florian Weimer wrote: On 07/01/2016 08:15 PM, Martin Sebor wrote: The attached patch enhances compile-time checking for buffer overflow and output truncation in non-trivial calls to the sprintf family of

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

On Tue, Jul 12, 2016 at 11:51:50AM +0200, Florian Weimer wrote: > On 07/01/2016 08:15 PM, Martin Sebor wrote: > >The attached patch enhances compile-time checking for buffer overflow > >and output truncation in non-trivial calls to the sprintf family of > >functions under a new option

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

On 07/01/2016 08:15 PM, Martin Sebor wrote: The attached patch enhances compile-time checking for buffer overflow and output truncation in non-trivial calls to the sprintf family of functions under a new option -Wformat-length=[12]. This initial patch handles printf directives with string,

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

On 07/01/2016 08:15 PM, Martin Sebor wrote: diff --git a/gcc/passes.c b/gcc/passes.c index 0565cfa..008799c 100644 --- a/gcc/passes.c +++ b/gcc/passes.c @@ -2425,8 +2425,15 @@ execute_pass_list_1 (opt_pass *pass) if (cfun == NULL) return; + + // inform (0, "executing pass:

  1   2   >