From 1a27d63a8891076ad9176f1a70f372003bc55c2f Mon Sep 17 00:00:00 2001 From: Adam Balogh Date: Thu, 30 Jan 2020 13:54:52 +0100 Subject: [PATCH] [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 --- .../StaticAnalyzer/Checkers/ContainerModeling.cpp | 3 +++ .../Checkers/DebugContainerModeling.cpp | 14 +++++++++++- .../Checkers/ExprInspectionChecker.cpp | 26 +++++++++++++++------- clang/test/Analysis/container-modeling.cpp | 17 ++++++-------- 4 files changed, 41 insertions(+), 19 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp index b225a61..8126fe8 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 8d05727..ce8dccb 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 10b2783..97e287e7 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp @@ -52,9 +52,12 @@ class ExprInspectionChecker : public Checker ExprVal = None) const; ExplodedNode *reportBug(llvm::StringRef Msg, BugReporter &BR, - ExplodedNode *N) const; + ExplodedNode *N, + Optional 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 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 ExprVal) const { if (!N) return nullptr; if (!BT) BT.reset(new BugType(this, "Checking analyzer assumptions", "debug")); - BR.emitReport(std::make_unique(*BT, Msg, N)); + auto R = std::make_unique(*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 9d63cc2..bf4a12a 100644 --- a/clang/test/Analysis/container-modeling.cpp +++ b/clang/test/Analysis/container-modeling.cpp @@ -76,8 +76,7 @@ void push_back(std::vector &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 &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 &V1, std::vector &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 &V1, std::vector &V2, int n) { @@ -232,15 +230,14 @@ void push_back2(std::vector &V1, std::vector &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 -- 2.7.4