Re: Invalid -Wstringop-overread warning for valid POSIX constructs
* Martin Sebor: > Thanks for the reminder. I have not forgotten about this. > I agreed in our discussion and in the GCC bug report where this > came up (PR 101751) that the GCC logic here is wrong and should > be relaxed. I consider it a GCC bug so I plan to make the change > in the bug fixing stage 3. GCC is in the development stage until > the 15th and I've been busy trying to wrap up what I'm working on. > Once it's changed in GCC 12 I'll backport it to GCC 11.3. Does > this timeframe work for you? GCC 11.3 will likely be released in spring 2022, right? That's rather far in the future for any planning. But then the fix is not terribly urgent. At this point I want to make sure that we get the necessary capability in GCC at one pointer. GCC 12 plus backport is fine. Thanks, Florian
Re: Invalid -Wstringop-overread warning for valid POSIX constructs
On 11/4/21 1:03 AM, Florian Weimer via Libc-alpha wrote:
This code:
#include
#include
void
f (pthread_key_t key)
{
pthread_setspecific (key, MAP_FAILED);
}
Results in a warning:
t.c: In function ‘f’:
t.c:7:3: warning: ‘pthread_setspecific’ expecting 1 byte in a region of size 0
[-Wstringop-overread]
7 | pthread_setspecific (key, MAP_FAILED);
| ^
In file included from t.c:1:
/usr/include/pthread.h:1308:12: note: in a call to function
‘pthread_setspecific’ declared with attribute ‘access (none, 2)’
1308 | extern int pthread_setspecific (pthread_key_t __key,
|^~~
This also results in the same warning, for different reasons:
#include
extern int x[1];
void
f (pthread_key_t key)
{
pthread_setspecific (key, [1]);
}
t.c: In function ‘f’:
t.c:8:3: warning: ‘pthread_setspecific’ expecting 1 byte in a region of size 0
[-Wstringop-overread]
8 | pthread_setspecific (key, [1]);
| ^~~~
t.c:3:12: note: at offset 4 into source object ‘x’ of size 4
3 | extern int x[1];
|^
In file included from t.c:1:
/usr/include/pthread.h:1308:12: note: in a call to function
‘pthread_setspecific’ declared with attribute ‘access (none, 2)’
1308 | extern int pthread_setspecific (pthread_key_t __key,
|^~~
The original argument justifying this warning was that passing
non-pointer constants is invalid. But MAP_FAILED is a valid POSIX
pointer constant, so it is allowed here as well. And the second example
shows that the warning also fires for completely valid pointers. So the
none access attribute is clearly not correct here. (“none” requires
that the pointer is valid, there just aren't any accesses to the object
it points to, but the object must exist. Apparently, this is what the
kernel expects for its use of the annotation.)
The root of the problem is the const void * pointer argument. Without
the access attribute, we warn for other examples:
typedef unsigned int pthread_key_t;
int pthread_setspecific (pthread_key_t __key, const void *);
void
f (pthread_key_t key)
{
int x;
pthread_setspecific (key, );
}
t.c: In function ‘f’:
t.c:10:3: warning: ‘x’ may be used uninitialized [-Wmaybe-uninitialized]
10 | pthread_setspecific (key, );
| ^
t.c:4:5: note: by argument 2 of type ‘const void *’ to ‘pthread_setspecific’
declared here
4 | int pthread_setspecific (pthread_key_t __key, const void *);
| ^~~
t.c:9:7: note: ‘x’ declared here
9 | int x;
| ^
This is why we added the none access attribute, but this leads to the
other problem.
We could change glibc to use a different attribute (preferable one that
we can probe using __has_attribute) if one were to become available, and
backport that. But so far, I see nothing on the GCC side, and
GCC PR 102329 seems to have stalled.
Thanks for the reminder. I have not forgotten about this.
I agreed in our discussion and in the GCC bug report where this
came up (PR 101751) that the GCC logic here is wrong and should
be relaxed. I consider it a GCC bug so I plan to make the change
in the bug fixing stage 3. GCC is in the development stage until
the 15th and I've been busy trying to wrap up what I'm working on.
Once it's changed in GCC 12 I'll backport it to GCC 11.3. Does
this timeframe work for you?
Martin
Invalid -Wstringop-overread warning for valid POSIX constructs
This code:
#include
#include
void
f (pthread_key_t key)
{
pthread_setspecific (key, MAP_FAILED);
}
Results in a warning:
t.c: In function ‘f’:
t.c:7:3: warning: ‘pthread_setspecific’ expecting 1 byte in a region of size 0
[-Wstringop-overread]
7 | pthread_setspecific (key, MAP_FAILED);
| ^
In file included from t.c:1:
/usr/include/pthread.h:1308:12: note: in a call to function
‘pthread_setspecific’ declared with attribute ‘access (none, 2)’
1308 | extern int pthread_setspecific (pthread_key_t __key,
|^~~
This also results in the same warning, for different reasons:
#include
extern int x[1];
void
f (pthread_key_t key)
{
pthread_setspecific (key, [1]);
}
t.c: In function ‘f’:
t.c:8:3: warning: ‘pthread_setspecific’ expecting 1 byte in a region of size 0
[-Wstringop-overread]
8 | pthread_setspecific (key, [1]);
| ^~~~
t.c:3:12: note: at offset 4 into source object ‘x’ of size 4
3 | extern int x[1];
|^
In file included from t.c:1:
/usr/include/pthread.h:1308:12: note: in a call to function
‘pthread_setspecific’ declared with attribute ‘access (none, 2)’
1308 | extern int pthread_setspecific (pthread_key_t __key,
|^~~
The original argument justifying this warning was that passing
non-pointer constants is invalid. But MAP_FAILED is a valid POSIX
pointer constant, so it is allowed here as well. And the second example
shows that the warning also fires for completely valid pointers. So the
none access attribute is clearly not correct here. (“none” requires
that the pointer is valid, there just aren't any accesses to the object
it points to, but the object must exist. Apparently, this is what the
kernel expects for its use of the annotation.)
The root of the problem is the const void * pointer argument. Without
the access attribute, we warn for other examples:
typedef unsigned int pthread_key_t;
int pthread_setspecific (pthread_key_t __key, const void *);
void
f (pthread_key_t key)
{
int x;
pthread_setspecific (key, );
}
t.c: In function ‘f’:
t.c:10:3: warning: ‘x’ may be used uninitialized [-Wmaybe-uninitialized]
10 | pthread_setspecific (key, );
| ^
t.c:4:5: note: by argument 2 of type ‘const void *’ to ‘pthread_setspecific’
declared here
4 | int pthread_setspecific (pthread_key_t __key, const void *);
| ^~~
t.c:9:7: note: ‘x’ declared here
9 | int x;
| ^
This is why we added the none access attribute, but this leads to the
other problem.
We could change glibc to use a different attribute (preferable one that
we can probe using __has_attribute) if one were to become available, and
backport that. But so far, I see nothing on the GCC side, and
GCC PR 102329 seems to have stalled.
Thanks,
Florian
