[Bug middle-end/98503] [11/12 regression] -Warray-bounds false positive with global variables at -O2 since r11-3306-g3f9a497d1b0dd9da
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98503
--- Comment #19 from Willy Tarreau ---
Hello Richard,
Thanks for looking at this old issue.
> that means next_step will do the bad access of 'head' using the struct ref
> type.
Actually there's no bad access, as if you look closely, next_step() restarts
from cur->list.n, hence undoes exactly what initial_step() did.
In addition for the special case here, the list happens to be at the beginning
of the struct so there's not even an offset of difference between the list head
and the member, so get_initial_step() really returns head here, and next_step()
will return exactly head as well, which does match the stop condition in the
following while() block and never causes any dereference of any unmapped area.
I noticed I have left a mistake when writing the simplified reproducer, I
didn't initialise head, thus I thought it could have contributed to the warning
in the reproducer, but retesting with:
static struct list head = { .n = , .p = };
doesn't change anything.
Since then we've worked around this problem by using some ugly casts because
removing the warning could have more long-term consequences if it occasionally
allows to trigger on a real issue. Still I find it sad when a warning forces us
to introduce dangerous casts as a workaround.
[Bug middle-end/98503] [11/12 regression] -Warray-bounds false positive with global variables at -O2 since r11-3306-g3f9a497d1b0dd9da
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98503 --- Comment #14 from Willy Tarreau --- (In reply to Martin Sebor from comment #13) > The patch was rejected so we'll have to live with a confusing warning. sadly it's not the first one and I don't count the number of bugs I have introduced in my code just to try to work around inappropriate warnings :-( It's really sad that over time code is getting more and more fragile instead of getting more and more robust :-(
[Bug c/98503] [11 regression] -Warray-bounds false positive with global variables at -O2 since r11-3306-g3f9a497d1b0dd9da
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98503 --- Comment #9 from Willy Tarreau --- Hi Richard, indeed, the >list == is the test for end of list that prevents any bad access from happening. I know that usually the right way to do this is by using a list element, but sometimes it requires placing casts all over the code, or container_of() and friends in every single call, which is way more error-prone. Writing state machines with different input types in general doesn't result in reliable long-term code. It turns out that in the code affected by this there were only two call places that I could tear a little bit to use the list instead (and an alias pointer, which I hate keeping) but I'm not extremely happy with this workaround. I'm well aware that there can be aliasing issues while doing this, which is also why I tested with -fno-strict-aliasing and saw the warninig remain, which I'd argue is definitely not appropriate in this case. I really think that this one is border-line. Not useless at all, but should only trigger at a higher level so that users don't have to get rid of -Warray-bounds just because of it. The linux kernel already had to disable it for other reasons and that's really sad in my opinion, which is why I'm trying hard to keep it. Thanks, Willy
[Bug tree-optimization/56456] [meta-bug] bogus/missing -Warray-bounds
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56456 Bug 56456 depends on bug 98503, which changed state. Bug 98503 Summary: [11 regression] -Warray-bounds false positive with global variables at -O2 since r11-3306-g3f9a497d1b0dd9da https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98503 What|Removed |Added Status|RESOLVED|REOPENED Resolution|INVALID |---
[Bug c/98503] [11 regression] -Warray-bounds false positive with global variables at -O2 since r11-3306-g3f9a497d1b0dd9da
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98503
Willy Tarreau changed:
What|Removed |Added
Resolution|INVALID |---
Status|RESOLVED|REOPENED
--- Comment #7 from Willy Tarreau ---
It's not easy because as often with optimizations, depending on the code moves
I'm doing, the issue appears and disappears. The closest looking one is below.
The general idea is that we're having a function that is called to dump a
certain number of elements to a buffer and return when the buffer is full, to
give back control to other parts of the code, then it's called again from the
last list element where the dump was started. A common practice with such
interruptible processing of list consists in starting from the head and letting
the dump function iterate. This would roughly look like this (still simplified
quite a bit to grasp the principle). The code below manages to trigger the
warning. If you're interested in the original code (I doubt it), it's on this
line: https://github.com/haproxy/haproxy/blob/master/src/ssl_sock.c#L6366 and
the initial call is made here:
https://github.com/haproxy/haproxy/blob/master/src/ssl_sock.c#L6437
#include
#define container_of(ptr, type, member) ({ \
void *__mptr = (void *)(ptr); \
((type *)(__mptr - __builtin_offsetof(type, member))); })
// returns number of bytes emitted or 0 if it failed to dump.
extern int try_dump_string(const char *name);
struct list {
struct list *n;
struct list *p;
};
static struct list head;
struct ref {
struct list list;
char *filename;
};
static struct ref *last_dumped;
/* try to dump one ref, returns 0 on failure */
static inline int try_dump_next_ref(struct ref *ref)
{
return try_dump_string(ref->filename);
}
static inline struct ref *get_initial_step()
{
return container_of(, struct ref, list);
}
static inline struct ref *next_step(struct ref *cur)
{
return container_of(cur->list.n, struct ref, list);
}
/* restarts a dump from dump_context or starts a new one if NULL */
struct ref *restart_dump(struct ref *last)
{
struct ref *curr;
if (!last)
last = get_initial_step();
curr = next_step(last);
while (>list != ) {
if (try_dump_next_ref(curr)) {
last = curr;
curr = next_step(curr);
} else {
/* do something to yield or flush the output */
}
}
return last;
}
void cont_dump()
{
restart_dump(last_dumped);
}
Please note that here "last_dump" isn't updated and if the compiler sees it
being written to, it eliminates some optimizations that result in the warning
(I think it sees that the get_initial_step() is conditional and thus doesn't
complain anymore).
I understand from your description that there could be an aliasing issue. I'd
argue that as soon as we're using lists there are potential aliasing issues and
that these may then be addressed using -fno-strict-aliasing but this one has no
effect here either.
Also I suspect based on your description, that accessing *any* field of a
struct implies the whole struct must be mapped in memory. But then there are
limits in case of aliased struct like above, or even when using VLAs since you
can't figure the possible extent of the variable part.
I think I see how I could cheat to eliminate the warning (not easy to test as I
don't have the trunk version of the compiler locally), but I suspect that doing
so would definitely make the code more error-prone.
I sincerely think that this *exact* case is counter-productive as it will
basically force a number of users of lists to disable the whole warning, which
was otherwise particularly effective at detecting real out-of-bounds accesses,
especially in such use cases. Or maybe you could assign it a different level
(-Warray-bounds=3 ?) to indicate a suspicious usage that might also be a valid
one ?
Thanks!
Willy
[Bug tree-optimization/56456] [meta-bug] bogus/missing -Warray-bounds
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56456 Bug 56456 depends on bug 98503, which changed state. Bug 98503 Summary: [11 regression] -Warray-bounds false positive with global variables at -O2 since r11-3306-g3f9a497d1b0dd9da https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98503 What|Removed |Added Status|RESOLVED|REOPENED Resolution|INVALID |---
[Bug c/98503] [11 regression] -Warray-bounds false positive with global variables at -O2 since r11-3306-g3f9a497d1b0dd9da
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98503 Willy Tarreau changed: What|Removed |Added Resolution|INVALID |--- Status|RESOLVED|REOPENED --- Comment #5 from Willy Tarreau --- apparently I closed it by accident, reopening, sorry.
[Bug c/98503] [11 regression] -Warray-bounds false positive with global variables at -O2 since r11-3306-g3f9a497d1b0dd9da
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98503 --- Comment #4 from Willy Tarreau --- Hi Martin, I'm sorry but I'm missing something, as this is how linked lists are implemented everywhere nowadays. I'm not actually casting the pointer, it was made for simplification. I'm only following the list elements which are linked together from a list head accessed via a container_of. The code does nothing but follow the list from the head (which is only a struct list) and visiting all nodes in turn. There is absolutely zero dereference of a list using a wrong pointer here.
[Bug c/98503] [11 regression] -Warray-bounds false positive with global variables at -O2
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98503 --- Comment #1 from Willy Tarreau --- Re-reading godbolt's version more closely, it's just 1 day old (I thought I read 20201010) so the issue is still valid.
[Bug c/98503] New: [11 regression] -Warray-bounds false positive with global variables at -O2
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98503
Bug ID: 98503
Summary: [11 regression] -Warray-bounds false positive with
global variables at -O2
Product: gcc
Version: 11.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c
Assignee: unassigned at gcc dot gnu.org
Reporter: w at 1wt dot eu
Target Milestone: ---
First, please note that I saw a handful of related reports but couldn't judge
if this one might be merged with another one; if you think it can, feel free to
do so!
We've had build errors reported on haproxy with gcc-11 due to walking over a
list whose head is in a global variable. We could simplify the reproducer to
this and confirm it on godbolt.org:
#include
struct list {
struct list *n;
struct list *p;
};
struct list head;
struct ref {
struct list list;
char *filename;
};
struct list *first(void)
{
struct ref *tmp;
/*tmp = (struct ref *)((char *) - (size_t)&((struct ref
*)0)->list);*/
tmp = (struct ref *)
//asm("" : "+rm"(tmp)); // hide tmp's origin to shut the warning.
//asm("" : "=rm"(tmp) : "0"()); // discretely assign tmp from head
return tmp->list.n;
}
Note that only the "ref->list" part is accessed here, hence it exactly matches
"head" (both of type struct list). But since gcc 11 (and only this one, 1.27 to
10.2 are OK), we're getting this:
: In function 'first':
:22:19: warning: array subscript 'struct ref[0]' is partly outside
array bounds of 'struct list[1]' [-Warray-bounds]
22 | return tmp->list.n;
| ^~
:8:13: note: while referencing 'head'
8 | struct list head;
| ^~~~
Compiler returned: 0
Changing "head" to a function argument makes the warning disappear, building at
-O1 as well, and obviously, cheating with the asm statement to hide "tmp"
solves it (though it degrades the code quality).
Removing the unused "filename" from struct "ref" removes the warning. It looks
to me as if the compiler doesn't know what part of a structure is being
dereferenced when we access any member, making the warning systematically bogus
for embedded members accessed via container_of() and friends when the list's
head is global.
For now we can disable the warning, I've confirmed the generated code is
correct, but since it's the first time we have a false positive on this one,
I'd prefer if we can keep it as much as possible as it's one which can
definitely help spot real bugs.
I've just verified the version used on godbolt.org, it's "gcc version 11.0.0
20210101 (experimental) (Compiler-Explorer-Build)". I haven't rebuilt the trunk
locally so it might be possible this bug got solved since as part of one of the
few related ones.
Thanks!
Willy
