This revision was automatically updated to reflect the committed changes.
Closed by commit rC337213: [analyzer] Bugfix for an overly eager suppression
for null pointer return from… (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.
Changed prior to commit:
https://reviews.llvm.org/D48856?vs=153815&id=155752#toc
Repository:
rC Clang
https://reviews.llvm.org/D48856
Files:
lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
test/Analysis/diagnostics/macro-null-return-suppression.cpp
Index: test/Analysis/diagnostics/macro-null-return-suppression.cpp
===================================================================
--- test/Analysis/diagnostics/macro-null-return-suppression.cpp
+++ test/Analysis/diagnostics/macro-null-return-suppression.cpp
@@ -43,3 +43,26 @@
DEREF_IN_MACRO(0) // expected-warning{{Dereference of null pointer}}
// expected-note@-1{{'p' initialized to a null}}
// expected-note@-2{{Dereference of null pointer}}
+
+// Warning should not be suppressed if the null returned by the macro
+// is not related to the warning.
+#define RETURN_NULL() (0)
+extern int* returnFreshPointer();
+int noSuppressMacroUnrelated() {
+ int *x = RETURN_NULL();
+ x = returnFreshPointer(); // expected-note{{Value assigned to 'x'}}
+ if (x) {} // expected-note{{Taking false branch}}
+ // expected-note@-1{{Assuming 'x' is null}}
+ return *x; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference}}
+}
+
+// Value haven't changed by the assignment, but the null pointer
+// did not come from the macro.
+int noSuppressMacroUnrelatedOtherReason() {
+ int *x = RETURN_NULL();
+ x = returnFreshPointer();
+ x = 0; // expected-note{{Null pointer value stored to 'x'}}
+ return *x; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference}}
+}
Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===================================================================
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -228,6 +228,38 @@
return EInfo.isFunctionMacroExpansion();
}
+/// \return Whether \c RegionOfInterest was modified at \p N,
+/// where \p ReturnState is a state associated with the return
+/// from the current frame.
+static bool wasRegionOfInterestModifiedAt(
+ const SubRegion *RegionOfInterest,
+ const ExplodedNode *N,
+ SVal ValueAfter) {
+ ProgramStateRef State = N->getState();
+ ProgramStateManager &Mgr = N->getState()->getStateManager();
+
+ if (!N->getLocationAs<PostStore>()
+ && !N->getLocationAs<PostInitializer>()
+ && !N->getLocationAs<PostStmt>())
+ return false;
+
+ // Writing into region of interest.
+ if (auto PS = N->getLocationAs<PostStmt>())
+ if (auto *BO = PS->getStmtAs<BinaryOperator>())
+ if (BO->isAssignmentOp() && RegionOfInterest->isSubRegionOf(
+ N->getSVal(BO->getLHS()).getAsRegion()))
+ return true;
+
+ // SVal after the state is possibly different.
+ SVal ValueAtN = N->getState()->getSVal(RegionOfInterest);
+ if (!Mgr.getSValBuilder().areEqual(State, ValueAtN, ValueAfter).isConstrainedTrue() &&
+ (!ValueAtN.isUndef() || !ValueAfter.isUndef()))
+ return true;
+
+ return false;
+}
+
+
namespace {
/// Put a diagnostic on return statement of all inlined functions
@@ -346,7 +378,7 @@
FramesModifyingCalculated.insert(
N->getLocationContext()->getStackFrame());
- if (wasRegionOfInterestModifiedAt(N, LastReturnState, ValueAtReturn)) {
+ if (wasRegionOfInterestModifiedAt(RegionOfInterest, N, ValueAtReturn)) {
const StackFrameContext *SCtx = N->getStackFrame();
while (!SCtx->inTopFrame()) {
auto p = FramesModifyingRegion.insert(SCtx);
@@ -365,33 +397,6 @@
} while (N);
}
- /// \return Whether \c RegionOfInterest was modified at \p N,
- /// where \p ReturnState is a state associated with the return
- /// from the current frame.
- bool wasRegionOfInterestModifiedAt(const ExplodedNode *N,
- ProgramStateRef ReturnState,
- SVal ValueAtReturn) {
- if (!N->getLocationAs<PostStore>()
- && !N->getLocationAs<PostInitializer>()
- && !N->getLocationAs<PostStmt>())
- return false;
-
- // Writing into region of interest.
- if (auto PS = N->getLocationAs<PostStmt>())
- if (auto *BO = PS->getStmtAs<BinaryOperator>())
- if (BO->isAssignmentOp() && RegionOfInterest->isSubRegionOf(
- N->getSVal(BO->getLHS()).getAsRegion()))
- return true;
-
- // SVal after the state is possibly different.
- SVal ValueAtN = N->getState()->getSVal(RegionOfInterest);
- if (!ReturnState->areEqual(ValueAtN, ValueAtReturn).isConstrainedTrue() &&
- (!ValueAtN.isUndef() || !ValueAtReturn.isUndef()))
- return true;
-
- return false;
- }
-
/// Get parameters associated with runtime definition in order
/// to get the correct parameter name.
ArrayRef<ParmVarDecl *> getCallParameters(CallEventRef<> Call) {
@@ -524,25 +529,28 @@
}
};
+/// Suppress null-pointer-dereference bugs where dereferenced null was returned
+/// the macro.
class MacroNullReturnSuppressionVisitor final : public BugReporterVisitor {
const SubRegion *RegionOfInterest;
+ const SVal ValueAtDereference;
-public:
- MacroNullReturnSuppressionVisitor(const SubRegion *R) : RegionOfInterest(R) {}
+ // Do not invalidate the reports where the value was modified
+ // after it got assigned to from the macro.
+ bool WasModified = false;
- static void *getTag() {
- static int Tag = 0;
- return static_cast<void *>(&Tag);
- }
-
- void Profile(llvm::FoldingSetNodeID &ID) const override {
- ID.AddPointer(getTag());
- }
+public:
+ MacroNullReturnSuppressionVisitor(const SubRegion *R,
+ const SVal V) : RegionOfInterest(R),
+ ValueAtDereference(V) {}
std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
const ExplodedNode *PrevN,
BugReporterContext &BRC,
BugReport &BR) override {
+ if (WasModified)
+ return nullptr;
+
auto BugPoint = BR.getErrorNode()->getLocation().getAs<StmtPoint>();
if (!BugPoint)
return nullptr;
@@ -556,6 +564,10 @@
BR.markInvalid(getTag(), MacroName.c_str());
}
}
+
+ if (wasRegionOfInterestModifiedAt(RegionOfInterest, N, ValueAtDereference))
+ WasModified = true;
+
return nullptr;
}
@@ -568,7 +580,16 @@
if (EnableNullFPSuppression && Options.shouldSuppressNullReturnPaths()
&& V.getAs<Loc>())
BR.addVisitor(llvm::make_unique<MacroNullReturnSuppressionVisitor>(
- R->getAs<SubRegion>()));
+ R->getAs<SubRegion>(), V));
+ }
+
+ void* getTag() const {
+ static int Tag = 0;
+ return static_cast<void *>(&Tag);
+ }
+
+ void Profile(llvm::FoldingSetNodeID &ID) const override {
+ ID.AddPointer(getTag());
}
private:
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
