[Bug middle-end/87489] [8/9/10/11 Regression] Spurious -Wnonnull warning

[Bug middle-end/87489] [8/9/10/11 Regression] Spurious -Wnonnull warning

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87489

--- Comment #12 from Martin Sebor <msebor at gcc dot gnu.org> ---
Created attachment 50098
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50098&action=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 (&j, 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.

[Bug middle-end/87489] [8/9/10/11 Regression] Spurious -Wnonnull warning

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87489

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #50098|0                           |1
        is obsolete|                            |

--- Comment #13 from Martin Sebor <msebor at gcc dot gnu.org> ---
Created attachment 50099
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50099&action=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

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87489

Martin Sebor <msebor at gcc dot gnu.org> 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 <msebor at gcc dot gnu.org> ---
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

[law at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87489

Jeffrey A. Law <law at redhat dot com> 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 <law at redhat dot com> ---
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

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87489

--- Comment #16 from Martin Sebor <msebor at gcc dot gnu.org> ---
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;
> 
>    <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.

[Bug middle-end/87489] [9/10/11/12/13 Regression] Spurious -Wnonnull warning

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87489

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|12.0                        |12.2

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 12.1 is being released, retargeting bugs to GCC 12.2.

[Bug middle-end/87489] [10/11/12/13 Regression] Spurious -Wnonnull warning

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87489

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|2020-06-03 00:00:00         |2022-12-5

--- Comment #19 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Andrew Church from comment #5)
> Simpler testcase (based on the testcase in bug 87041):
> 
> extern int strcmp(const char *a, const char *b) __attribute__((nonnull(1,
> 2)));
> int foo(void) {
>     const char * const s = 0;
>     if (s)
>         return strcmp(s, "");
>     else
>         return 2;
> }
> 
> foo.c: In function 'foo':
> foo.c:5:16: warning: null argument where non-null required (argument 1)
> [-Wnonnull]
>          return strcmp(s, "");
>                 ^~~~~~

This one warns in the frontend where we substituted the const char * const s
initializer:

#0  warning_at (location=258981, opt=697, gmsgid=0x2fe4c10 "argument %u null
where non-null expected")
    at /home/rguenther/src/trunk/gcc/diagnostic.cc:1876
#1  0x0000000000d2bf3b in check_nonnull_arg (ctx=0x7fffffffc440,
param=<integer_cst 0x7ffff641b9a8>, param_num=1)
    at /home/rguenther/src/trunk/gcc/c-family/c-common.cc:5822
#2  0x0000000000d2d22a in check_function_arguments_recurse (callback=0xd2bcdc
<check_nonnull_arg(void*, tree, unsigned long)>, 
    ctx=0x7fffffffc440, param=<integer_cst 0x7ffff641b9a8>, param_num=1,
opt=OPT_Wnonnull)
    at /home/rguenther/src/trunk/gcc/c-family/c-common.cc:6195
#3  0x0000000000d2b4c9 in check_function_nonnull (ctx=..., nargs=2,
argarray=0x7ffff640bc60)
    at /home/rguenther/src/trunk/gcc/c-family/c-common.cc:5621
#4  0x0000000000d2cc9b in check_function_arguments (loc=258981,
fndecl=<function_decl 0x7ffff638a600 strcmp>, 
    fntype=<function_type 0x7ffff6417498>, nargs=2, argarray=0x7ffff640bc60,
arglocs=0x7fffffffc4a0)
    at /home/rguenther/src/trunk/gcc/c-family/c-common.cc:6071
#5  0x0000000000c3fad6 in build_function_call_vec (loc=258981, arg_loc=...,
function=<addr_expr 0x7ffff641d3a0>, 
    params=0x7ffff640bc58 = {...}, origtypes=0x7ffff640be60 = {...},
orig_fundecl=<function_decl 0x7ffff638a600 strcmp>)
    at /home/rguenther/src/trunk/gcc/c/c-typeck.cc:3319
#6  0x0000000000c40294 in c_build_function_call_vec (loc=258981, arg_loc=...,
function=<function_decl 0x7ffff638a600 strcmp>, 
    params=0x7ffff640bc58 = {...}, origtypes=0x7ffff640be60 = {...}) at
/home/rguenther/src/trunk/gcc/c/c-typeck.cc:3387
#7  0x0000000000ca02bb in c_parser_postfix_expression_after_primary
(parser=0x7ffff6275c60, expr_loc=258981, expr=...)
    at /home/rguenther/src/trunk/gcc/c/c-parser.cc:11245


Re-confirmed for the other cases.

[Bug middle-end/87489] [10/11/12/13/14 Regression] Spurious -Wnonnull warning

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87489

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|12.3                        |12.4

--- Comment #20 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 12.3 is being released, retargeting bugs to GCC 12.4.