[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 #10 from Martin Sebor --- The main purpose of the "partly outside" warning is to detect reads/writes that cross the trailing array boundary. Those typically come up when a "typeless" buffer (either char array or one allocated by a function like malloc) is used to store a sequence of heterogeneous elements. Besides these bugs the warning also detects other kinds of invalid accesses that span the boundary, like in comment #0. The access there in invalid for a different reason than the usual -Warray-bounds, so issuing a warning that's controlled by a different option would be appropriate (-Wstrict-aliasing seems like a good fit). It's something we have discussed doing but it's not at the top my to do list. Making it conditional on -fstrict-aliasing might be worth considering as well.
[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 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 #8 from Richard Biener --- Note the example clearly violates C TBAA rules and the "optimization" of eliding the data member(s) for 'head' are invalid. It's not only about diagnostics but about wrong-code generation waiting to happen (unless you use -fno-strict-aliasing). If you access only the list head part then use a pointer to list for the access. Note I don't see the actual bogus access (the accesses seem to be guarded with >list != ) but the warning might be exposed by path isolating of actually unreachable code.
[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 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 Martin Sebor changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution|--- |INVALID --- Comment #6 from Martin Sebor --- In the test case in comment #0 the operand of the return statement in first() dereferences the tmp pointer: return tmp->list.n; The expression is equivalent to (*tmp).list.n where the dereference should be more obvious. The dereference is invalid if tmp points to an object of an incompatible type. This is a basic type-based aliasing requirement that GCC relies on. If the test case isn't representative of the problem you see in the code base please submit one that does.
[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 since r11-3306-g3f9a497d1b0dd9da
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98503 Martin Sebor changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |INVALID --- Comment #3 from Martin Sebor --- The phrasing of the warning could stand to be made clearer but it is by design. With the exception of a union with a /common initial sequence/, it's not valid to access [members of] an object of one type (struct list) using a pointer to an incompatible type (struct ref). It doesn't matter if the offset of the member is less than the size of the object.
[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 Martin Liška changed: What|Removed |Added Known to fail||11.0 Summary|[11 regression] |[11 regression] |-Warray-bounds false|-Warray-bounds false |positive with global|positive with global |variables at -O2|variables at -O2 since ||r11-3306-g3f9a497d1b0dd9da Known to work||10.2.0 CC||marxin at gcc dot gnu.org, ||msebor at gcc dot gnu.org Status|UNCONFIRMED |NEW Last reconfirmed||2021-01-04 Ever confirmed|0 |1 Target Milestone|--- |11.0 --- Comment #2 from Martin Liška --- Thank you for the report, started with r11-3306-g3f9a497d1b0dd9da.
[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.
