Patchwork Work around GCC 10 warning regression in string/tester.c

login
register
mail settings
Submitter Joseph Myers
Date Oct. 1, 2019, 9:50 p.m.
Message ID <alpine.DEB.2.21.1910012150040.24964@digraph.polyomino.org.uk>
Download mbox | patch
Permalink /patch/34785/
State New
Headers show

Comments

Joseph Myers - Oct. 1, 2019, 9:50 p.m.
string/tester.c contains code that correctly triggers various GCC
warnings about dubious uses of string functions (uses that are being
deliberately tested there), and duly disables those warnings around
the relevant code.

A change in GCC mainline resulted in this code failing to compile with
a -Warray-bounds error, despite the location with the error having
-Warray-bounds already disabled.  This has been reported as
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91890>.  Since this
doesn't look like being fixed soon, and to keep regression testing
with GCC mainline useful rather than potentially covering up other
issues that might be appearing, this patch adds -Warray-bounds
disabling for a larger region of code that does suffice to avoid the
problem warning.  This is on the basis that this would be removed if
the regression is fixed before GCC 10 is released.

Tested with build-many-glibcs.py for aarch64-linux-gnu with GCC
mainline.

2019-10-01  Joseph Myers  <joseph@codesourcery.com>

	* string/tester.c (test_strncat) [__GNUC_PREREQ (10, 0)]: Ignore
	-Warray-bounds for more of the function.

---

Any comments on either the patch itself or the principle of working
around such issues in glibc to allow more effective testing if not
fixed quickly in GCC?
Florian Weimer - Oct. 2, 2019, 12:41 p.m.
* Joseph Myers:

> Any comments on either the patch itself or the principle of working
> around such issues in glibc to allow more effective testing if not
> fixed quickly in GCC?

I think we should disable those middle-end warnings for the entire file
altogether.

Thanks,
Florian
Jeff Law - Oct. 2, 2019, 1:10 p.m.
On 10/2/19 6:41 AM, Florian Weimer wrote:
> * Joseph Myers:
> 
>> Any comments on either the patch itself or the principle of working
>> around such issues in glibc to allow more effective testing if not
>> fixed quickly in GCC?
> 
> I think we should disable those middle-end warnings for the entire file
> altogether.
When y'all figure out how you want to do this, can you please pull the
fix into Fedora?  I was running the Fedora tester using the latest gcc
snapshot:

> BUILDSTDERR: tester.c: In function 'test_strncat':
> BUILDSTDERR: tester.c:395:10: error: 'strncat' forming offset [50, 98] is out of the bounds [0, 50] of object 'one' with type 'char[50]' [-Werror=array-bounds]
> BUILDSTDERR:   395 |   (void) strncat (one, two, 99);
> BUILDSTDERR:       |          ^~~~~~~~~~~~~~~~~~~~~~
> BUILDSTDERR: tester.c:61:6: note: 'one' declared here
> BUILDSTDERR:    61 | char one[50];
> BUILDSTDERR:       |      ^~~

So this going to affect Fedora when gcc-10 drops in.

jeff
Florian Weimer - Oct. 2, 2019, 1:18 p.m.
* Jeff Law:

> On 10/2/19 6:41 AM, Florian Weimer wrote:
>> * Joseph Myers:
>> 
>>> Any comments on either the patch itself or the principle of working
>>> around such issues in glibc to allow more effective testing if not
>>> fixed quickly in GCC?
>> 
>> I think we should disable those middle-end warnings for the entire file
>> altogether.
> When y'all figure out how you want to do this, can you please pull the
> fix into Fedora?  I was running the Fedora tester using the latest gcc
> snapshot:
>
>> BUILDSTDERR: tester.c: In function 'test_strncat':
>> BUILDSTDERR: tester.c:395:10: error: 'strncat' forming offset [50, 98] is out of the bounds [0, 50] of object 'one' with type 'char[50]' [-Werror=array-bounds]
>> BUILDSTDERR:   395 |   (void) strncat (one, two, 99);
>> BUILDSTDERR:       |          ^~~~~~~~~~~~~~~~~~~~~~
>> BUILDSTDERR: tester.c:61:6: note: 'one' declared here
>> BUILDSTDERR:    61 | char one[50];
>> BUILDSTDERR:       |      ^~~
>
> So this going to affect Fedora when gcc-10 drops in.

Sure, we pull current upstream master into Fedora once per week.

Thanks,
Florian
Jeff Law - Oct. 2, 2019, 1:22 p.m.
On 10/2/19 7:18 AM, Florian Weimer wrote:
> * Jeff Law:
> 
>> On 10/2/19 6:41 AM, Florian Weimer wrote:
>>> * Joseph Myers:
>>>
>>>> Any comments on either the patch itself or the principle of working
>>>> around such issues in glibc to allow more effective testing if not
>>>> fixed quickly in GCC?
>>>
>>> I think we should disable those middle-end warnings for the entire file
>>> altogether.
>> When y'all figure out how you want to do this, can you please pull the
>> fix into Fedora?  I was running the Fedora tester using the latest gcc
>> snapshot:
>>
>>> BUILDSTDERR: tester.c: In function 'test_strncat':
>>> BUILDSTDERR: tester.c:395:10: error: 'strncat' forming offset [50, 98] is out of the bounds [0, 50] of object 'one' with type 'char[50]' [-Werror=array-bounds]
>>> BUILDSTDERR:   395 |   (void) strncat (one, two, 99);
>>> BUILDSTDERR:       |          ^~~~~~~~~~~~~~~~~~~~~~
>>> BUILDSTDERR: tester.c:61:6: note: 'one' declared here
>>> BUILDSTDERR:    61 | char one[50];
>>> BUILDSTDERR:       |      ^~~
>>
>> So this going to affect Fedora when gcc-10 drops in.
> 
> Sure, we pull current upstream master into Fedora once per week.
Perfect.  That works for me.

Jeff

Patch

diff --git a/string/tester.c b/string/tester.c
index 24b0dad1cb..9350538996 100644
--- a/string/tester.c
+++ b/string/tester.c
@@ -359,6 +359,14 @@  test_strncat (void)
   /* First test it as strcat, with big counts, then test the count
      mechanism.  */
   it = "strncat";
+#if __GNUC_PREREQ (10, 0)
+  DIAG_PUSH_NEEDS_COMMENT;
+  /* A regression in GCC 10
+     <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91890>, present as
+     of 2019-10-01, means this warning needs to be disabled around
+     more code than just the relevant strncat call.  */
+  DIAG_IGNORE_NEEDS_COMMENT (8, "-Warray-bounds");
+#endif
   (void) strcpy (one, "ijk");
   DIAG_PUSH_NEEDS_COMMENT;
 #if __GNUC_PREREQ (7, 0)
@@ -394,6 +402,10 @@  test_strncat (void)
   DIAG_IGNORE_NEEDS_COMMENT (8, "-Warray-bounds");
   (void) strncat (one, two, 99);
   DIAG_POP_NEEDS_COMMENT;
+#if __GNUC_PREREQ (10, 0)
+  /* See the comment above about a regression in GCC 10.  */
+  DIAG_POP_NEEDS_COMMENT;
+#endif
   equal (one, "ghef", 5);			/* Basic test encore. */
   equal (two, "ef", 6);			/* Stomped on source? */