[Bug middle-end/87489] [8/9/10/11 Regression] Spurious -Wnonnull warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87489
--- Comment #16 from Martin Sebor ---
The test case in
https://gcc.gnu.org/pipermail/gcc-patches/2021-February/565564.html shows that
running the -Wnonnull pass later, after FRE, allows the warning to detect the
bug in the following test case:
class b {
public:
long c();
};
class B {
public:
B() : f() {}
b *f;
};
long d, e;
class g : B {
public:
void h() {
long a = f->c(); <<< -Wnonnull
d = e = a;
}
};
class j {
j();
g i;
};
j::j() { i.h(); }
My response
(https://gcc.gnu.org/pipermail/gcc-patches/2021-February/565590.html) is copied
below:
> Thanks. Yes, the warning would be useful here since the f pointer
> in the call to f->c() is null when i.h() is called from j's ctor.
> The FRE3 pass clearly exposes this :
>
> void j::j (struct j * const this)
> {
>long int _9;
>
> [local count: 1073741824]:
>MEM[(struct B *)this_3(D)] ={v} {CLOBBER};
>MEM[(struct B *)this_3(D)].f = 0B;
>_9 = b::c (0B);
>...
>
> Because the warning runs early (after CCP2), the null pointer is
> not detected. To see it the warning code would have to work quite
> hard to figure out that the two assignments below refer to the same
> location (it would essentially have to do what FRE does):
>
>MEM[(struct B *)this_3(D)].f = 0B;
>_7 = MEM[(struct b * *)_1];
>_9 = b::c (_7);
>
> It's probably too late to make this change for GCC 11 as Jeff has
> already decided that it should be deferred until GCC 12, and even
> then there's a concern that doing so might cause false positives.
> I think it's worth revisiting the idea in GCC 12 to see if
> the concern is founded. Let me make a note of it in the bug.
[Bug middle-end/87489] [8/9/10/11 Regression] Spurious -Wnonnull warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87489 Jeffrey A. Law changed: What|Removed |Added Assignee|msebor at gcc dot gnu.org |law at gcc dot gnu.org Target Milestone|11.0|12.0 --- Comment #15 from Jeffrey A. Law --- I'm explicitly pushing this out of gcc-11. As I've mentioned in the thread for Martin's patch, reordering passes is generally not the right approach to solving most issues. I've got a proof-of-concept improvement to the jump threader that nearly catches this case that I'm attaching to this case so it doesn't get lost. The most important missing bit in that patch is when we have two statements in a block that must be considered, it gives up. While that was a reasonable limitation in the past, it's one we need to fix anyway and this BZ is a good motivator. While I'm confident that could be fixed and that we'd handle this issue, I'm not comfortable dropping in improvements like this into gcc-11 at this stage. Hence the explicit deferment to gcc-12.
[Bug middle-end/87489] [8/9/10/11 Regression] Spurious -Wnonnull warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87489 Martin Sebor changed: What|Removed |Added Keywords|deferred|patch Assignee|unassigned at gcc dot gnu.org |msebor at gcc dot gnu.org Status|NEW |ASSIGNED --- Comment #14 from Martin Sebor --- Patch posted for review: https://gcc.gnu.org/pipermail/gcc-patches/2021-February/564597.html
[Bug middle-end/87489] [8/9/10/11 Regression] Spurious -Wnonnull warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87489 Martin Sebor changed: What|Removed |Added Attachment #50098|0 |1 is obsolete|| --- Comment #13 from Martin Sebor --- Created attachment 50099 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50099=edit Simplified patch to run -Wnonnull later. Rather than moving the implementation of the warning around this simplified patch moves the pass instead.
[Bug middle-end/87489] [8/9/10/11 Regression] Spurious -Wnonnull warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87489
--- Comment #12 from Martin Sebor ---
Created attachment 50098
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50098=edit
Patch to move -Wnonnull to pass_merge_phi.
No change in GCC 11 so far. I've reduced the test case in attachment 44775 to
just the essentials and copied it below. The dumps confirm that the warning in
this case is issued too early, before the strlen(0B) call has been eliminated.
As Jeff notes in comment #11, FRE3 is the first pass where the bad strlen call
has been removed.
$ cat pr87489.c && gcc -O -S -Wall -Wpedantic -fdump-tree-all pr87489.c && grep
strlen * | grep 0B
extern void f (const void*, int);
static void g (int i, const char *s)
{
int j = 0;
if (i)
j |= 1;
if (j)
f (, 0);
if (j & 1)
f (s, __builtin_strlen (s));
}
void h (void)
{
g (0, 0);
}
In function ‘g’,
inlined from ‘h’ at ../pr87489.c:19:3:
pr87489.c:14:11: warning: argument 1 null where non-null expected [-Wnonnull]
14 | f (s, __builtin_strlen (s));
| ^~~~
pr87489.c: In function ‘h’:
pr87489.c:14:11: note: in a call to built-in function ‘__builtin_strlen’
pr87489.c.092t.fixup_cfg3: _6 = __builtin_strlen (0B);
pr87489.c.097t.adjust_alignment: _6 = __builtin_strlen (0B);
pr87489.c.098t.ccp2: _6 = __builtin_strlen (0B);
pr87489.c.099t.post_ipa_warn1: _6 = __builtin_strlen (0B);
pr87489.c.101t.backprop: _6 = __builtin_strlen (0B);
pr87489.c.102t.phiprop: _6 = __builtin_strlen (0B);
pr87489.c.103t.forwprop2: _6 = __builtin_strlen (0B);
pr87489.c.104t.objsz2: _6 = __builtin_strlen (0B);
pr87489.c.105t.alias: _6 = __builtin_strlen (0B);
pr87489.c.106t.retslot: _6 = __builtin_strlen (0B);
As an experiment, I tried running -Wnonnull later, in mergephi2 (see the
attached patch titled gcc-87489.diff). That avoids the warning, passes
bootstrap, and causes no testsuite regressions.
