CallAndMessage false positive for "T* const member" while it's not nullptr

[irishrover]

Bugzilla Link	44492
Version	trunk
OS	All
CC	@devincoughlin,@AnnaZaks,@haoNoQ,@Xazax-hun

Extended Description

The following code emits false-positive

---

include

include

class C { public: int foo(int x) { return x * 2; } };

class A { public: A(C* c, std::string s) : c(c), s(s) { assert(c); bar("12345"); c->foo(42); } void bar(std::string s) { // s, not c! // c is not changed here. s = s; }

private: std::string s; // [1] C* const c = nullptr; // [2] };

int main(int argc, char* /unused/) { C c = new C; A a(c, "123"); return 1; };

clang-tidy.exe -checks=-,CallAndMessage* test.cc ...

test.cc:14:5: warning: Called C++ object pointer is null [clang-analyzer-core.CallAndMessage] c->foo(42); ^ test.cc:29:5: note: Calling constructor for 'A' A a(c, "123"); ^ test.cc:12:12: note: Field 'c' is non-null assert(c); ^ test.cc:12:12: note: Field 'c' is non-null test.cc:12:5: note: Left side of '||' is true assert(c); ^ C:\Program Files (x86)\Windows Kits\10\Include\10.0.18362.0\ucrt\assert.h:33:30: note: expanded from macro 'assert' (!!(expression)) || \ ^ test.cc:13:5: note: Calling 'A::bar' bar("12345"); ^ test.cc:19:5: note: Null pointer value stored to 'a.c' s = s; ^ test.cc:13:5: note: Returning from 'A::bar' bar("12345"); ^ test.cc:14:5: note: Called C++ object pointer is null c->foo(42); ^

"c_" is considered nullptr, while it's not changed during a call to "bar".

Moreover, 1) if we swap [1] and [2] lines, then CallAndMessage is not raised; 2) if we remove 'const' in [2] then CallAndMessage is not raised.

[irishrover]

We have a number of such false-positives in our Chromium-based proprietary code.

[haoNoQ]

Sorry, i've got a lot of these on my plate. Is this a common false positive that you're seeing many instances of, or is it a one-off thing? If it's common i could try to prioritize that.

Does this hypothesis explain why:

It doesn't immediately and this aspect of the problem definitely requires more investigation but i have a general experience that this code of ours is messy and such weird effects aren't unheard of.

[irishrover]

Gentle ping) Any new ideas?

[irishrover]

Yeah, over-eager invalidation, thanks!

Gabor, maybe we should apply https://reviews.llvm.org/D57230 to standard functions/methods. I.e., make a blanket statement that standard library functions and methods never invalidate superregions.

Does this hypothesis explain why:

1) if we swap [1] and [2] lines, then CallAndMessage is not raised; 2) if we remove 'const' in [2] then CallAndMessage is not raised

?

[irishrover]

Yeah, over-eager invalidation, thanks!

Gabor, maybe we should apply https://reviews.llvm.org/D57230 to standard functions/methods. I.e., make a blanket statement that standard library functions and methods never invalidate superregions.

Does this hypothesis explain why:

1) if we swap [1] and [2] lines, then CallAndMessage is not raised; 2) if we remove 'const' in [2] then CallAndMessage is not raised

?

[irishrover]

Thanks!

@​Gábor: I faced with it when analyzed Chromium-based browser code with Codechecker :-)

[Xazax-hun]

Gabor, maybe we should apply https://reviews.llvm.org/D57230 to standard functions/methods. I.e., make a blanket statement that standard library functions and methods never invalidate superregions.

Sounds good to me! :) I hope eventually we can land the whole thing though.

[haoNoQ]

Yeah, over-eager invalidation, thanks!

Gabor, maybe we should apply https://reviews.llvm.org/D57230 to standard functions/methods. I.e., make a blanket statement that standard library functions and methods never invalidate superregions.

[irishrover]

class C { public: int foo(int); };

class D { public: void foo(int); };

class A { public: A(const D& s) : ccc(new C), ddd(D()) { ccc_->foo(42); }

private: C* const ccc = nullptr; D ddd;
};

test.cc:13:37: note: Null pointer value stored to field 'ccc'0 (Loc) A(const D& s) : ccc(new C), ddd(D()) { ccc->foo(42); } ^

[irishrover]

In fact "main" is not required to reproduce.

[irishrover]

assigned to @haoNoQ