* [PATCH, GCC] Deal with singular sentences in builtin-sprintf-warn-1.c regex
@ 2016-09-23 15:55 Thomas Preudhomme
2016-09-23 16:02 ` Thomas Preudhomme
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Preudhomme @ 2016-09-23 15:55 UTC (permalink / raw)
To: gcc-patches, msebor, Richard Biener, Jakub Jelinek
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.
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, GCC] Deal with singular sentences in builtin-sprintf-warn-1.c regex
2016-09-23 15:55 [PATCH, GCC] Deal with singular sentences in builtin-sprintf-warn-1.c regex Thomas Preudhomme
@ 2016-09-23 16:02 ` Thomas Preudhomme
2016-09-23 17:02 ` Martin Sebor
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Preudhomme @ 2016-09-23 16:02 UTC (permalink / raw)
To: gcc-patches, msebor, Richard Biener, Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 973 bytes --]
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.
>
> 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
[-- Attachment #2: accept_single_form_byte.patch --]
[-- Type: text/x-patch, Size: 2974 bytes --]
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. */
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, GCC] Deal with singular sentences in builtin-sprintf-warn-1.c regex
2016-09-23 16:02 ` Thomas Preudhomme
@ 2016-09-23 17:02 ` Martin Sebor
2016-09-23 17:04 ` Thomas Preudhomme
2016-09-26 10:02 ` Thomas Preudhomme
0 siblings, 2 replies; 6+ messages in thread
From: Martin Sebor @ 2016-09-23 17:02 UTC (permalink / raw)
To: Thomas Preudhomme, gcc-patches, Richard Biener, Jakub Jelinek
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, GCC] Deal with singular sentences in builtin-sprintf-warn-1.c regex
2016-09-23 17:02 ` Martin Sebor
@ 2016-09-23 17:04 ` Thomas Preudhomme
2016-09-26 10:02 ` Thomas Preudhomme
1 sibling, 0 replies; 6+ messages in thread
From: Thomas Preudhomme @ 2016-09-23 17:04 UTC (permalink / raw)
To: Martin Sebor, gcc-patches, Richard Biener, Jakub Jelinek
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, GCC] Deal with singular sentences in builtin-sprintf-warn-1.c regex
2016-09-23 17:02 ` Martin Sebor
2016-09-23 17:04 ` Thomas Preudhomme
@ 2016-09-26 10:02 ` Thomas Preudhomme
2016-09-26 15:46 ` Jeff Law
1 sibling, 1 reply; 6+ messages in thread
From: Thomas Preudhomme @ 2016-09-26 10:02 UTC (permalink / raw)
To: Martin Sebor, gcc-patches, Richard Biener, Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 1730 bytes --]
How about this reworked patch?
Best regards,
Thomas
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'
>
> 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
>
[-- Attachment #2: accept_single_form_byte.patch --]
[-- Type: text/x-patch, Size: 1386 bytes --]
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..efa69b8a8b29da4ed3253e19ba646168a6a1ca58 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
@@ -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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, GCC] Deal with singular sentences in builtin-sprintf-warn-1.c regex
2016-09-26 10:02 ` Thomas Preudhomme
@ 2016-09-26 15:46 ` Jeff Law
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Law @ 2016-09-26 15:46 UTC (permalink / raw)
To: Thomas Preudhomme, Martin Sebor, gcc-patches, Richard Biener,
Jakub Jelinek
On 09/26/2016 03:55 AM, Thomas Preudhomme wrote:
> How about this reworked patch?
>
> Best regards,
>
> Thomas
>
> 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'
>>
>> 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
>>
>
> accept_single_form_byte.patch
>
>
> 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..efa69b8a8b29da4ed3253e19ba646168a6a1ca58 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
> @@ -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" } */
This is fine.
jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-26 15:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-23 15:55 [PATCH, GCC] Deal with singular sentences in builtin-sprintf-warn-1.c regex Thomas Preudhomme
2016-09-23 16:02 ` Thomas Preudhomme
2016-09-23 17:02 ` Martin Sebor
2016-09-23 17:04 ` Thomas Preudhomme
2016-09-26 10:02 ` Thomas Preudhomme
2016-09-26 15:46 ` Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).