[RFC patch] avoid warning on accesses to hardwired address

[RFC patch] avoid warning on accesses to hardwired address

[Martin Sebor via Libc-alpha]

[-- Attachment #1: Type: text/plain, Size: 1457 bytes --]

Thanks to recent code refactoring in GCC 12, -Warray-bounds has
started to diagnose accesses to constant addresses just like many
other flow based warnings do (e.g., -Wstringop-overflow).
The warnings are meant to help detect accesses resulting from
invalid arithmetic on null pointers.  There may be a better way
to detect them but GCC doesn't have the detection yet.  This
warning change has in turn exposed Glibc's uses of this trick
in the implementation of the THREAD_SELF macro.

I have tried a few approaches to avoid the warning but none worked
or seemed satisfactory:

1) Using #pragma GCC diagnostic doesn't work in macros.
2) Using _Pragma doesn't work there either due to a GCC limitation.
3) Declaring the pointer volatile works but prevents accesses to
    it from being eliminated when the result isn't used (something
    Glibc apparently cares about).
4) Defining a simple static inline function to wrap the code and
    the #pragmas doesn't work because the header is #included in
    files where struct pthread isn't defined.
5) Introducing a global variable with the address and initializing
    it in some .c file seems too heavy-weight.
6) Falling back on the pre-GCC 6 asm would be safer but seems like
    a step back.

Finally I have come up with the attached solution that combines (4)
and (1).  If (6) is preferable I'm happy to go that route.  If there
are other alternatives I'd be glad to consider them as well.

Thanks
Martin

[-- Attachment #2: glibc-thread_self.diff --]
[-- Type: text/x-patch, Size: 974 bytes --]

diff --git a/sysdeps/x86_64/nptl/tls.h b/sysdeps/x86_64/nptl/tls.h
index a78c4f4d01..a5bd245aa9 100644
--- a/sysdeps/x86_64/nptl/tls.h
+++ b/sysdeps/x86_64/nptl/tls.h
@@ -180,8 +180,20 @@ _Static_assert (offsetof (tcbhead_t, __glibc_unused2) == 0x80,
    assignments like
 	pthread_descr self = thread_self();
    do not get optimized away.  */
-# if __GNUC_PREREQ (6, 0)
-#  define THREAD_SELF \
+
+# if __GNUC_PREREQ (12, 0)
+/* Hide access to a hardwided address to avoid GCC -Warray-bounds.  */
+static inline void *__thread_self (size_t __off)
+{
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Warray-bounds"
+  return *(void *__seg_fs *) __off;
+#pragma GCC diagnostic pop
+}
+#  define THREAD_SELF							\
+  ((struct pthread *)__thread_self (offsetof (struct pthread, header.self)))
+# elif __GNUC_PREREQ (6, 0)
+#  define THREAD_SELF							\
   (*(struct pthread *__seg_fs *) offsetof (struct pthread, header.self))
 # else
 #  define THREAD_SELF \

Re: [RFC patch] avoid warning on accesses to hardwired address

[Florian Weimer via Libc-alpha]

* Martin Sebor via Libc-alpha:

> Thanks to recent code refactoring in GCC 12, -Warray-bounds has
> started to diagnose accesses to constant addresses just like many
> other flow based warnings do (e.g., -Wstringop-overflow).
> The warnings are meant to help detect accesses resulting from
> invalid arithmetic on null pointers.  There may be a better way
> to detect them but GCC doesn't have the detection yet.  This
> warning change has in turn exposed Glibc's uses of this trick
> in the implementation of the THREAD_SELF macro.

The warning needs to be disabled in GCC for named address spaces.  Null
pointers are not special for them.

Thanks,
Florian

Re: [RFC patch] avoid warning on accesses to hardwired address

[Florian Weimer via Libc-alpha]

* Florian Weimer:

> * Martin Sebor via Libc-alpha:
>
>> Thanks to recent code refactoring in GCC 12, -Warray-bounds has
>> started to diagnose accesses to constant addresses just like many
>> other flow based warnings do (e.g., -Wstringop-overflow).
>> The warnings are meant to help detect accesses resulting from
>> invalid arithmetic on null pointers.  There may be a better way
>> to detect them but GCC doesn't have the detection yet.  This
>> warning change has in turn exposed Glibc's uses of this trick
>> in the implementation of the THREAD_SELF macro.
>
> The warning needs to be disabled in GCC for named address spaces.  Null
> pointers are not special for them.

Is there already a GCC PR for this warning regression?

Thanks,
Florian

Re: [RFC patch] avoid warning on accesses to hardwired address

[Martin Sebor via Libc-alpha]

On 7/21/21 7:06 AM, Florian Weimer wrote:
> * Florian Weimer:
> 
>> * Martin Sebor via Libc-alpha:
>>
>>> Thanks to recent code refactoring in GCC 12, -Warray-bounds has
>>> started to diagnose accesses to constant addresses just like many
>>> other flow based warnings do (e.g., -Wstringop-overflow).
>>> The warnings are meant to help detect accesses resulting from
>>> invalid arithmetic on null pointers.  There may be a better way
>>> to detect them but GCC doesn't have the detection yet.  This
>>> warning change has in turn exposed Glibc's uses of this trick
>>> in the implementation of the THREAD_SELF macro.
>>
>> The warning needs to be disabled in GCC for named address spaces.  Null
>> pointers are not special for them.
> 
> Is there already a GCC PR for this warning regression?

Not one about null pointers and named address spaces.  If you could
open one and point to where the spec allows null pointers to be 
dereferenced that would be helpful.

Thanks
Martin

Re: [RFC patch] avoid warning on accesses to hardwired address

[Martin Liška]

Hello.

Small note here, the patch is incomplete as i386 target still fails:

[   96s] cc1: all warnings being treated as errors
[   96s] make[2]: *** [../o-iterator.mk:9: /home/abuild/rpmbuild/BUILD/glibc-2.34.9000.39.g6e8a0aac2f/cc-base/intl/loadmsgcat.o] Error 1
[   96s] make[2]: *** Waiting for unfinished jobs....
[   96s] In file included from ../sysdeps/i386/i686/nptl/tls.h:33,
[   96s]                  from ../sysdeps/generic/libc-tsd.h:44,
[   96s]                  from ../include/../locale/localeinfo.h:224,
[   96s]                  from ../include/ctype.h:26,
[   96s]                  from loadmsgcat.c:29:
[   96s] loadmsgcat.c: In function '_nl_load_domain':
[   96s] ../sysdeps/i386/nptl/tls.h:239:4: error: array subscript 0 is outside array bounds of '__seg_gs struct pthread * __seg_gs[0]' [-Werror=array-bounds]
[   96s]   239 |   (*(struct pthread *__seg_gs *) offsetof (struct pthread, header.self))
[   96s]       |   ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[   96s] ../sysdeps/nptl/libc-lock.h:92:18: note: in expansion of macro 'THREAD_SELF'
[   96s]    92 |     void *self = THREAD_SELF;                                                 \
[   96s]       |                  ^~~~~~~~~~~
[   96s] loadmsgcat.c:770:3: note: in expansion of macro '__libc_lock_lock_recursive'
[   96s]   770 |   __libc_lock_lock_recursive (lock);
[   96s]       |   ^~~~~~~~~~~~~~~~~~~~~~~~~~

Re: [RFC patch] avoid warning on accesses to hardwired address

[Martin Sebor via Libc-alpha]

On 8/16/21 5:27 AM, Martin Liška wrote:
> Hello.
> 
> Small note here, the patch is incomplete as i386 target still fails:

The Glibc patch I submitted wasn't approved and I haven't implemented
the GCC change yet to handle named address spaces specially.  It's on
my radar but will take some time to resolve.

Martin

> 
> [   96s] cc1: all warnings being treated as errors
> [   96s] make[2]: *** [../o-iterator.mk:9: 
> /home/abuild/rpmbuild/BUILD/glibc-2.34.9000.39.g6e8a0aac2f/cc-base/intl/loadmsgcat.o] 
> Error 1
> [   96s] make[2]: *** Waiting for unfinished jobs....
> [   96s] In file included from ../sysdeps/i386/i686/nptl/tls.h:33,
> [   96s]                  from ../sysdeps/generic/libc-tsd.h:44,
> [   96s]                  from ../include/../locale/localeinfo.h:224,
> [   96s]                  from ../include/ctype.h:26,
> [   96s]                  from loadmsgcat.c:29:
> [   96s] loadmsgcat.c: In function '_nl_load_domain':
> [   96s] ../sysdeps/i386/nptl/tls.h:239:4: error: array subscript 0 is 
> outside array bounds of '__seg_gs struct pthread * __seg_gs[0]' 
> [-Werror=array-bounds]
> [   96s]   239 |   (*(struct pthread *__seg_gs *) offsetof (struct 
> pthread, header.self))
> [   96s]       |   
> ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> [   96s] ../sysdeps/nptl/libc-lock.h:92:18: note: in expansion of macro 
> 'THREAD_SELF'
> [   96s]    92 |     void *self = 
> THREAD_SELF;                                                 \
> [   96s]       |                  ^~~~~~~~~~~
> [   96s] loadmsgcat.c:770:3: note: in expansion of macro 
> '__libc_lock_lock_recursive'
> [   96s]   770 |   __libc_lock_lock_recursive (lock);
> [   96s]       |   ^~~~~~~~~~~~~~~~~~~~~~~~~~
>