Bug 1263817 - glibc fails to build with incorrect -Werror=array-bounds failure.
Summary: glibc fails to build with incorrect -Werror=array-bounds failure.
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: gcc
Version: 24
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Jakub Jelinek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: ARMTracker
TreeView+ depends on / blocked
 
Reported: 2015-09-16 19:17 UTC by Carlos O'Donell
Modified: 2016-03-10 15:20 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-03-10 15:20:04 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
ld-ctype.i (503.59 KB, text/plain)
2015-09-16 19:57 UTC, Carlos O'Donell
no flags Details
dns-host.i (336.62 KB, text/plain)
2015-09-16 20:12 UTC, Carlos O'Donell
no flags Details
ld-ctype.i (502.98 KB, text/plain)
2015-09-16 20:28 UTC, Carlos O'Donell
no flags Details

Description Carlos O'Donell 2015-09-16 19:17:47 UTC
The glibc builds use -Werror to ensure that we don't have any new warnings that might be real problems.

Recently with gcc 5.1.1-4 there have been 5 instances of this which I've had to fix by disabling the warning with pragmas. The following are the changes I had to make and rationale for why each one can't possibly be a real -Warray-bounds problem.

This looks a lot like PR/59124?

If glibc is having this problem, then other packages are having this problem. I'm filling this bug so we can get something into rawhide to fix the situation.

You can see this yourself today by:
fedpkg clone glibc
cd glibc
git checkout 444c2ecfbca6d5761aeb1c1888a16f0973d073c8
fedpkg mockbuild

To build the fedora rawhide glibc which fails due to -Werror.

You can then build all of glibc with:
mock -r fedora-rawhide-x86_64 --resultdir=results_glibc/ --no-cleanup-after --without=werror ./glibc-2.22.90-5.fc24.src.rpm

That will fail, and you can enter the mock chroot and inspect:
mock -r fedora-rawhide-x86_64 --shell
cd /builddir/build/BUILD/glibc-2.22-193-g315267a/build-x86_64-redhat-linux
make -j4


Index: glibc-2.22-193-g315267a/resolv/nss_dns/dns-host.c
===================================================================
--- glibc-2.22-193-g315267a.orig/resolv/nss_dns/dns-host.c
+++ glibc-2.22-193-g315267a/resolv/nss_dns/dns-host.c
@@ -561,10 +561,19 @@ addrsort (char **ap, int num)
     num = MAX_NR_ADDRS;
   for (i = 0; i < num; i++, p++)
     {
+      DIAG_PUSH_NEEDS_COMMENT
+#if __GNUC_PREREQ (5, 0)
+      /* GCC 5.0 warns about array subscript being above array bounds,
+	 but that's not entirely possible since i is limited to
+	 _res.nsort which is limited to MAXRESOLVSORT. This
+	is likely PR/59124 which is still not fixed.  */
+      DIAG_IGNORE_NEEDS_COMMENT (5.0, "-Warray-bounds")
+#endif
       for (j = 0 ; (unsigned)j < _res.nsort; j++)
 	if (_res.sort_list[j].addr.s_addr ==
 	    (((struct in_addr *)(*p))->s_addr & _res.sort_list[j].mask))
 	  break;
+      DIAG_POP_NEEDS_COMMENT
       aval[i] = j;
       if (needsort == 0 && i > 0 && j < aval[i-1])
 	needsort = i;
Index: glibc-2.22-193-g315267a/resolv/gethnamaddr.c
===================================================================
--- glibc-2.22-193-g315267a.orig/resolv/gethnamaddr.c
+++ glibc-2.22-193-g315267a/resolv/gethnamaddr.c
@@ -986,10 +986,19 @@ addrsort(ap, num)
 
 	p = ap;
 	for (i = 0; i < num; i++, p++) {
+	    DIAG_PUSH_NEEDS_COMMENT
+#if __GNUC_PREREQ (5, 0)
+	    /* GCC 5.0 warns about array subscript being above array bounds,
+	       but that's not entirely possible since i is limited to
+	       _res.nsort which is limited to MAXRESOLVSORT. This
+	       is likely PR/59124 which is still not fixed.  */
+	    DIAG_IGNORE_NEEDS_COMMENT (5.0, "-Warray-bounds")
+#endif
 	    for (j = 0 ; (unsigned)j < _res.nsort; j++)
 		if (_res.sort_list[j].addr.s_addr ==
 		    (((struct in_addr *)(*p))->s_addr & _res.sort_list[j].mask))
 			break;
+	    DIAG_POP_NEEDS_COMMENT
 	    aval[i] = j;
 	    if (needsort == 0 && i > 0 && j < aval[i-1])
 		needsort = i;
Index: glibc-2.22-193-g315267a/locale/programs/ld-ctype.c
===================================================================
--- glibc-2.22-193-g315267a.orig/locale/programs/ld-ctype.c
+++ glibc-2.22-193-g315267a/locale/programs/ld-ctype.c
@@ -2534,9 +2534,19 @@ with character code range values one mus
 	    {
 	      size_t cnt;
 
+	      DIAG_PUSH_NEEDS_COMMENT
+#if __GNUC_PREREQ (5, 0)
+		/* GCC 5.0 warns about array subscript being above array bounds,
+		   but that's not possible since ctype_map_new prevents
+		   map_collection_nr from being greater than MAX_NR_CHARMP which
+		   is the size of mapnames. This is likely PR/59124 which is still
+		   not fixed.  */
+	      DIAG_IGNORE_NEEDS_COMMENT (5.0, "-Warray-bounds")
+#endif
 	      for (cnt = 2; cnt < ctype->map_collection_nr; ++cnt)
 		if (strcmp (now->val.str.startmb, ctype->mapnames[cnt]) == 0)
 		  break;
+	      DIAG_POP_NEEDS_COMMENT
 
 	      if (cnt < ctype->map_collection_nr)
 		free (now->val.str.startmb);
@@ -2807,9 +2817,19 @@ previous definition was here")));
 
 	  /* This could mean one of several things.  First test whether
 	     it's a character class name.  */
+	  DIAG_PUSH_NEEDS_COMMENT
+#if __GNUC_PREREQ (5, 0)
+	    /* GCC 5.0 warns about array subscript being above array bounds,
+	       but that's not possible since ctype_class_new prevents
+	       nr_charclass from being greater than MAX_NR_CHARCLASS which
+	       is the size of classnames. This is likely PR/59124 which is still
+	       not fixed.  */
+	  DIAG_IGNORE_NEEDS_COMMENT (5.0, "-Warray-bounds")
+#endif
 	  for (cnt = 0; cnt < ctype->nr_charclass; ++cnt)
 	    if (strcmp (now->val.str.startmb, ctype->classnames[cnt]) == 0)
 	      break;
+	  DIAG_POP_NEEDS_COMMENT
 	  if (cnt < ctype->nr_charclass)
 	    {
 	      class_bit = _ISwbit (cnt);
Index: glibc-2.22-193-g315267a/resolv/res_hconf.c
===================================================================
--- glibc-2.22-193-g315267a.orig/resolv/res_hconf.c
+++ glibc-2.22-193-g315267a/resolv/res_hconf.c
@@ -523,7 +523,16 @@ _res_hconf_trim_domain (char *hostname)
 
   for (i = 0; i < _res_hconf.num_trimdomains; ++i)
     {
+      DIAG_PUSH_NEEDS_COMMENT
+#if __GNUC_PREREQ (5, 0)
+      /* GCC 5.0 warns about array subscript being above array bounds,
+	 but that's not entirely possible since i is limited to
+	 num_trimdomains which is limited to <= TRIMDOMAINS_MAX. This
+	 is likely PR/59124 which is still not fixed.  */ 
+      DIAG_IGNORE_NEEDS_COMMENT (5.0, "-Warray-bounds")
+#endif
       const char *trim = _res_hconf.trimdomain[i];
+      DIAG_POP_NEEDS_COMMENT
 
       trim_len = strlen (trim);
       if (hostname_len > trim_len
---

There are even more patches for the testsuite which we also build with -Werror.

Related:
https://bugzilla.redhat.com/show_bug.cgi?id=1236784

Comment 1 Carlos O'Donell 2015-09-16 19:21:35 UTC
I'll attach preprocessed source shortly.

Comment 2 Carlos O'Donell 2015-09-16 19:56:15 UTC
gcc version 5.1.1 20150618 (Red Hat 5.1.1-4) (GCC):

gcc ld-ctype.i -c -std=gnu99 -fgnu89-inline  -O3 -Wall -Wundef -Wwrite-strings -fasynchronous-unwind-tables -fmerge-all-constants -frounding-math -g -mtune=generic -Wstrict-prototypes

programs/ld-ctype.c: In function ‘ctype_read’:
programs/ld-ctype.c:2842:552: warning: array subscript is above array bounds [-Warray-bounds]

Comment 3 Carlos O'Donell 2015-09-16 19:57:30 UTC
Created attachment 1074178 [details]
ld-ctype.i

Comment 4 Carlos O'Donell 2015-09-16 19:58:46 UTC
Note: I had to inline `-include include/libc-symbols.h` otherwise it's a pain to get usable pre-processed source from glibc. Therefore the file is not exactly the same as it was, but contains a top line `#include <libc-symbols.h>` such that `-save-temps` gets all the information it needs into the *.i file.

Comment 5 Carlos O'Donell 2015-09-16 20:12:33 UTC
gcc dns-host.i -c -std=gnu99 -fgnu89-inline  -O3 -Wall -Wundef -Wwrite-strings -fasynchronous-unwind-tables -fmerge-all-constants -frounding-math -g -mtune=generic -Wno-strict-prototypes -Wno-write-strings -Wstrict-prototypes   -fPIC

nss_dns/dns-host.c: In function ‘getanswer_r’:
nss_dns/dns-host.c:567:25: warning: array subscript is above array bounds [-Warray-bounds]
  if (_res.sort_list[j].addr.s_addr ==
                         ^
nss_dns/dns-host.c:568:61: warning: array subscript is above array bounds [-Warray-bounds]
      (((struct in_addr *)(*p))->s_addr & _res.sort_list[j].mask))
                                                             ^

Comment 6 Carlos O'Donell 2015-09-16 20:12:59 UTC
Created attachment 1074180 [details]
dns-host.i

Comment 7 Carlos O'Donell 2015-09-16 20:27:20 UTC
(In reply to Carlos O'Donell from comment #2)
> gcc version 5.1.1 20150618 (Red Hat 5.1.1-4) (GCC):
> 
> gcc ld-ctype.i -c -std=gnu99 -fgnu89-inline  -O3 -Wall -Wundef
> -Wwrite-strings -fasynchronous-unwind-tables -fmerge-all-constants
> -frounding-math -g -mtune=generic -Wstrict-prototypes
> 
> programs/ld-ctype.c: In function ‘ctype_read’:
> programs/ld-ctype.c:2842:552: warning: array subscript is above array bounds
> [-Warray-bounds]

Sorry, I just noticed the compiler adds yet another previously not reported array bounds error after I mark the first two, thus I need a new fix.

So this will look like this when run:

programs/ld-ctype.c: In function ‘ctype_read’:
programs/ld-ctype.c:2539:549: warning: array subscript is above array bounds [-Warray-bounds]
programs/ld-ctype.c:2822:552: warning: array subscript is above array bounds [-Warray-bounds]

Comment 8 Carlos O'Donell 2015-09-16 20:28:41 UTC
Created attachment 1074182 [details]
ld-ctype.i

Comment 9 Carlos O'Donell 2015-09-16 20:29:33 UTC
With the new ld-ctype patch looking like this.

Index: glibc-2.22-193-g315267a/locale/programs/ld-ctype.c
===================================================================
--- glibc-2.22-193-g315267a.orig/locale/programs/ld-ctype.c
+++ glibc-2.22-193-g315267a/locale/programs/ld-ctype.c
@@ -2534,9 +2534,19 @@ with character code range values one mus
 	    {
 	      size_t cnt;
 
+	      DIAG_PUSH_NEEDS_COMMENT
+#if __GNUC_PREREQ (5, 0)
+		/* GCC 5.0 warns about array subscript being above array bounds,
+		   but that's not possible since ctype_map_new prevents
+		   map_collection_nr from being greater than MAX_NR_CHARMP which
+		   is the size of mapnames. This is likely PR/59124 which is still
+		   not fixed.  */
+	      DIAG_IGNORE_NEEDS_COMMENT (5.0, "-Warray-bounds")
+#endif
 	      for (cnt = 2; cnt < ctype->map_collection_nr; ++cnt)
 		if (strcmp (now->val.str.startmb, ctype->mapnames[cnt]) == 0)
 		  break;
+	      DIAG_POP_NEEDS_COMMENT
 
 	      if (cnt < ctype->map_collection_nr)
 		free (now->val.str.startmb);
@@ -2807,9 +2817,19 @@ previous definition was here")));
 
 	  /* This could mean one of several things.  First test whether
 	     it's a character class name.  */
+	  DIAG_PUSH_NEEDS_COMMENT
+#if __GNUC_PREREQ (5, 0)
+	    /* GCC 5.0 warns about array subscript being above array bounds,
+	       but that's not possible since ctype_class_new prevents
+	       nr_charclass from being greater than MAX_NR_CHARCLASS which
+	       is the size of classnames. This is likely PR/59124 which is still
+	       not fixed.  */
+	  DIAG_IGNORE_NEEDS_COMMENT (5.0, "-Warray-bounds")
+#endif
 	  for (cnt = 0; cnt < ctype->nr_charclass; ++cnt)
 	    if (strcmp (now->val.str.startmb, ctype->classnames[cnt]) == 0)
 	      break;
+	  DIAG_POP_NEEDS_COMMENT
 	  if (cnt < ctype->nr_charclass)
 	    {
 	      class_bit = _ISwbit (cnt);
@@ -2817,9 +2837,19 @@ previous definition was here")));
 	      free (now->val.str.startmb);
 	      goto read_charclass;
 	    }
+	  DIAG_PUSH_NEEDS_COMMENT
+#if __GNUC_PREREQ (5, 0)
+	  /* GCC 5.0 warns about array subscript being above array bounds,
+	     but that's not possible since ctype_map_new prevents
+	     map_collection_nr from being greater than MAX_NR_CHARMP which
+	     is the size of mapnames. This is likely PR/59124 which is still
+	     not fixed.  */
+	  DIAG_IGNORE_NEEDS_COMMENT (5.0, "-Warray-bounds")
+#endif
 	  for (cnt = 0; cnt < ctype->map_collection_nr; ++cnt)
 	    if (strcmp (now->val.str.startmb, ctype->mapnames[cnt]) == 0)
 	      break;
+	  DIAG_POP_NEEDS_COMMENT
 	  if (cnt < ctype->map_collection_nr)
 	    {
 	      mapidx = cnt;
---

Comment 10 Carlos O'Donell 2015-09-16 20:30:59 UTC
And for clarity:
---
/* The macros to control diagnostics are structured like this, rather
   than a single macro that both pushes and pops diagnostic state and
   takes the affected code as an argument, because the GCC pragmas
   work by disabling the diagnostic for a range of source locations
   and do not work when all the pragmas and the affected code are in a
   single macro expansion.  */

/* Push diagnostic state.  */
#define DIAG_PUSH_NEEDS_COMMENT _Pragma ("GCC diagnostic push")

/* Pop diagnostic state.  */
#define DIAG_POP_NEEDS_COMMENT _Pragma ("GCC diagnostic pop")

#define _DIAG_STR1(s) #s
#define _DIAG_STR(s) _DIAG_STR1(s)

/* Ignore the diagnostic OPTION.  VERSION is the most recent GCC
   version for which the diagnostic has been confirmed to appear in
   the absence of the pragma (in the form MAJOR.MINOR for GCC 4.x,
   just MAJOR for GCC 5 and later).  Uses of this pragma should be
   reviewed when the GCC version given is no longer supported for
   building glibc; the version number should always be on the same
   source line as the macro name, so such uses can be found with grep.
   Uses should come with a comment giving more details of the
   diagnostic, and an architecture on which it is seen if possibly
   optimization-related and not in architecture-specific code.  This
   macro should only be used if the diagnostic seems hard to fix (for
   example, optimization-related false positives).  */
#define DIAG_IGNORE_NEEDS_COMMENT(version, option)      \
  _Pragma (_DIAG_STR (GCC diagnostic ignored option))
---

Comment 11 Marek Polacek 2015-09-17 14:11:12 UTC
I could reproduce this with gcc-5.1.1-4.fc22.x86_64, but not with any upstream version or with gcc built from latest redhat/gcc-5-branch.

I think this has been fixed in PR66422.  This fix isn't in gcc-5.1.1-4.fc22, but is in gcc-5.1.1-5.fc22, so I hope this annoying issue will be resolved when Fedora lets us upgrade to a newer version of gcc.  I'll try to check though.

Comment 12 Carlos O'Donell 2015-09-17 14:13:30 UTC
(In reply to Marek Polacek from comment #11)
> I could reproduce this with gcc-5.1.1-4.fc22.x86_64, but not with any
> upstream version or with gcc built from latest redhat/gcc-5-branch.
> 
> I think this has been fixed in PR66422.  This fix isn't in gcc-5.1.1-4.fc22,
> but is in gcc-5.1.1-5.fc22, so I hope this annoying issue will be resolved
> when Fedora lets us upgrade to a newer version of gcc.  I'll try to check
> though.

Awesome, thanks for verifying. I'd like to see this fixed in Rawhide, but there is no time pressure since I'm working around the new warnings. It's just annoying :-(

Comment 13 Jakub Jelinek 2015-09-17 14:25:50 UTC
Fixing it in rawhide is problematic, because the arm builders are too slow (they need more than 24 hours to build gcc) and there is a 24 hour timeout for the builds.  So, the last few builds of gcc in rawhide or f23 just failed.

Comment 14 Marek Polacek 2015-09-17 14:35:10 UTC
I've verified in mock that with gcc-5.1.1-5.fc22 we don't issue any -Warray-bounds warnings on neither ld-ctype.i nor dns-host.i.

Comment 15 Carlos O'Donell 2015-09-17 15:26:27 UTC
(In reply to Jakub Jelinek from comment #13)
> Fixing it in rawhide is problematic, because the arm builders are too slow
> (they need more than 24 hours to build gcc) and there is a 24 hour timeout
> for the builds.  So, the last few builds of gcc in rawhide or f23 just
> failed.

I've spoken with Peter Robinson and he's looking into some ways in which we might look at either why this is slow (did hardening make it slow? %global _hardened_build 0) or how to speed it up (use SSDs). No promises though, but I think we can make this build faster.

Comment 16 Jan Kurik 2016-02-24 13:45:39 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 24 development cycle.
Changing version to '24'.

More information and reason for this action is here:
https://fedoraproject.org/wiki/Fedora_Program_Management/HouseKeeping/Fedora24#Rawhide_Rebase

Comment 17 Marek Polacek 2016-03-10 15:20:04 UTC
This ought to be fixed since gcc-5.1.1-5.fc23.  And the rawhide GCC appears to be fixed as well.  Closing out thus.


Note You need to log in before you can comment on or make changes to this bug.