/// \sa shouldPruneNullReturnPaths
llvm::Optional<bool> PruneNullReturnPaths;
+ /// \sa shouldAvoidSuppressingNullArgumentPaths
+ llvm::Optional<bool> AvoidSuppressingNullArgumentPaths;
+
/// \sa getGraphTrimInterval
llvm::Optional<unsigned> GraphTrimInterval;
/// which accepts the values "true" and "false".
bool shouldPruneNullReturnPaths();
+ /// Returns whether a bug report should \em not be suppressed if its path
+ /// includes a call with a null argument, even if that call has a null return.
+ ///
+ /// This option has no effect when #shouldPruneNullReturnPaths() is false.
+ ///
+ /// This is a counter-heuristic to avoid false negatives.
+ ///
+ /// This is controlled by the 'avoid-suppressing-null-argument-paths' config
+ /// option, which accepts the values "true" and "false".
+ bool shouldAvoidSuppressingNullArgumentPaths();
+
// Returns the size of the functions (in basic blocks), which should be
// considered to be small enough to always inline.
//
/// \param IsArg Whether the statement is an argument to an inlined function.
/// If this is the case, \p N \em must be the CallEnter node for
/// the function.
-void trackNullOrUndefValue(const ExplodedNode *N, const Stmt *S, BugReport &R,
+///
+/// \return Whether or not the function was able to add visitors for this
+/// statement. Note that returning \c true does not actually imply
+/// that any visitors were added.
+bool trackNullOrUndefValue(const ExplodedNode *N, const Stmt *S, BugReport &R,
bool IsArg = false);
const Stmt *GetDerefExpr(const ExplodedNode *N);
/* Default = */ true);
}
+bool AnalyzerOptions::shouldAvoidSuppressingNullArgumentPaths() {
+ return getBooleanOption(AvoidSuppressingNullArgumentPaths,
+ "avoid-suppressing-null-argument-paths",
+ /* Default = */ false);
+}
+
int AnalyzerOptions::getOptionAsInteger(StringRef Name, int DefaultVal) {
llvm::SmallString<10> StrBuf;
llvm::raw_svector_ostream OS(StrBuf);
/// interesting value comes from an inlined function call.
class ReturnVisitor : public BugReporterVisitorImpl<ReturnVisitor> {
const StackFrameContext *StackFrame;
- bool Satisfied;
+ enum {
+ Initial,
+ MaybeSuppress,
+ Satisfied
+ } Mode;
+
public:
ReturnVisitor(const StackFrameContext *Frame)
- : StackFrame(Frame), Satisfied(false) {}
+ : StackFrame(Frame), Mode(Initial) {}
static void *getTag() {
static int Tag = 0;
}
}
- PathDiagnosticPiece *VisitNode(const ExplodedNode *N,
- const ExplodedNode *PrevN,
- BugReporterContext &BRC,
- BugReport &BR) {
- if (Satisfied)
- return 0;
+ /// Returns true if any counter-suppression heuristics are enabled for
+ /// ReturnVisitor.
+ static bool hasCounterSuppression(AnalyzerOptions &Options) {
+ return Options.shouldAvoidSuppressingNullArgumentPaths();
+ }
+ PathDiagnosticPiece *visitNodeInitial(const ExplodedNode *N,
+ const ExplodedNode *PrevN,
+ BugReporterContext &BRC,
+ BugReport &BR) {
// Only print a message at the interesting return statement.
if (N->getLocationContext() != StackFrame)
return 0;
return 0;
// Don't print any more notes after this one.
- Satisfied = true;
+ Mode = Satisfied;
const Expr *RetE = Ret->getRetValue();
assert(RetE && "Tracking a return value for a void function");
// report invalid. We still want to emit a path note, however, in case
// the report is resurrected as valid later on.
ExprEngine &Eng = BRC.getBugReporter().getEngine();
- if (Eng.getAnalysisManager().options.shouldPruneNullReturnPaths())
- BR.markInvalid(ReturnVisitor::getTag(), StackFrame);
+ AnalyzerOptions &Options = Eng.getAnalysisManager().options;
+ if (Options.shouldPruneNullReturnPaths()) {
+ if (hasCounterSuppression(Options))
+ Mode = MaybeSuppress;
+ else
+ BR.markInvalid(ReturnVisitor::getTag(), StackFrame);
+ }
if (RetE->getType()->isObjCObjectPointerType())
Out << "Returning nil";
PathDiagnosticLocation L(Ret, BRC.getSourceManager(), StackFrame);
return new PathDiagnosticEventPiece(L, Out.str());
}
+
+ PathDiagnosticPiece *visitNodeMaybeSuppress(const ExplodedNode *N,
+ const ExplodedNode *PrevN,
+ BugReporterContext &BRC,
+ BugReport &BR) {
+ // Are we at the entry node for this call?
+ const CallEnter *CE = N->getLocationAs<CallEnter>();
+ if (!CE)
+ return 0;
+
+ if (CE->getCalleeContext() != StackFrame)
+ return 0;
+
+ Mode = Satisfied;
+
+ ExprEngine &Eng = BRC.getBugReporter().getEngine();
+ AnalyzerOptions &Options = Eng.getAnalysisManager().options;
+ if (Options.shouldAvoidSuppressingNullArgumentPaths()) {
+ // Don't automatically suppress a report if one of the arguments is
+ // known to be a null pointer. Instead, start tracking /that/ null
+ // value back to its origin.
+ ProgramStateManager &StateMgr = BRC.getStateManager();
+ CallEventManager &CallMgr = StateMgr.getCallEventManager();
+
+ ProgramStateRef State = N->getState();
+ CallEventRef<> Call = CallMgr.getCaller(StackFrame, State);
+ for (unsigned I = 0, E = Call->getNumArgs(); I != E; ++I) {
+ SVal ArgV = Call->getArgSVal(I);
+ if (!isa<Loc>(ArgV))
+ continue;
+
+ const Expr *ArgE = Call->getArgExpr(I);
+ if (!ArgE)
+ continue;
+
+ // Is it possible for this argument to be non-null?
+ if (State->assume(cast<Loc>(ArgV), true))
+ continue;
+
+ if (bugreporter::trackNullOrUndefValue(N, ArgE, BR, /*IsArg=*/true))
+ return 0;
+
+ // If we /can't/ track the null pointer, we should err on the side of
+ // false negatives, and continue towards marking this report invalid.
+ // (We will still look at the other arguments, though.)
+ }
+ }
+
+ // There is no reason not to suppress this report; go ahead and do it.
+ BR.markInvalid(ReturnVisitor::getTag(), StackFrame);
+ return 0;
+ }
+
+ PathDiagnosticPiece *VisitNode(const ExplodedNode *N,
+ const ExplodedNode *PrevN,
+ BugReporterContext &BRC,
+ BugReport &BR) {
+ switch (Mode) {
+ case Initial:
+ return visitNodeInitial(N, PrevN, BRC, BR);
+ case MaybeSuppress:
+ return visitNodeMaybeSuppress(N, PrevN, BRC, BR);
+ case Satisfied:
+ return 0;
+ }
+
+ llvm_unreachable("Invalid visit mode!");
+ }
};
} // end anonymous namespace
return NULL;
}
-void bugreporter::trackNullOrUndefValue(const ExplodedNode *N, const Stmt *S,
+bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N, const Stmt *S,
BugReport &report, bool IsArg) {
if (!S || !N)
- return;
+ return false;
if (const OpaqueValueExpr *OVE = dyn_cast<OpaqueValueExpr>(S))
S = OVE->getSourceExpr();
} while (N);
if (!N)
- return;
+ return false;
}
ProgramStateRef state = N->getState();
}
report.addVisitor(new FindLastStoreBRVisitor(V, R));
- return;
+ return true;
}
}
}
S = E->IgnoreParenCasts();
ReturnVisitor::addVisitorIfNecessary(N, S, report);
}
+
+ return true;
}
BugReporterVisitor *
const CFGBlock *BSrc = BE->getSrc();
S = BSrc->getTerminatorCondition();
}
- else if (const PostStmt *PS = dyn_cast<PostStmt>(&P)) {
- S = PS->getStmt();
+ else if (const StmtPoint *SP = dyn_cast<StmtPoint>(&P)) {
+ S = SP->getStmt();
}
else if (const PostImplicitCall *PIE = dyn_cast<PostImplicitCall>(&P)) {
return PathDiagnosticLocation(PIE->getLocation(), SMng);
CEE->getLocationContext(),
SMng);
}
+ else {
+ llvm_unreachable("Unexpected ProgramPoint");
+ }
return PathDiagnosticLocation(S, SMng, P.getLocationContext());
}
// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-config suppress-null-return-paths=false -verify %s
-// RUN: %clang_cc1 -analyze -analyzer-checker=core -verify -DSUPPRESSED %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -verify -DSUPPRESSED=1 %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-config avoid-suppressing-null-argument-paths=true -DSUPPRESSED=1 -DNULL_ARGS=1 -verify %s
int opaquePropertyCheck(void *object);
int coin();
+int *getNull() {
+ return 0;
+}
+
int *dynCastToInt(void *ptr) {
if (opaquePropertyCheck(ptr))
return (int *)ptr;
}
+// --------------------------
+// "Suppression suppression"
+// --------------------------
+
+void testDynCastOrNullOfNull() {
+ // Don't suppress when one of the arguments is NULL.
+ int *casted = dynCastOrNull(0);
+ *casted = 1;
+#if !SUPPRESSED || NULL_ARGS
+ // expected-warning@-2 {{Dereference of null pointer}}
+#endif
+}
+
+void testDynCastOfNull() {
+ // Don't suppress when one of the arguments is NULL.
+ int *casted = dynCastToInt(0);
+ *casted = 1;
+#if !SUPPRESSED || NULL_ARGS
+ // expected-warning@-2 {{Dereference of null pointer}}
+#endif
+}
+
+int *lookUpInt(int unused) {
+ if (coin())
+ return 0;
+ static int x;
+ return &x;
+}
+
+void testZeroIsNotNull() {
+ // /Do/ suppress when the argument is 0 (an integer).
+ int *casted = lookUpInt(0);
+ *casted = 1;
+#ifndef SUPPRESSED
+ // expected-warning@-2 {{Dereference of null pointer}}
+#endif
+}
+
+void testTrackNull() {
+ // /Do/ suppress if the null argument came from another call returning null.
+ int *casted = dynCastOrNull(getNull());
+ *casted = 1;
+#ifndef SUPPRESSED
+ // expected-warning@-2 {{Dereference of null pointer}}
+#endif
+}
+
+void testTrackNullVariable() {
+ // /Do/ suppress if the null argument came from another call returning null.
+ int *ptr;
+ ptr = getNull();
+ int *casted = dynCastOrNull(ptr);
+ *casted = 1;
+#ifndef SUPPRESSED
+ // expected-warning@-2 {{Dereference of null pointer}}
+#endif
+}
+
+
// ---------------------------------------
// FALSE NEGATIVES (over-suppression)
// ---------------------------------------
-void testDynCastOrNullOfNull() {
+void testNoArguments() {
+ // In this case the function has no branches, and MUST return null.
+ int *casted = getNull();
+ *casted = 1;
+#ifndef SUPPRESSED
+ // expected-warning@-2 {{Dereference of null pointer}}
+#endif
+}
+
+int *getNullIfNonNull(void *input) {
+ if (input)
+ return 0;
+ static int x;
+ return &x;
+}
+
+void testKnownPath(void *input) {
+ if (!input)
+ return;
+
// In this case we have a known value for the argument, and thus the path
// through the function doesn't ever split.
- int *casted = dynCastOrNull(0);
+ int *casted = getNullIfNonNull(input);
*casted = 1;
#ifndef SUPPRESSED
// expected-warning@-2 {{Dereference of null pointer}}
#endif
}
-void testDynCastOfNull() {
+int *alwaysReturnNull(void *input) {
+ if (opaquePropertyCheck(input))
+ return 0;
+ return 0;
+}
+
+void testAlwaysReturnNull(void *input) {
// In this case all paths out of the function return 0, but they are all
// dominated by a branch whose condition we don't know!
- int *casted = dynCastToInt(0);
+ int *casted = alwaysReturnNull(input);
*casted = 1;
#ifndef SUPPRESSED
// expected-warning@-2 {{Dereference of null pointer}}
projects / platform / upstream / llvm.git / commitdiff