Bug 26687 - -Warray-bounds instances building with GCC 11
Summary: -Warray-bounds instances building with GCC 11
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: build (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: 2.33
Assignee: Adhemerval Zanella
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-10-01 00:28 UTC by Martin Sebor
Modified: 2020-10-30 21:41 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed: 2020-10-06 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2020-10-01 00:28:36 UTC
GCC 11 has recently enhanced the -Warray-bounds warning to detect accesses whose starting address is in bounds but whose ending address is beyond the end of the accessed object.  A Glibc build with the latest GCC shows the following instances of this warning.  I haven't fully investigated them yet but going by the first message alone, the access newp->sigev_notify implies that newp points to memory big enough for the whole of *newp (i.e., sizeof *newp big).  If that's not the case as the malloc argument seems to suggest the warning will trigger.

../sysdeps/unix/sysv/linux/timer_create.c:83:17: warning: array subscript ‘struct timer[0]’ is partly outside array bounds of ‘unsigned char[8]’ [-Warray-bounds]
   83 |             newp->sigev_notify = (evp != NULL
      |                 ^~
../sysdeps/unix/sysv/linux/timer_create.c:59:47: note: referencing an object of size 8 allocated by ‘malloc’
   59 |         struct timer *newp = (struct timer *) malloc (offsetof (struct timer,
      |                                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   60 |                                                                 thrfunc));
      |                                                                 ~~~~~~~~~
../sysdeps/unix/sysv/linux/timer_create.c:85:17: warning: array subscript ‘struct timer[0]’ is partly outside array bounds of ‘unsigned char[8]’ [-Warray-bounds]
   85 |             newp->ktimerid = ktimerid;
      |                 ^~
../sysdeps/unix/sysv/linux/timer_create.c:59:47: note: referencing an object of size 8 allocated by ‘malloc’
   59 |         struct timer *newp = (struct timer *) malloc (offsetof (struct timer,
      |                                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   60 |                                                                 thrfunc));
      |                                                                 ~~~~~~~~~
./sysdeps/unix/sysv/linux/timer_create.c:83:17: warning: array subscript ‘struct timer[0]’ is partly outside array bounds of ‘unsigned char[8]’ [-Warray-bounds]
   83 |             newp->sigev_notify = (evp != NULL
      |                 ^~
../sysdeps/unix/sysv/linux/timer_create.c:59:47: note: referencing an object of size 8 allocated by ‘malloc’
   59 |         struct timer *newp = (struct timer *) malloc (offsetof (struct timer,
      |                                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   60 |                                                                 thrfunc));
      |                                                                 ~~~~~~~~~
../sysdeps/unix/sysv/linux/timer_create.c:85:17: warning: array subscript ‘struct timer[0]’ is partly outside array bounds of ‘unsigned char[8]’ [-Warray-bounds]
   85 |             newp->ktimerid = ktimerid;
      |                 ^~
../sysdeps/unix/sysv/linux/timer_create.c:59:47: note: referencing an object of size 8 allocated by ‘malloc’
   59 |         struct timer *newp = (struct timer *) malloc (offsetof (struct timer,
      |                                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   60 |                                                                 thrfunc));
      |                                                                 ~~~~~~~~~
ypclnt.c:376:54: warning: array subscript ‘struct ypresp_val[0]’ is partly outside array bounds of ‘ypresp_master[1]’ [-Warray-bounds]
  376 |     status = ypprot_err (((struct ypresp_val *) resp)->stat);
      |                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
ypclnt.c:592:17: note: while referencing ‘resp’
  592 |   ypresp_master resp;
      |                 ^~~~
ypclnt.c: In function ‘yp_order’:
ypclnt.c:376:54: warning: array subscript ‘struct ypresp_val[0]’ is partly outside array bounds of ‘struct ypresp_order[1]’ [-Warray-bounds]
  376 |     status = ypprot_err (((struct ypresp_val *) resp)->stat);
      |                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
ypclnt.c:622:23: note: while referencing ‘resp’
  622 |   struct ypresp_order resp;
      |                       ^~~~
ypclnt.c: In function ‘yp_maplist’:
ypclnt.c:376:54: warning: array subscript ‘struct ypresp_val[0]’ is partly outside array bounds of ‘struct ypresp_maplist[1]’ [-Warray-bounds]
  376 |     status = ypprot_err (((struct ypresp_val *) resp)->stat);
      |                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
ypclnt.c:795:25: note: while referencing ‘resp’
  795 |   struct ypresp_maplist resp;
      |                         ^~~~
Comment 1 Martin Sebor 2020-10-05 21:02:28 UTC
After inspecting the Glibc code I believe the warning works as designed and this instance is justified.  But the comment above the malloc call explains that it's written this way deliberately so it's in all likelihood safe.

	/* We avoid allocating too much memory by basically
	   using struct timer as a derived class with the
	   first two elements being in the superclass.  We only
	   need these two elements here.  */
	struct timer *newp = (struct timer *) malloc (offsetof (struct timer,
								thrfunc));
	if (newp == NULL)
	  /* No more memory.  */
	  return -1;

	...

	kernel_timer_t ktimerid;
	int retval = INLINE_SYSCALL (timer_create, 3, syscall_clockid, evp,
				     &ktimerid);

	if (retval != -1)
	  {
	    newp->sigev_notify = (evp != NULL
				  ? evp->sigev_notify : SIGEV_SIGNAL);
	    newp->ktimerid = ktimerid;

	    *timerid = (timer_t) newp;
	  }

That said, if I saw it in any other code base I'd consider the code unsafe and recommend rewriting it in a cleaner, safer way.  Here's one attempt:

diff --git a/sysdeps/unix/sysv/linux/timer_create.c b/sysdeps/unix/sysv/linux/timer_create.c
index 370c99a517..f3b309a9b2 100644
--- a/sysdeps/unix/sysv/linux/timer_create.c
+++ b/sysdeps/unix/sysv/linux/timer_create.c
@@ -56,8 +56,19 @@ timer_create (clockid_t clock_id, struct sigevent *evp, timer_t *timerid)
           using struct timer as a derived class with the
           first two elements being in the superclass.  We only
           need these two elements here.  */
-       struct timer *newp = (struct timer *) malloc (offsetof (struct timer,
-                                                               thrfunc));
+       typedef struct
+       {
+         int sigev_notify;
+         kernel_timer_t ktimerid;
+       } timer_superclass;
+
+       static_assert (offsetof (struct timer, sigev_notify)
+                      == offsetof (timer_superclass, sigev_notify));
+       static_assert (offsetof (struct timer, ktimerid)
+                      == offsetof (timer_superclass, ktimerid));
+
+       timer_superclass *newp
+         = (timer_superclass *) malloc (sizeof (timer_superclass));
        if (newp == NULL)
          /* No more memory.  */
          return -1;
Comment 2 Adhemerval Zanella 2020-10-06 18:32:47 UTC
Fixed on 2.33 (7a887dd537cd00fe3cdf42b788b3f0e3b430b0ed)
Comment 3 Sourceware Commits 2020-10-30 21:41:00 UTC
The master branch has been updated by Joseph Myers <jsm28@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=882774658cb8daee4c16677a3fd674f6052cc157

commit 882774658cb8daee4c16677a3fd674f6052cc157
Author: Joseph Myers <joseph@codesourcery.com>
Date:   Fri Oct 30 21:40:25 2020 +0000

    Disable spurious -Warray-bounds for ypclnt.c (bug 26687)
    
    Included among the GCC 11 warnings listed in bug 26687, but not fixed
    when that bug was marked as FIXED, are -Warray-bounds warnings in
    nis/ypclnt.c.  These are all for different calls to the same piece of
    code, which already has a comment explaining that the element accessed
    is in a common prefix of the various structures.  On the basis of that
    comment, this patch treats the warning as a false positive and
    disables it for that code.
    
    Tested with build-many-glibcs.py for arm-linux-gnueabi, where,
    together with my previous two patches, this allows the build of glibc
    to complete with GCC 11 (further build failures appear in the
    testsuite).
    
    Reviewed-by: DJ Delorie <dj@redhat.com>