Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -159,8 +159,321 @@ return std::move(P); } +/// \return name of the macro inside the location \p Loc. +static StringRef getMacroName(SourceLocation Loc, + BugReporterContext &BRC) { + return Lexer::getImmediateMacroName( + Loc, + BRC.getSourceManager(), + BRC.getASTContext().getLangOpts()); +} + +/// \return Whether given spelling location corresponds to an expansion +/// of a function-like macro. +static bool isFunctionMacroExpansion(SourceLocation Loc, + const SourceManager &SM) { + if (!Loc.isMacroID()) + return false; + std::pair TLInfo = SM.getDecomposedLoc(Loc); + SrcMgr::SLocEntry SE = SM.getSLocEntry(TLInfo.first); + const SrcMgr::ExpansionInfo &EInfo = SE.getExpansion(); + return EInfo.isFunctionMacroExpansion(); +} namespace { + +/// Put a diagnostic on return statement of all inlined functions +/// for which the region of interest \p RegionOfInterest was passed into, +/// but not written inside, and it has caused an undefined read or a null +/// pointer dereference outside. +class NoStoreFuncVisitor final + : public BugReporterVisitorImpl { + + const SubRegion *RegionOfInterest; + static constexpr const char *DiagnosticsMsg = + "Returning without writing to '"; + bool Initialized = false; + + /// Frames writing into \c RegionOfInterest. + /// This visitor generates a note only if a function does not write into + /// a region of interest. This information is not immediately available + /// by looking at the node associated with the exit from the function + /// (usually the return statement). To avoid recomputing the same information + /// many times (going up the path for each node and checking whether the + /// region was written into) we instead pre-compute and store all + /// stack frames along the path which write into the region of interest + /// on the first \c VisitNode invocation. + llvm::SmallPtrSet FramesModifyingRegion; + +public: + NoStoreFuncVisitor(const SubRegion *R) : RegionOfInterest(R) {} + + void Profile(llvm::FoldingSetNodeID &ID) const override { + static int Tag = 0; + ID.AddPointer(&Tag); + } + + std::shared_ptr VisitNode(const ExplodedNode *N, + const ExplodedNode *PrevN, + BugReporterContext &BRC, + BugReport &BR) override { + if (!Initialized) { + findModifyingFrames(N); + Initialized = true; + } + + const LocationContext *Ctx = N->getLocationContext(); + const StackFrameContext *SCtx = Ctx->getCurrentStackFrame(); + ProgramStateRef State = N->getState(); + auto CallExitLoc = N->getLocationAs(); + + // No diagnostic if region was modified inside the frame. + if (!CallExitLoc || FramesModifyingRegion.count(SCtx)) + return nullptr; + + CallEventRef<> Call = + BRC.getStateManager().getCallEventManager().getCaller(SCtx, State); + + const PrintingPolicy &PP = BRC.getASTContext().getPrintingPolicy(); + const SourceManager &SM = BRC.getSourceManager(); + if (auto *CCall = dyn_cast(Call)) { + const MemRegion *ThisRegion = CCall->getCXXThisVal().getAsRegion(); + if (RegionOfInterest->isSubRegionOf(ThisRegion) && + !CCall->getDecl()->isImplicit()) + return notModifiedInConstructorDiagnostics(Ctx, SM, PP, *CallExitLoc, + CCall, ThisRegion); + } + + ArrayRef parameters = getCallParameters(Call); + for (unsigned I = 0, E = Call->getNumArgs(); I != E; ++I) { + const ParmVarDecl *PVD = parameters[I]; + const MemRegion *R = Call->getArgSVal(I).getAsRegion(); + if (RegionOfInterest->isSubRegionOf(R) && + !isPointerToConst(PVD->getType())) + return notModifiedDiagnostics(Ctx, SM, PP, *CallExitLoc, Call, PVD, R); + } + + return nullptr; + } + +private: + /// Write to \c FramesModifyingRegion all stack frames along + /// the path which modify \c RegionOfInterest. + void findModifyingFrames(const ExplodedNode *N) { + ProgramStateRef LastReturnState; + do { + ProgramStateRef State = N->getState(); + auto CallExitLoc = N->getLocationAs(); + if (CallExitLoc) { + LastReturnState = State; + } + if (LastReturnState && + wasRegionOfInterestModifiedAt(N, LastReturnState)) { + const StackFrameContext *SCtx = + N->getLocationContext()->getCurrentStackFrame(); + while (!SCtx->inTopFrame()) { + auto p = FramesModifyingRegion.insert(SCtx); + if (!p.second) + break; // Frame and all its parents already inserted. + SCtx = SCtx->getParent()->getCurrentStackFrame(); + } + } + + N = N->getFirstPred(); + } 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 = ReturnState->getSVal(RegionOfInterest); + + // 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) { + if (isa(Call->getDecl())) + return dyn_cast(Call->getRuntimeDefinition().getDecl()) + ->parameters(); + return Call->parameters(); + } + + /// \return whether \p Ty points to a const type, or is a const reference. + bool isPointerToConst(QualType Ty) { + return !Ty->getPointeeType().isNull() && + Ty->getPointeeType().getCanonicalType().isConstQualified(); + } + + std::shared_ptr notModifiedInConstructorDiagnostics( + const LocationContext *Ctx, + const SourceManager &SM, + const PrintingPolicy &PP, + CallExitBegin &CallExitLoc, + const CXXConstructorCall *Call, + const MemRegion *ArgRegion) { + + SmallString<256> sbuf; + llvm::raw_svector_ostream os(sbuf); + os << DiagnosticsMsg; + os << "this"; + bool out = prettyPrintRegionName("->", ArgRegion, os, PP); + if (!out) // Return nothing if we have failed to pretty-print. + return nullptr; + os << "'"; + PathDiagnosticLocation L = + getPathDiagnosticLocation(nullptr, SM, Ctx, Call); + return std::make_shared(L, os.str()); + } + + std::shared_ptr + notModifiedDiagnostics(const LocationContext *Ctx, + const SourceManager &SM, + const PrintingPolicy &PP, + CallExitBegin &CallExitLoc, CallEventRef<> Call, + const ParmVarDecl *PVD, + const MemRegion *ArgRegion) { + + PathDiagnosticLocation L = getPathDiagnosticLocation( + CallExitLoc.getReturnStmt(), SM, Ctx, Call); + SmallString<256> sbuf; + llvm::raw_svector_ostream os(sbuf); + os << DiagnosticsMsg; + PVD->printQualifiedName(os); + const char *Sep = PVD->getType()->isReferenceType() ? "." : "->"; + + // If printing fails, we just use previously printed parameter name. + prettyPrintRegionName(Sep, ArgRegion, os, PP); + os << "'"; + return std::make_shared(L, os.str()); + } + + /// \return a path diagnostic location for the optionally + /// present return statement \p RS. + PathDiagnosticLocation getPathDiagnosticLocation(const ReturnStmt *RS, + const SourceManager &SM, + const LocationContext *Ctx, + CallEventRef<> Call) { + if (RS) + return PathDiagnosticLocation::createBegin(RS, SM, Ctx); + return PathDiagnosticLocation( + Call->getRuntimeDefinition().getDecl()->getSourceRange().getEnd(), SM); + } + + /// Pretty-print region \p ArgRegion starting from parent to \p os. + /// \return whether printing has succeeded + bool prettyPrintRegionName(const char *Sep, const MemRegion *ArgRegion, + llvm::raw_svector_ostream &os, + const PrintingPolicy &PP) { + SmallVector Regions; + const MemRegion *R = RegionOfInterest; + while (R != ArgRegion) { + if (!(isa(R) || isa(R))) + return false; // Pattern-matching failed. + Regions.push_back(R); + R = dyn_cast(R)->getSuperRegion(); + } + + for (auto I = Regions.rbegin(), E = Regions.rend(); I != E; ++I) { + if (auto *FR = dyn_cast(*I)) { + os << Sep; + FR->getDecl()->getDeclName().print(os, PP); + Sep = "."; + } else if (auto *CXXR = dyn_cast(*I)) { + continue; // Just keep going up to the base region. + } else { + llvm_unreachable("Previous check has missed an unexpected region"); + } + } + return true; + } +}; + +} // end anonymous namespace + +namespace { + +class MacroNullReturnSuppressionVisitor final + : public BugReporterVisitorImpl { + + const SubRegion *RegionOfInterest; + +public: + MacroNullReturnSuppressionVisitor(const SubRegion *R) : RegionOfInterest(R) {} + + static void *getTag() { + static int Tag = 0; + return static_cast(&Tag); + } + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.AddPointer(getTag()); + } + + std::shared_ptr VisitNode(const ExplodedNode *N, + const ExplodedNode *PrevN, + BugReporterContext &BRC, + BugReport &BR) override { + auto BugPoint = BR.getErrorNode()->getLocation().getAs(); + if (!BugPoint) + return nullptr; + + const SourceManager &SMgr = BRC.getSourceManager(); + if (auto Loc = matchAssignment(N, BRC)) { + if (isFunctionMacroExpansion(*Loc, SMgr)) { + std::string MacroName = getMacroName(*Loc, BRC); + SourceLocation BugLoc = BugPoint->getStmt()->getLocStart(); + if (!BugLoc.isMacroID() || getMacroName(BugLoc, BRC) != MacroName) + BR.markInvalid(getTag(), MacroName.c_str()); + } + } + return nullptr; + } + +private: + /// \return Source location of right hand side of an assignment + /// into \c RegionOfInterest, empty optional if none found. + Optional matchAssignment(const ExplodedNode *N, + BugReporterContext &BRC) { + const Stmt *S = PathDiagnosticLocation::getStmt(N); + ProgramStateRef State = N->getState(); + auto *LCtx = N->getLocationContext(); + if (!S) + return None; + + if (auto *DS = dyn_cast(S)) { + if (const VarDecl *VD = dyn_cast(DS->getSingleDecl())) + if (const Expr *RHS = VD->getInit()) + if (RegionOfInterest->isSubRegionOf( + State->getLValue(VD, LCtx).getAsRegion())) + return RHS->getLocStart(); + } else if (auto *BO = dyn_cast(S)) { + const MemRegion *R = N->getSVal(BO->getLHS()).getAsRegion(); + const Expr *RHS = BO->getRHS(); + if (BO->isAssignmentOp() && RegionOfInterest->isSubRegionOf(R)) { + return RHS->getLocStart(); + } + } + return None; + } +}; + /// Emits an extra note at the return statement of an interesting stack frame. /// /// The returned value is marked as an interesting value, and if it's null, @@ -854,13 +1167,6 @@ return "IDCVisitor"; } -/// \return name of the macro inside the location \p Loc. -static StringRef getMacroName(SourceLocation Loc, - BugReporterContext &BRC) { - return Lexer::getImmediateMacroName( - Loc, BRC.getSourceManager(), BRC.getASTContext().getLangOpts()); -} - std::shared_ptr SuppressInlineDefensiveChecksVisitor::VisitNode(const ExplodedNode *Succ, const ExplodedNode *Pred, @@ -1123,6 +1429,10 @@ // Mark both the variable region and its contents as interesting. SVal V = LVState->getRawSVal(loc::MemRegionVal(R)); + report.addVisitor( + llvm::make_unique( + R->getAs())); + report.markInteresting(R); report.markInteresting(V); report.addVisitor(llvm::make_unique(R)); Index: test/Analysis/diagnostics/macro-null-return-suppression.cpp =================================================================== --- /dev/null +++ test/Analysis/diagnostics/macro-null-return-suppression.cpp @@ -0,0 +1,38 @@ +// RUN: %clang_analyze_cc1 -x c -analyzer-checker=core -analyzer-output=text -verify %s + +#define NULL 0 + +int test_noparammacro() { + int *x = NULL; // expected-note{{'x' initialized to a null pointer value}} + return *x; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}} + // expected-note@-1{{Dereference of null pointer (loaded from variable 'x')}} +} + +#define DYN_CAST(X) (X ? (char*)X : 0) +char test_assignment(int *param) { + char *param2; + param2 = DYN_CAST(param); + return *param2; +} + +char test_declaration(int *param) { + char *param2 = DYN_CAST(param); + return *param2; +} + +int coin(); + +int test_multi_decl(int *paramA, int *paramB) { + char *param1 = DYN_CAST(paramA), *param2 = DYN_CAST(paramB); + if (coin()) + return *param1; + return *param2; + +} + +// Warning should not be suppressed if it happens in the same macro. +#define DEREF_IN_MACRO(X) int fn() {int *p = 0; return *p; } + +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}}