On 2/19/21 2:48 AM, Franz Sirl wrote:
Am 2021-02-01 um 01:31 schrieb Martin Sebor via Gcc-patches:
The initial -Wnonnull implementation in the middle end took place
too late in the pipeline (just before expansion), and as a result
was prone to false positives (bug 78817).  In an attempt to avoid
the worst of those, the warning was moved to the ccp2 pass in
r243874.  However, as the test case in PR 87489 shows, this is
in turn too early and causes other false positives as a result.

A few experiments with running the warning later suggest that
just before the mergephi2 pass is a good point to avoid this class
of false positives without causing any regressions or introducing
any new warnings either in Glibc or in Binutils/GDB.

Since PR 87489 is a GCC 8-11 regression I propose to make this
change on the trunk as well as on the release branches.

Hi Martin,

I tested your patch and it showed also one more warning for this testcase with -O2 -Wnonnull:

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();
     d = e = a;
   }
};
class j {
   j();
   g i;
};
j::j() { i.h(); }

I hope cvise didn't minimize too much, but at least in the original much larger code the warning looks reasonable too.

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;

  <bb 2> [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.

Martin

Reply via email to