[RFC] avoid warning on accesses to hardwired address
Checks
| Context |
Check |
Description |
| dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| dj/TryBot-32bit |
success
|
Build for i686
|
Commit Message
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
Comments
* 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
* 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
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
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] | ^~~~~~~~~~~~~~~~~~~~~~~~~~
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] | ^~~~~~~~~~~~~~~~~~~~~~~~~~
>
@@ -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 \