Bug 77298 - -Wnonnull-compare only emitted for code which is invoked
Summary: -Wnonnull-compare only emitted for code which is invoked
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 6.1.0
: P3 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2016-08-19 15:05 UTC by wipedout
Modified: 2021-08-27 18:53 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 6.1.0, 7.0
Last reconfirmed: 2021-08-27 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description wipedout 2016-08-19 15:05:52 UTC
On http://gcc.godbolt.org/ I select gcc 6.1 and specify -O3 -Wall

The code is:
struct CHandle {
  int handle;
  int GetHandle() const
  {
    if (this != 0) //[-Wnonnull-compare] emitted
      return handle;
    return 0;
  }
};
int main()
{
  CHandle* h = new CHandle();
  return h->GetHandle();
}

Initially [-Wnonnull-compare] is emitted where "this" is checked for null. Now I change main() to this:

int main()
{
  CHandle* h = new CHandle();
  return 0;//h->GetHandle();
}

and the warning is no longer emitted. The code is compiled and it's known at compile time that there's a check of "this" against null yet not warning.
Comment 1 Manuel López-Ibáñez 2016-08-19 15:52:46 UTC
This is by design. Most users do not want to get warnings for code that is not reachable or if the thing compared against NULL has changed and become NULL (https://gcc.gnu.org/PR69835). In your case, the whole function gets optimized out.

This is similar to:

 if (false) {
    if (this != 0)
      return handle;
 }
Comment 2 Martin Sebor 2016-08-19 15:59:31 UTC
I was about to comment on this but Manuel beat me to it.

The problem seems to be that the function is defined inline.  When it's out-of-line, the warning is emitted.

I'm inclined to agree with the submitter.  The warning should be issued consistently for inline functions as well as out-of-line ones, regardless of whether they are called.  Otherwise it will miss the problem in inline functions in class libraries (that aren't being sufficiently tested).

$ cat z.C && /build/gcc-trunk-svn/gcc/xgcc -B /build/gcc-trunk-svn/gcc -O0 -Wall -Wextra z.C
struct A {
  int f () { return this ? 1 : 0; }
};

struct B {
  int f () { return this ? 1 : 0; }   // missing -Wnonnull-compare
};

struct C {
  int f ();
};

int C::f () { return this ? 1 : 0; }

int main()
{
  return A ().f ();
}
z.C: In member function ‘int A::f()’:
z.C:2:26: warning: nonnull argument ‘this’ compared to NULL [-Wnonnull-compare]
   int f () { return this ? 1 : 0; }
                     ~~~~~^~~~~~~
z.C: In member function ‘int C::f()’:
z.C:13:27: warning: nonnull argument ‘this’ compared to NULL [-Wnonnull-compare]
 int C::f () { return this ? 1 : 0; }
                      ~~~~~^~~~~~~
Comment 3 Martin Sebor 2016-08-19 16:11:23 UTC
I should add that Clang issues the warning for all three functions:

$ /build/llvm-trunk/bin/clang -S -Wall -Wextra -Wpedantic z.C
z.C:2:21: warning: 'this' pointer cannot be null in well-defined C++ code;
      pointer may be assumed to always convert to true
      [-Wundefined-bool-conversion]
  int f () { return this ? 1 : 0; }
                    ^~~~ ~
z.C:6:21: warning: 'this' pointer cannot be null in well-defined C++ code;
      pointer may be assumed to always convert to true
      [-Wundefined-bool-conversion]
  int f () { return this ? 1 : 0; }
                    ^~~~ ~
z.C:13:22: warning: 'this' pointer cannot be null in well-defined C++ code;
      pointer may be assumed to always convert to true
      [-Wundefined-bool-conversion]
int C::f () { return this ? 1 : 0; }
                     ^~~~ ~
3 warnings generated.
Comment 4 Jakub Jelinek 2016-08-19 16:48:09 UTC
The warning for proper behavior needs the function being gimplified and in SSA form, unless the FE start doing that themselves, otherwise it can't generally determine if an argument is known to be non-NULL or has been reassigned afterwards and no guarantees are known about it.
Of course, this is quite a special case, because this isn't a parameter one can change in the function in any way, so theoretically we could warn for this in the FE.
Comment 5 Jakub Jelinek 2016-08-19 16:50:42 UTC
You can always use -fkeep-inline-functions to get the warning even for unused inlines (by forcing them to be emitted). -Wnonnull-compare isn't the only warning that isn't performed in the FEs early, think about -Wuninitialized and -Wmaybe-uninitialized and many others.  Those have the same behavior here.
Comment 6 Martin Sebor 2016-08-19 17:33:22 UTC
Yes, that makes sense to me as an explanation of the limitation of the GCC implementation and a solution/workaround for it.   I don't think it's something users unfamiliar with GCC internals think of, though.  I don't have a sense of how much work it would be to change GCC to issue these warnings consistently, whether or not functions are used, but I suspect it's not a trivial "bug fix."

Until this change is made (if it ever is made), it seems that documenting the limitation and the -fkeep-inline-functions workaround might help users who are relying on these warnings to be consistent.  If you all agree I volunteer to reopen thus bug and add something to the manual.  Yes?
Comment 7 Jakub Jelinek 2016-08-19 17:44:50 UTC
Different warnings are simply done at different compilation phases.  This is similar to how you get only a subset of FE warnings on uninstantiated templates, only something can be warned reliably at that phase, and something is just too hard to warn at that phase.  So, lots of warnings you get only when actually instantiating the templates.
Comment 8 Manuel López-Ibáñez 2016-08-19 19:42:27 UTC
(In reply to Martin Sebor from comment #6)
> Yes, that makes sense to me as an explanation of the limitation of the GCC
> implementation and a solution/workaround for it.   I don't think it's
> something users unfamiliar with GCC internals think of, though.  I don't
> have a sense of how much work it would be to change GCC to issue these
> warnings consistently, whether or not functions are used, but I suspect it's
> not a trivial "bug fix."

As Jakub said, making the warning consistent for "this" is not hard. You could warn in the FE for 'this', then skip 'this' when warning in the ME.

Warning the FE is trivial: https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/cp/typeck.c?r1=233472&r2=233471&pathrev=233472

and it will allow you to give a nicer, more specific message like Clang does.

(I think there is even a PR about the message)

Feel free to reopen this PR and refashion it with that purpose.
Comment 9 Martin Sebor 2016-08-21 22:42:58 UTC
Reopening as valid, though I don't expect to have the time to work on this anytime soon (if at all).