[feature request] null pointer check after dereference

[54aefcd4-c07d-4252-8441-723563c8826f]

Bugzilla Link	30232
Version	unspecified
OS	All
CC	@JonasToth,@EugeneZelenko,@AnnaZaks,@k15tfu,@RKSimon

Extended Description

I think that a function-scope check that warns on null pointer tests after dereferencing would be very useful.

For example, in the following snippet:

int* foo(); void baz();

void bar() { int* a = foo(); a[0] = 2; if(a) baz(); }

a is used before the test that check if its a null pointer:

• if foo() can return a null pointer, the check should come before the assignment. - if it cannot, then the check is unnecessary.

In both cases, there is always something suspicious going on and either clang or clang-tidy should warn about it (clang would be preferable).

Note, in real examples, the dereference of a before the nullpointer check can happened behind a macro, or inside an intermediate function.

A good place to start would be to restrict such checks to a function scope first, which should catch issues inside macros, but omit following every function call that can access the pointer in the analysis.

If desired, doing that can be done a posteriory, but even if it is found that a function validates the pointer, then a second validation after it becomes unnecessary, so the check should still warn about it.

[RKSimon]

https://www.viva64.com/en/b/0629/

The recent PVS Studio check of the llvm/clang source found a LOT of these "check a pointer after dereferencing" cases - getting even basic detection for this into the clang tools would be very useful.

int testnull(int p); int testnull(int p) { int value = *p; return p ? value : 0; }

https://godbolt.org/z/B7-cZv

[JonasToth]

Hi,

i think such a check can be could implement cppcoreguidelines gsl::not_null<T*> semantics. Every pointer, that is not checked before dereferencing should be declared as not_null.

Redundant checks are then easily spottable.

What do you think about that +Daniel Posch, would you implement this?

[llvmbot]

It is still open, so feel free to implement it :)

[llvmbot]

Hi,

Im looking to find a project for GSoC and found this on their website as a good project to do for the summer. Is it still available and if so would you like someone to have this project as their job this summer?

[llvmbot]

Arturo Campos declared to work on it, so I temporarily assign myself to it (until Arturo will have access to bugzilla)

[0f73b9cf-134f-41af-a8b1-14d9f305ee95]

We cannot add a path-sensitive analyzer check for this because the check requires checking a property over all paths, but the analyzer does not guarantee that it will cover all paths.

This can be implemented as a flow-sensitive analysis on top of clang's CFG. An example of another data-flow analysis is LiveVariables.

This could be a compiler warning. There is no reason to restrict the check to clang-tidy users only.

[llvmbot]

If I remember corectly

int a = foo(); a = 42; if (!a) { something }

will be found with DeadCode checker

but I guess it won't gonna find your case.

[EugeneZelenko]

Doesn't Static Analyzer core.NullDereference do this?