Skip to content

Commit

Permalink
[analyzer] Fix false negative when using a nullable parameter directl…
Browse files Browse the repository at this point in the history
…y without binding to a variable

If a parameter has a nullability annotation, the nullability information
of the parameter should be set as a NullabilityState trait at the
beginning of the function.

Patch By tripleCC!

Differential Revision: https://reviews.llvm.org/D153017
  • Loading branch information
tripleCC authored and steakhal committed Jul 3, 2023
1 parent 180f9ef commit 77a599a
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 3 deletions.
40 changes: 37 additions & 3 deletions clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,13 @@

#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"

#include "clang/Analysis/AnyCall.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"

#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/Path.h"
Expand Down Expand Up @@ -81,7 +82,8 @@ class NullabilityChecker
: public Checker<check::Bind, check::PreCall, check::PreStmt<ReturnStmt>,
check::PostCall, check::PostStmt<ExplicitCastExpr>,
check::PostObjCMessage, check::DeadSymbols, eval::Assume,
check::Location, check::Event<ImplicitNullDerefEvent>> {
check::Location, check::Event<ImplicitNullDerefEvent>,
check::BeginFunction> {

public:
// If true, the checker will not diagnose nullabilility issues for calls
Expand All @@ -102,6 +104,7 @@ class NullabilityChecker
void checkEvent(ImplicitNullDerefEvent Event) const;
void checkLocation(SVal Location, bool IsLoad, const Stmt *S,
CheckerContext &C) const;
void checkBeginFunction(CheckerContext &Ctx) const;
ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
bool Assumption) const;

Expand Down Expand Up @@ -563,6 +566,37 @@ void NullabilityChecker::checkEvent(ImplicitNullDerefEvent Event) const {
}
}

void NullabilityChecker::checkBeginFunction(CheckerContext &C) const {
if (!C.inTopFrame())
return;

const LocationContext *LCtx = C.getLocationContext();
auto AbstractCall = AnyCall::forDecl(LCtx->getDecl());
if (!AbstractCall || AbstractCall->parameters().empty())
return;

ProgramStateRef State = C.getState();
for (const ParmVarDecl *Param : AbstractCall->parameters()) {
if (!isValidPointerType(Param->getType()))
continue;

Nullability RequiredNullability =
getNullabilityAnnotation(Param->getType());
if (RequiredNullability != Nullability::Nullable)
continue;

const VarRegion *ParamRegion = State->getRegion(Param, LCtx);
const MemRegion *ParamPointeeRegion =
State->getSVal(ParamRegion).getAsRegion();
if (!ParamPointeeRegion)
continue;

State = State->set<NullabilityMap>(ParamPointeeRegion,
NullabilityState(RequiredNullability));
}
C.addTransition(State);
}

// Whenever we see a load from a typed memory region that's been annotated as
// 'nonnull', we want to trust the user on that and assume that it is is indeed
// non-null.
Expand Down
11 changes: 11 additions & 0 deletions clang/test/Analysis/nullability.mm
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,17 @@ void testArgumentTracking(Dummy *_Nonnull nonnull, Dummy *_Nullable nullable) {
}
}

void testArgumentTrackingDirectly(Dummy *_Nonnull nonnull, Dummy *_Nullable nullable) {
switch(getRandom()) {
case 1: testMultiParamChecking(nonnull, nullable, nonnull); break;
case 2: testMultiParamChecking(nonnull, nonnull, nonnull); break;
case 3: testMultiParamChecking(nonnull, nullable, nullable); break; // expected-warning {{Nullable pointer is passed to a callee that requires a non-null 3rd parameter}}
case 4: testMultiParamChecking(nullable, nullable, nonnull); // expected-warning {{Nullable pointer is passed to a callee that requires a non-null 1st parameter}}
case 5: testMultiParamChecking(nullable, nullable, nullable); // expected-warning {{Nullable pointer is passed to a callee that requires a non-null 1st parameter}}
case 6: testMultiParamChecking((Dummy *_Nonnull)0, nullable, nonnull); break;
}
}

Dummy *_Nonnull testNullableReturn(Dummy *_Nullable a) {
Dummy *p = a;
return p; // expected-warning {{Nullable pointer is returned from a function that is expected to return a non-null value}}
Expand Down

0 comments on commit 77a599a

Please sign in to comment.