Bug 92718 - [11/12/13/14 Regression] Bogus Wstringop-overflow in __builtin_memset() of an element of array of size 1 of struct
Summary: [11/12/13/14 Regression] Bogus Wstringop-overflow in __builtin_memset() of an...
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 8.3.0
: P2 normal
Target Milestone: 11.5
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic, missed-optimization
Depends on:
Blocks: Wstringop-overflow
  Show dependency treegraph
 
Reported: 2019-11-28 21:28 UTC by Ryan Libby
Modified: 2023-07-07 10:36 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2019-11-28 00:00:00


Attachments
Minimized test case (150 bytes, text/plain)
2019-11-28 21:28 UTC, Ryan Libby
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ryan Libby 2019-11-28 21:28:26 UTC
Created attachment 47392 [details]
Minimized test case

Gcc is emitting bogus, or maybe pessimistic, Wstringop-overflow or
Warray-bounds warnings (depending on warning flags).

$ cat min.c
struct s {
	int x;
};

extern int n;
struct s a[1];

void
f(void)
{
	struct s *ps;
	int i;

	for (i = 0; i < n; i++) {
		ps = &a[i];
		__builtin_memset(ps, 0, sizeof(*ps));
		ps->x = 1;
	}
}

$ /tmp/gcc/bin/bin/gcc -O -c min.c
min.c: In function ‘f’:
min.c:16:3: warning: ‘__builtin_memset’ writing 4 bytes into a region of size 0 overflows the destination [-Wstringop-overflow=]
   16 |   __builtin_memset(ps, 0, sizeof(*ps));
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

$ /tmp/gcc/bin/bin/gcc -Wall -O -c min.c
min.c: In function ‘f’:
min.c:16:3: warning: ‘__builtin_memset’ offset [4, 7] is out of the bounds [0, 4] of object ‘a’ with type ‘struct s[1]’ [-Warray-bounds]
   16 |   __builtin_memset(ps, 0, sizeof(*ps));
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
min.c:6:10: note: ‘a’ declared here
    6 | struct s a[1];
      |          ^

My unprofessional opinion is that it seems like the compiler is emitting
a warning for when i >= 1, but the compiler does not know whether that
is actually possible.  Note that a warning is not emitted if any of
these changes are made:
 - The size of array "a" is changed from 1 to 2.
 - A check of "n" against the array size is inserted:
	if (n > 1)
		return;
 - The assignment of "x" after __builtin_memset() is deleted.
 - The array of struct is changed to array of int.

The warning is emitted as -Wstringop-overflow with the default warning
options.  In latest source, with -Wall, the warning is emitted as
Warray-bounds, but in previous versions (I checked 8.3.0) -Wall emits
Wstringop-overflow.  I'm not sure when this aspect may have changed.

I have tested this case and seen a warning on:
 - gcc 7.4.0
 - gcc 8.3.0
 - gcc current sources (r278812, "10.0.0")

I looked through the bugs linked to bug 88443 and was not immediately
able to identify this as a duplicate of one.  Apologies if it is.
Comment 1 Jakub Jelinek 2019-11-28 21:55:07 UTC
Started with r243419.
evrp is computing strange ranges, [0, 1] rather than just [0, 0] for the iterator.  It is true that &a[1] is still valid, but the memset with non-zero size at that spot or MEM[(struct s *)&a][i_2].x = 1; for i_2 equal to 1 is already UB.
Later on cunroll completely unrolls the loop based on that, into two iterations, the first one contains both the memset and ps->x and the second one just memset (which is UB too) and ps->x already replaced by __builtin_unreachable.
The warning is then during expansion of that second iteration.
Comment 2 Martin Sebor 2019-11-29 19:54:10 UTC
Replacing the memset call with the assignment '*p = (struct s){ 0 };' avoids the warning and also results in better/optimal code.  (As suggested in pr36602, that would be a useful optimization independent of the warning.)
Comment 3 Ryan Libby 2019-11-30 16:28:25 UTC
(In reply to Martin Sebor from comment #2)
> Replacing the memset call with the assignment '*p = (struct s){ 0 };' avoids
> the warning and also results in better/optimal code.  (As suggested in
> pr36602, that would be a useful optimization independent of the warning.)

In the real code that prompted the warning, the struct has some members
which are declared const, which I believe prevents using aggregate
initialization as a workaround (or at least, it produces an error).
Comment 4 Martin Sebor 2019-12-02 16:11:23 UTC
Modifying const members of a struct is undefined, regardless of how it's done: either by assignment or by overwriting it using memset.  GCC doesn't diagnose it yet but I expect it will, maybe as early as in GCC 11, similarly to how G++ diagnoses it.
Comment 5 Jakub Jelinek 2020-03-04 09:34:25 UTC
GCC 8.4.0 has been released, adjusting target milestone.
Comment 6 Jakub Jelinek 2021-05-14 09:52:23 UTC
GCC 8 branch is being closed.
Comment 7 Richard Biener 2021-06-01 08:15:37 UTC
GCC 9.4 is being released, retargeting bugs to GCC 9.5.
Comment 8 Konstantin Kharlamov 2021-07-20 09:43:55 UTC
(I hope a duplicate comment won't appear, my previous one didn't appear for some reason)

Should this be closed? Judging by comments it was fixed, and I also can't reproduce it with the attached testcase on GCC 11.1.0.
Comment 9 Konstantin Kharlamov 2021-07-20 09:47:48 UTC
Omg, I am sorry, please ignore my comment. For some incomprehensible reason bugzilla circles through bug entries upon sending a comment. My comment here was supposed for another report, but then apparently bugzilla circled through to another report, and I after I didn't see my comment I concluded it glitched out and wrote it anew.
Comment 10 Richard Biener 2022-05-27 09:41:35 UTC
GCC 9 branch is being closed
Comment 11 Jakub Jelinek 2022-06-28 10:38:58 UTC
GCC 10.4 is being released, retargeting bugs to GCC 10.5.
Comment 12 Richard Biener 2023-07-07 10:36:15 UTC
GCC 10 branch is being closed.