Author: Adam Balogh Date: 2020-03-26T09:44:16+01:00 New Revision: 1a27d63a8891076ad9176f1a70f372003bc55c2f
URL: https://github.com/llvm/llvm-project/commit/1a27d63a8891076ad9176f1a70f372003bc55c2f DIFF: https://github.com/llvm/llvm-project/commit/1a27d63a8891076ad9176f1a70f372003bc55c2f.diff LOG: [Analyzer] Only add container note tags to the operations of the affected container If an error happens which is related to a container the Container Modeling checker adds note tags to all the container operations along the bug path. This may be disturbing if there are other containers beside the one which is affected by the bug. This patch restricts the note tags to only the affected container and adjust the debug checkers to be able to test this change. Differential Revision: https://reviews.llvm.org/D75514 Added: Modified: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp clang/test/Analysis/container-modeling.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp index b225a61db439..8126fe8260c8 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp @@ -721,6 +721,9 @@ const NoteTag *ContainerModeling::getChangeTag(CheckerContext &C, return C.getNoteTag( [Text, Name, ContReg](PathSensitiveBugReport &BR) -> std::string { + if (!BR.isInteresting(ContReg)) + return ""; + SmallString<256> Msg; llvm::raw_svector_ostream Out(Msg); Out << "Container " << (!Name.empty() ? ("'" + Name.str() + "' ") : "" ) diff --git a/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp index 8d0572723991..ce8dccb22333 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp @@ -92,7 +92,19 @@ void DebugContainerModeling::analyzerContainerDataField(const CallExpr *CE, if (Field) { State = State->BindExpr(CE, C.getLocationContext(), nonloc::SymbolVal(Field)); - C.addTransition(State); + + // Progpagate interestingness from the container's data (marked + // interesting by an `ExprInspection` debug call to the container + // itself. + const NoteTag *InterestingTag = + C.getNoteTag( + [Cont, Field](PathSensitiveBugReport &BR) -> std::string { + if (BR.isInteresting(Field)) { + BR.markInteresting(Cont); + } + return ""; + }); + C.addTransition(State, InterestingTag); return; } } diff --git a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp index 10b27831d89f..97e287e7a221 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp @@ -52,9 +52,12 @@ class ExprInspectionChecker : public Checker<eval::Call, check::DeadSymbols, typedef void (ExprInspectionChecker::*FnCheck)(const CallExpr *, CheckerContext &C) const; - ExplodedNode *reportBug(llvm::StringRef Msg, CheckerContext &C) const; + // Optional parameter `ExprVal` for expression value to be marked interesting. + ExplodedNode *reportBug(llvm::StringRef Msg, CheckerContext &C, + Optional<SVal> ExprVal = None) const; ExplodedNode *reportBug(llvm::StringRef Msg, BugReporter &BR, - ExplodedNode *N) const; + ExplodedNode *N, + Optional<SVal> ExprVal = None) const; public: bool evalCall(const CallEvent &Call, CheckerContext &C) const; @@ -144,22 +147,28 @@ static const char *getArgumentValueString(const CallExpr *CE, } ExplodedNode *ExprInspectionChecker::reportBug(llvm::StringRef Msg, - CheckerContext &C) const { + CheckerContext &C, + Optional<SVal> ExprVal) const { ExplodedNode *N = C.generateNonFatalErrorNode(); - reportBug(Msg, C.getBugReporter(), N); + reportBug(Msg, C.getBugReporter(), N, ExprVal); return N; } ExplodedNode *ExprInspectionChecker::reportBug(llvm::StringRef Msg, BugReporter &BR, - ExplodedNode *N) const { + ExplodedNode *N, + Optional<SVal> ExprVal) const { if (!N) return nullptr; if (!BT) BT.reset(new BugType(this, "Checking analyzer assumptions", "debug")); - BR.emitReport(std::make_unique<PathSensitiveBugReport>(*BT, Msg, N)); + auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N); + if (ExprVal) { + R->markInteresting(*ExprVal); + } + BR.emitReport(std::move(R)); return N; } @@ -406,7 +415,8 @@ void ExprInspectionChecker::analyzerExpress(const CallExpr *CE, return; } - SymbolRef Sym = C.getSVal(CE->getArg(0)).getAsSymbol(); + SVal ArgVal = C.getSVal(CE->getArg(0)); + SymbolRef Sym = ArgVal.getAsSymbol(); if (!Sym) { reportBug("Not a symbol", C); return; @@ -419,7 +429,7 @@ void ExprInspectionChecker::analyzerExpress(const CallExpr *CE, return; } - reportBug(*Str, C); + reportBug(*Str, C, ArgVal); } void ExprInspectionChecker::analyzerIsTainted(const CallExpr *CE, diff --git a/clang/test/Analysis/container-modeling.cpp b/clang/test/Analysis/container-modeling.cpp index 9d63cc2fce69..bf4a12a0e0fe 100644 --- a/clang/test/Analysis/container-modeling.cpp +++ b/clang/test/Analysis/container-modeling.cpp @@ -76,8 +76,7 @@ void push_back(std::vector<int> &V, int n) { clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()"); clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()"); - V.push_back(n); // expected-note{{Container 'V' extended to the back by 1 position}} - // expected-note@-1{{Container 'V' extended to the back by 1 position}} + V.push_back(n); // expected-note 2{{Container 'V' extended to the back by 1 position}} clang_analyzer_express(clang_analyzer_container_begin(V)); // expected-warning{{$V.begin()}} // expected-note@-1{{$V.begin()}} @@ -99,7 +98,6 @@ void emplace_back(std::vector<int> &V, int n) { V.emplace_back(n); // expected-note 2{{Container 'V' extended to the back by 1 position}} - clang_analyzer_express(clang_analyzer_container_begin(V)); // expected-warning{{$V.begin()}} // expected-note@-1{{$V.begin()}} clang_analyzer_express(clang_analyzer_container_end(V)); // expected-warning{{$V.end() + 1}} @@ -217,10 +215,10 @@ void push_back1(std::vector<int> &V1, std::vector<int> &V2, int n) { clang_analyzer_denote(clang_analyzer_container_begin(V1), "$V1.begin()"); - V2.push_back(n); // expected-note{{Container 'V2' extended to the back by 1 position}} FIXME: This note should not appear since `V2` is not affected in the "bug" + V2.push_back(n); // no-note clang_analyzer_express(clang_analyzer_container_begin(V1)); // expected-warning{{$V1.begin()}} - // expected-note@-1{{$V1.begin()}} + // expected-note@-1{{$V1.begin()}} } void push_back2(std::vector<int> &V1, std::vector<int> &V2, int n) { @@ -232,15 +230,14 @@ void push_back2(std::vector<int> &V1, std::vector<int> &V2, int n) { clang_analyzer_denote(clang_analyzer_container_begin(V1), "$V1.begin()"); clang_analyzer_denote(clang_analyzer_container_begin(V2), "$V2.begin()"); - V1.push_back(n); // expected-note 2{{Container 'V1' extended to the back by 1 position}} - // FIXME: This should appear only once since there is only - // one "bug" where `V1` is affected + V1.push_back(n); // expected-note{{Container 'V1' extended to the back by 1 position}} + // Only once! clang_analyzer_express(clang_analyzer_container_begin(V1)); // expected-warning{{$V1.begin()}} - // expected-note@-1{{$V1.begin()}} + // expected-note@-1{{$V1.begin()}} clang_analyzer_express(clang_analyzer_container_begin(V2)); // expected-warning{{$V2.begin()}} - // expected-note@-1{{$V2.begin()}} + // expected-note@-1{{$V2.begin()}} } /// Print Container Data as Part of the Program State _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits