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() + && !N->getLocationAs() + && !N->getLocationAs()) + return false; + + // Writing into region of interest. + if (auto PS = N->getLocationAs()) + if (auto *BO = PS->getStmtAs()) + 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() - && !N->getLocationAs() - && !N->getLocationAs()) - return false; - - // Writing into region of interest. - if (auto PS = N->getLocationAs()) - if (auto *BO = PS->getStmtAs()) - 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 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(&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 VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN, BugReporterContext &BRC, BugReport &BR) override { + if (WasModified) + return nullptr; + auto BugPoint = BR.getErrorNode()->getLocation().getAs(); 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()) BR.addVisitor(llvm::make_unique( - R->getAs())); + R->getAs(), V)); + } + + void* getTag() const { + static int Tag = 0; + return static_cast(&Tag); + } + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.AddPointer(getTag()); } private: 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}} +}