| Message ID | 1a92da59-caba-3d2b-4f76-daadff2226e9@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
On 09/23/2016 09:42 AM, Thomas Preudhomme wrote: > Sorry, forgot the patch. Please find it attached. > > Best regards, > > Thomas > > On 23/09/16 16:40, Thomas Preudhomme wrote: >> Hi, >> >> New builtin-sprintf-warn-1.c testcase contains a few regex of the form >> "\[0-9\]+ >> bytes" or ". bytes". This does not account for the case where the >> number of byte >> is 0 in which case byte would be in the singular form. This caused a >> FAIL on >> arm-none-eabi targets. This patch makes the s optional in all cases >> where the >> number of bytes is unknown. >> >> I did not commit this fix as obvious as people might want to only do >> the changes >> for lines where the number of bytes *could* be 0 so I prefer to get >> review. Thanks for the patch. The %p fixes look correct to me (someone else needs to approve the final patch). For the INT_MAX tests, I think it's a sign of a bug in the pass if for the following call sprintf (buf, "X%*c", INT_MAX, '1'); GCC prints warning: directive output of 0 byte causes result to exceed 'INT_MAX' My guess is that the bug is specific to a 32-bit compiler (I can't reproduce it in a 64-bit one with -m32). Let me build one and look into fixing it. Martin >> >> ChangeLog entry is as follows: >> >> *** gcc/testsuite/ChangeLog *** >> >> 2016-09-23 Thomas Preud'homme <thomas.preudhomme@arm.com> >> >> * gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Adjust regex to >> accept >> singular form of byte when quantity is unknown. >> >> >> Is this ok for trunk? >> >> Best regards, >> >> Thomas
Hi Martin, On 23/09/16 17:17, Martin Sebor wrote: > On 09/23/2016 09:42 AM, Thomas Preudhomme wrote: >> Sorry, forgot the patch. Please find it attached. >> >> Best regards, >> >> Thomas >> >> On 23/09/16 16:40, Thomas Preudhomme wrote: >>> Hi, >>> >>> New builtin-sprintf-warn-1.c testcase contains a few regex of the form >>> "\[0-9\]+ >>> bytes" or ". bytes". This does not account for the case where the >>> number of byte >>> is 0 in which case byte would be in the singular form. This caused a >>> FAIL on >>> arm-none-eabi targets. This patch makes the s optional in all cases >>> where the >>> number of bytes is unknown. >>> >>> I did not commit this fix as obvious as people might want to only do >>> the changes >>> for lines where the number of bytes *could* be 0 so I prefer to get >>> review. > > Thanks for the patch. The %p fixes look correct to me (someone > else needs to approve the final patch). > > For the INT_MAX tests, I think it's a sign of a bug in the pass if > for the following call > > sprintf (buf, "X%*c", INT_MAX, '1'); > > GCC prints > > warning: directive output of 0 byte causes result to exceed 'INT_MAX' I only had a failed warning on line 101: T (0, "%p", (void*)0x1); /* { dg-warning ".%p. directive writing . bytes into a region of size 0" } */ The other were fine. I just changed all the one that were using . or explicitely including 0 when matching the amount of bytes. I did not look into which one make sense to be 0 which is why I wanted review. I'm happy to just keep the %p ones (or even only the one with 0, "%p") if you think that's the only one that needs it. Best regards, Thomas
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 index e098be92bb0377414b1f9cacf5e4d2a3398e74ec..85dbcd9d6d3a5b1ad810037f03451207284a25b1 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c @@ -77,13 +77,13 @@ void test_sprintf_c_const (void) T (-1, "%*c", INT_MAX - 1, '1'); T (-1, "%*c", INT_MAX, '1'); T (-1, "X%*c", INT_MAX - 1, '1'); - T (-1, "X%*c", INT_MAX, '1'); /* { dg-warning "directive output of \[0-9\]+ bytes causes result to exceed .INT_MAX." } */ + T (-1, "X%*c", INT_MAX, '1'); /* { dg-warning "directive output of \[0-9\]+ bytes? causes result to exceed .INT_MAX." } */ - T (-1, "%*c%*c", INT_MAX - 1, '1', INT_MAX - 1, '2'); /* { dg-warning "directive output of \[0-9\]+ bytes causes result to exceed .INT_MAX." } */ + T (-1, "%*c%*c", INT_MAX - 1, '1', INT_MAX - 1, '2'); /* { dg-warning "directive output of \[0-9\]+ bytes? causes result to exceed .INT_MAX." } */ T (-1, "%*cX", INT_MAX - 2, '1'); T (-1, "%*cX", INT_MAX - 1, '1'); - T (-1, "%*cX", INT_MAX, '1'); /* { dg-warning "output of \[0-9\]+ bytes causes result to exceed .INT_MAX." } */ + T (-1, "%*cX", INT_MAX, '1'); /* { dg-warning "output of \[0-9\]+ bytes? causes result to exceed .INT_MAX." } */ } /* Exercise the "%p" directive with constant arguments. */ @@ -98,9 +98,9 @@ void test_sprintf_p_const (void) /* The exact output for %p is unspecified by C. Two formats are known: same as %tx (for example AIX) and same as %#tx (for example Solaris). */ - T (0, "%p", (void*)0x1); /* { dg-warning ".%p. directive writing . bytes into a region of size 0" } */ - T (1, "%p", (void*)0x12); /* { dg-warning ".%p. directive writing . bytes into a region of size 1" } */ - T (2, "%p", (void*)0x123); /* { dg-warning ".%p. directive writing . bytes into a region of size 2" } */ + T (0, "%p", (void*)0x1); /* { dg-warning ".%p. directive writing . bytes? into a region of size 0" } */ + T (1, "%p", (void*)0x12); /* { dg-warning ".%p. directive writing . bytes? into a region of size 1" } */ + T (2, "%p", (void*)0x123); /* { dg-warning ".%p. directive writing . bytes? into a region of size 2" } */ /* GLIBC and uClibc treat the ' ' flag with the "%p" directive the same as with signed integer conversions (i.e., it prepends a space). Other @@ -299,7 +299,7 @@ void test_sprintf_chk_s_const (void) when the size of the destination object is unknown. */ T (-1, "%*s", INT_MAX - 1, ""); T (-1, "%*s", INT_MAX, ""); - T (-1, "X%*s", INT_MAX, ""); /* { dg-warning "directive output of \[0-9\]+ bytes causes result to exceed .INT_MAX." } */ + T (-1, "X%*s", INT_MAX, ""); /* { dg-warning "directive output of \[0-9\]+ bytes? causes result to exceed .INT_MAX." } */ /* Multiple directives. */