[clang] [analyzer][clangsa] Add new option to alpha.security.cert.InvalidPtrChecker (PR #67663)
Endre Fülöp via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 24 01:46:38 PDT 2023
https://github.com/gamesh411 updated https://github.com/llvm/llvm-project/pull/67663
>From 65a4bf21e38a925f643c6cca3d3cad4c2910071c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.se>
Date: Fri, 8 Sep 2023 14:20:00 +0200
Subject: [PATCH 01/12] [analyzer][clangsa] Add new option to
alpha.security.cert.InvalidPtrChecker
The invalidation of pointer pointers returned by subsequent calls to genenv is
suggested by the POSIX standard, but is too strict from a practical point of
view. A new checker option 'InvalidatingGetEnv' is introduced, and is set to a
more lax default value, which does not consider consecutive getenv calls
invalidating.
The handling of the main function's possible specification where an environment
pointer is also pecified as a third parameter is also considered now.
Differential Revision: https://reviews.llvm.org/D154603
---
.../clang/StaticAnalyzer/Checkers/Checkers.td | 9 ++
.../Checkers/cert/InvalidPtrChecker.cpp | 86 ++++++++++++++-----
clang/test/Analysis/analyzer-config.c | 1 +
.../Analysis/cert/env34-c-cert-examples.c | 40 ++++++++-
clang/test/Analysis/cert/env34-c.c | 1 +
clang/test/Analysis/invalid-ptr-checker.c | 50 +++++++++++
6 files changed, 163 insertions(+), 24 deletions(-)
create mode 100644 clang/test/Analysis/invalid-ptr-checker.c
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index be813bde8be41ea..b6e9f0fae1c7f48 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1002,6 +1002,15 @@ let ParentPackage = ENV in {
def InvalidPtrChecker : Checker<"InvalidPtr">,
HelpText<"Finds usages of possibly invalidated pointers">,
+ CheckerOptions<[
+ CmdLineOption<Boolean,
+ "InvalidatingGetEnv",
+ "Regard getenv as an invalidating call (as per POSIX "
+ "standard), which can lead to false positives depending on "
+ "implementation.",
+ "false",
+ InAlpha>,
+ ]>,
Documentation<HasDocumentation>;
} // end "alpha.cert.env"
diff --git a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
index aae1a17bc0ae53e..8849eb1148564b7 100644
--- a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
@@ -38,6 +38,15 @@ class InvalidPtrChecker
CheckerContext &C) const;
// SEI CERT ENV31-C
+
+ // If set to true, consider getenv calls as invalidating operations on the
+ // environment variable buffer. This is implied in the standard, but in
+ // practice does not cause problems (in the commonly used environments).
+ bool InvalidatingGetEnv = false;
+
+ // GetEnv can be treated invalidating and non-invalidating as well.
+ const CallDescription GetEnvCall{{"getenv"}, 1};
+
const CallDescriptionMap<HandlerFn> EnvpInvalidatingFunctions = {
{{{"setenv"}, 3}, &InvalidPtrChecker::EnvpInvalidatingCall},
{{{"unsetenv"}, 1}, &InvalidPtrChecker::EnvpInvalidatingCall},
@@ -51,7 +60,6 @@ class InvalidPtrChecker
// SEI CERT ENV34-C
const CallDescriptionMap<HandlerFn> PreviousCallInvalidatingFunctions = {
- {{{"getenv"}, 1}, &InvalidPtrChecker::postPreviousReturnInvalidatingCall},
{{{"setlocale"}, 2},
&InvalidPtrChecker::postPreviousReturnInvalidatingCall},
{{{"strerror"}, 1},
@@ -62,6 +70,10 @@ class InvalidPtrChecker
&InvalidPtrChecker::postPreviousReturnInvalidatingCall},
};
+ // The private members of this checker corresponding to commandline options
+ // are set in this function.
+ friend void ento::registerInvalidPtrChecker(CheckerManager &);
+
public:
// Obtain the environment pointer from 'main()' (if present).
void checkBeginFunction(CheckerContext &C) const;
@@ -84,7 +96,10 @@ class InvalidPtrChecker
REGISTER_SET_WITH_PROGRAMSTATE(InvalidMemoryRegions, const MemRegion *)
// Stores the region of the environment pointer of 'main' (if present).
-REGISTER_TRAIT_WITH_PROGRAMSTATE(EnvPtrRegion, const MemRegion *)
+REGISTER_TRAIT_WITH_PROGRAMSTATE(MainEnvPtrRegion, const MemRegion *)
+
+// Stores the regions of environments returned by getenv calls.
+REGISTER_SET_WITH_PROGRAMSTATE(GetenvEnvPtrRegions, const MemRegion *)
// Stores key-value pairs, where key is function declaration and value is
// pointer to memory region returned by previous call of this function
@@ -95,22 +110,35 @@ void InvalidPtrChecker::EnvpInvalidatingCall(const CallEvent &Call,
CheckerContext &C) const {
StringRef FunctionName = Call.getCalleeIdentifier()->getName();
ProgramStateRef State = C.getState();
- const MemRegion *SymbolicEnvPtrRegion = State->get<EnvPtrRegion>();
- if (!SymbolicEnvPtrRegion)
- return;
- State = State->add<InvalidMemoryRegions>(SymbolicEnvPtrRegion);
+ auto PlaceInvalidationNote = [&C, FunctionName,
+ &State](const MemRegion *Region,
+ StringRef Message, ExplodedNode *Pred) {
+ State = State->add<InvalidMemoryRegions>(Region);
+
+ // Make copy of string data for the time when notes are *actually* created.
+ const NoteTag *Note =
+ C.getNoteTag([Region, FunctionName = std::string{FunctionName},
+ Message = std::string{Message}](
+ PathSensitiveBugReport &BR, llvm::raw_ostream &Out) {
+ if (!BR.isInteresting(Region))
+ return;
+ Out << '\'' << FunctionName << "' " << Message;
+ });
+ return C.addTransition(State, Pred, Note);
+ };
- const NoteTag *Note =
- C.getNoteTag([SymbolicEnvPtrRegion, FunctionName](
- PathSensitiveBugReport &BR, llvm::raw_ostream &Out) {
- if (!BR.isInteresting(SymbolicEnvPtrRegion))
- return;
- Out << '\'' << FunctionName
- << "' call may invalidate the environment parameter of 'main'";
- });
+ ExplodedNode *CurrentChainEnd = C.getPredecessor();
+
+ if (const MemRegion *MainEnvPtr = State->get<MainEnvPtrRegion>())
+ CurrentChainEnd = PlaceInvalidationNote(
+ MainEnvPtr, "call may invalidate the environment parameter of 'main'",
+ CurrentChainEnd);
- C.addTransition(State, Note);
+ for (const MemRegion *EnvPtr : State->get<GetenvEnvPtrRegions>())
+ CurrentChainEnd = PlaceInvalidationNote(
+ EnvPtr, "call may invalidate the environment returned by getenv",
+ CurrentChainEnd);
}
void InvalidPtrChecker::postPreviousReturnInvalidatingCall(
@@ -146,8 +174,7 @@ void InvalidPtrChecker::postPreviousReturnInvalidatingCall(
// Remember to this region.
const auto *SymRegOfRetVal = cast<SymbolicRegion>(RetVal.getAsRegion());
- const MemRegion *MR =
- const_cast<MemRegion *>(SymRegOfRetVal->getBaseRegion());
+ const MemRegion *MR = SymRegOfRetVal->getBaseRegion();
State = State->set<PreviousCallResultMap>(FD, MR);
ExplodedNode *Node = C.addTransition(State, Note);
@@ -185,6 +212,18 @@ static const MemRegion *findInvalidatedSymbolicBase(ProgramStateRef State,
// function call as an argument.
void InvalidPtrChecker::checkPostCall(const CallEvent &Call,
CheckerContext &C) const {
+
+ ProgramStateRef State = C.getState();
+
+ // Model 'getenv' calls
+ if (GetEnvCall.matches(Call)) {
+ const MemRegion *Region = Call.getReturnValue().getAsRegion();
+ if (Region) {
+ State = State->add<GetenvEnvPtrRegions>(Region);
+ C.addTransition(State);
+ }
+ }
+
// Check if function invalidates 'envp' argument of 'main'
if (const auto *Handler = EnvpInvalidatingFunctions.lookup(Call))
(this->**Handler)(Call, C);
@@ -193,14 +232,16 @@ void InvalidPtrChecker::checkPostCall(const CallEvent &Call,
if (const auto *Handler = PreviousCallInvalidatingFunctions.lookup(Call))
(this->**Handler)(Call, C);
+ // If pedantic mode is on, regard 'getenv' calls invalidating as well
+ if (InvalidatingGetEnv && GetEnvCall.matches(Call))
+ postPreviousReturnInvalidatingCall(Call, C);
+
// Check if one of the arguments of the function call is invalidated
// If call was inlined, don't report invalidated argument
if (C.wasInlined)
return;
- ProgramStateRef State = C.getState();
-
for (unsigned I = 0, NumArgs = Call.getNumArgs(); I < NumArgs; ++I) {
if (const auto *SR = dyn_cast_or_null<SymbolicRegion>(
@@ -243,7 +284,7 @@ void InvalidPtrChecker::checkBeginFunction(CheckerContext &C) const {
// Save the memory region pointed by the environment pointer parameter of
// 'main'.
- C.addTransition(State->set<EnvPtrRegion>(EnvpReg));
+ C.addTransition(State->set<MainEnvPtrRegion>(EnvpReg));
}
// Check if invalidated region is being dereferenced.
@@ -268,7 +309,10 @@ void InvalidPtrChecker::checkLocation(SVal Loc, bool isLoad, const Stmt *S,
}
void ento::registerInvalidPtrChecker(CheckerManager &Mgr) {
- Mgr.registerChecker<InvalidPtrChecker>();
+ auto *Checker = Mgr.registerChecker<InvalidPtrChecker>();
+ Checker->InvalidatingGetEnv =
+ Mgr.getAnalyzerOptions().getCheckerBooleanOption(Checker,
+ "InvalidatingGetEnv");
}
bool ento::shouldRegisterInvalidPtrChecker(const CheckerManager &) {
diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c
index 794ef8b9cc08680..8abfbf84983d811 100644
--- a/clang/test/Analysis/analyzer-config.c
+++ b/clang/test/Analysis/analyzer-config.c
@@ -11,6 +11,7 @@
// CHECK-NEXT: alpha.osx.cocoa.DirectIvarAssignment:AnnotatedFunctions = false
// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04
// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01
+// CHECK-NEXT: alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv = false
// CHECK-NEXT: alpha.security.taint.TaintPropagation:Config = ""
// CHECK-NEXT: alpha.unix.Errno:AllowErrnoReadOutsideConditionExpressions = true
// CHECK-NEXT: apply-fixits = false
diff --git a/clang/test/Analysis/cert/env34-c-cert-examples.c b/clang/test/Analysis/cert/env34-c-cert-examples.c
index 19ca73f34c7ee5e..7e5b9be3cba6bb0 100644
--- a/clang/test/Analysis/cert/env34-c-cert-examples.c
+++ b/clang/test/Analysis/cert/env34-c-cert-examples.c
@@ -1,15 +1,49 @@
+// Default options.
// RUN: %clang_analyze_cc1 \
// RUN: -analyzer-checker=core,alpha.security.cert.env.InvalidPtr \
// RUN: -verify -Wno-unused %s
+//
+// Test the laxer handling of getenv function (this is the default).
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=core,alpha.security.cert.env.InvalidPtr \
+// RUN: -analyzer-config alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=false \
+// RUN: -verify -Wno-unused %s
+//
+// Test the stricter handling of getenv function.
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=core,alpha.security.cert.env.InvalidPtr \
+// RUN: -analyzer-config alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
+// RUN: -verify=pedantic -Wno-unused %s
#include "../Inputs/system-header-simulator.h"
char *getenv(const char *name);
+int setenv(const char *name, const char *value, int overwrite);
int strcmp(const char*, const char*);
char *strdup(const char*);
void free(void *memblock);
void *malloc(size_t size);
-void incorrect_usage(void) {
+void incorrect_usage_setenv_getenv_invalidation(void) {
+ char *tmpvar;
+ char *tempvar;
+
+ tmpvar = getenv("TMP");
+
+ if (!tmpvar)
+ return;
+
+ setenv("TEMP", "", 1); //setenv can invalidate env
+
+ if (!tmpvar)
+ return;
+
+ if (strcmp(tmpvar, "") == 0) { // body of strcmp is unknown
+ // expected-warning at -1{{use of invalidated pointer 'tmpvar' in a function call}}
+ // pedantic-warning at -2{{use of invalidated pointer 'tmpvar' in a function call}}
+ }
+}
+
+void incorrect_usage_double_getenv_invalidation(void) {
char *tmpvar;
char *tempvar;
@@ -18,13 +52,13 @@ void incorrect_usage(void) {
if (!tmpvar)
return;
- tempvar = getenv("TEMP");
+ tempvar = getenv("TEMP"); //getenv should not invalidate env in non-pedantic mode
if (!tempvar)
return;
if (strcmp(tmpvar, tempvar) == 0) { // body of strcmp is unknown
- // expected-warning at -1{{use of invalidated pointer 'tmpvar' in a function call}}
+ // pedantic-warning at -1{{use of invalidated pointer 'tmpvar' in a function call}}
}
}
diff --git a/clang/test/Analysis/cert/env34-c.c b/clang/test/Analysis/cert/env34-c.c
index dc7b0340c311ed5..94bc2cf95ccc9b0 100644
--- a/clang/test/Analysis/cert/env34-c.c
+++ b/clang/test/Analysis/cert/env34-c.c
@@ -1,5 +1,6 @@
// RUN: %clang_analyze_cc1 \
// RUN: -analyzer-checker=alpha.security.cert.env.InvalidPtr\
+// RUN: -analyzer-config alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
// RUN: -analyzer-output=text -verify -Wno-unused %s
#include "../Inputs/system-header-simulator.h"
diff --git a/clang/test/Analysis/invalid-ptr-checker.c b/clang/test/Analysis/invalid-ptr-checker.c
new file mode 100644
index 000000000000000..d2250e03f3d3d2f
--- /dev/null
+++ b/clang/test/Analysis/invalid-ptr-checker.c
@@ -0,0 +1,50 @@
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=alpha.security.cert.env.InvalidPtr \
+// RUN: -analyzer-config alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=false \
+// RUN: -analyzer-output=text -verify -Wno-unused %s
+//
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=alpha.security.cert.env.InvalidPtr \
+// RUN: -analyzer-config \
+// RUN: alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
+// RUN: -analyzer-output=text -verify=expected,pedantic -Wno-unused %s
+
+#include "Inputs/system-header-simulator.h"
+
+char *getenv(const char *name);
+int setenv(const char *name, const char *value, int overwrite);
+int strcmp(const char *, const char *);
+
+int custom_env_handler(const char **envp);
+
+void getenv_after_getenv(void) {
+ char *v1 = getenv("V1");
+ // pedantic-note at -1{{previous function call was here}}
+
+ char *v2 = getenv("V2");
+ // pedantic-note at -1{{'getenv' call may invalidate the result of the previous 'getenv'}}
+
+ strcmp(v1, v2);
+ // pedantic-warning at -1{{use of invalidated pointer 'v1' in a function call}}
+ // pedantic-note at -2{{use of invalidated pointer 'v1' in a function call}}
+}
+
+void setenv_after_getenv(void) {
+ char *v1 = getenv("VAR1");
+
+ setenv("VAR2", "...", 1);
+ // expected-note at -1{{'setenv' call may invalidate the environment returned by getenv}}
+
+ strcmp(v1, "");
+ // expected-warning at -1{{use of invalidated pointer 'v1' in a function call}}
+ // expected-note at -2{{use of invalidated pointer 'v1' in a function call}}
+}
+
+int main(int argc, const char *argv[], const char *envp[]) {
+ setenv("VAR", "...", 0);
+ // expected-note at -1 2 {{'setenv' call may invalidate the environment parameter of 'main'}}
+
+ *envp;
+ // expected-warning at -1 2 {{dereferencing an invalid pointer}}
+ // expected-note at -2 2 {{dereferencing an invalid pointer}}
+}
>From f824e39731bbe736789231770e6de7f7dc64fc17 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.se>
Date: Thu, 21 Sep 2023 16:03:46 +0200
Subject: [PATCH 02/12] Add explicit State parameter
---
.../Checkers/cert/InvalidPtrChecker.cpp | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
index 8849eb1148564b7..5b576ace1d547d3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
@@ -109,11 +109,11 @@ REGISTER_MAP_WITH_PROGRAMSTATE(PreviousCallResultMap, const FunctionDecl *,
void InvalidPtrChecker::EnvpInvalidatingCall(const CallEvent &Call,
CheckerContext &C) const {
StringRef FunctionName = Call.getCalleeIdentifier()->getName();
- ProgramStateRef State = C.getState();
- auto PlaceInvalidationNote = [&C, FunctionName,
- &State](const MemRegion *Region,
- StringRef Message, ExplodedNode *Pred) {
+ auto PlaceInvalidationNote = [&C, FunctionName](ProgramStateRef State,
+ const MemRegion *Region,
+ StringRef Message,
+ ExplodedNode *Pred) {
State = State->add<InvalidMemoryRegions>(Region);
// Make copy of string data for the time when notes are *actually* created.
@@ -128,16 +128,18 @@ void InvalidPtrChecker::EnvpInvalidatingCall(const CallEvent &Call,
return C.addTransition(State, Pred, Note);
};
+ ProgramStateRef State = C.getState();
ExplodedNode *CurrentChainEnd = C.getPredecessor();
if (const MemRegion *MainEnvPtr = State->get<MainEnvPtrRegion>())
CurrentChainEnd = PlaceInvalidationNote(
- MainEnvPtr, "call may invalidate the environment parameter of 'main'",
+ State, MainEnvPtr,
+ "call may invalidate the environment parameter of 'main'",
CurrentChainEnd);
for (const MemRegion *EnvPtr : State->get<GetenvEnvPtrRegions>())
CurrentChainEnd = PlaceInvalidationNote(
- EnvPtr, "call may invalidate the environment returned by getenv",
+ State, EnvPtr, "call may invalidate the environment returned by getenv",
CurrentChainEnd);
}
>From 659b2cc01b1dcf60d947a848d21de6d7a7ef3dfe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.se>
Date: Thu, 21 Sep 2023 16:04:13 +0200
Subject: [PATCH 03/12] Add NoteTag filtering based on exact BugType* equality
---
.../Checkers/cert/InvalidPtrChecker.cpp | 26 ++++++++++++++-----
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
index 5b576ace1d547d3..4a8e71743f3a845 100644
--- a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
@@ -25,12 +25,20 @@
using namespace clang;
using namespace ento;
+
namespace {
+
class InvalidPtrChecker
: public Checker<check::Location, check::BeginFunction, check::PostCall> {
private:
- BugType BT{this, "Use of invalidated pointer", categories::MemoryError};
+ static const BugType *InvalidPtrBugType;
+ // For accurate emission of NoteTags, the BugType of this checker should have
+ // a unique address.
+ void InitInvalidPtrBugType() {
+ InvalidPtrBugType = new BugType(this, "Use of invalidated pointer",
+ categories::MemoryError);
+ }
void EnvpInvalidatingCall(const CallEvent &Call, CheckerContext &C) const;
@@ -90,6 +98,8 @@ class InvalidPtrChecker
CheckerContext &C) const;
};
+const BugType *InvalidPtrChecker::InvalidPtrBugType;
+
} // namespace
// Set of memory regions that were invalidated
@@ -121,7 +131,8 @@ void InvalidPtrChecker::EnvpInvalidatingCall(const CallEvent &Call,
C.getNoteTag([Region, FunctionName = std::string{FunctionName},
Message = std::string{Message}](
PathSensitiveBugReport &BR, llvm::raw_ostream &Out) {
- if (!BR.isInteresting(Region))
+ if (!BR.isInteresting(Region) ||
+ &BR.getBugType() != InvalidPtrBugType)
return;
Out << '\'' << FunctionName << "' " << Message;
});
@@ -156,7 +167,7 @@ void InvalidPtrChecker::postPreviousReturnInvalidatingCall(
State = State->add<InvalidMemoryRegions>(PrevReg);
Note = C.getNoteTag([PrevReg, FD](PathSensitiveBugReport &BR,
llvm::raw_ostream &Out) {
- if (!BR.isInteresting(PrevReg))
+ if (!BR.isInteresting(PrevReg) || &BR.getBugType() != InvalidPtrBugType)
return;
Out << '\'';
FD->getNameForDiagnostic(Out, FD->getASTContext().getLangOpts(), true);
@@ -182,7 +193,7 @@ void InvalidPtrChecker::postPreviousReturnInvalidatingCall(
ExplodedNode *Node = C.addTransition(State, Note);
const NoteTag *PreviousCallNote =
C.getNoteTag([MR](PathSensitiveBugReport &BR, llvm::raw_ostream &Out) {
- if (!BR.isInteresting(MR))
+ if (!BR.isInteresting(MR) || &BR.getBugType() != InvalidPtrBugType)
return;
Out << '\'' << "'previous function call was here" << '\'';
});
@@ -261,8 +272,8 @@ void InvalidPtrChecker::checkPostCall(const CallEvent &Call,
C.getASTContext().getPrintingPolicy());
Out << "' in a function call";
- auto Report =
- std::make_unique<PathSensitiveBugReport>(BT, Out.str(), ErrorNode);
+ auto Report = std::make_unique<PathSensitiveBugReport>(
+ *InvalidPtrBugType, Out.str(), ErrorNode);
Report->markInteresting(InvalidatedSymbolicBase);
Report->addRange(Call.getArgSourceRange(I));
C.emitReport(std::move(Report));
@@ -305,7 +316,7 @@ void InvalidPtrChecker::checkLocation(SVal Loc, bool isLoad, const Stmt *S,
return;
auto Report = std::make_unique<PathSensitiveBugReport>(
- BT, "dereferencing an invalid pointer", ErrorNode);
+ *InvalidPtrBugType, "dereferencing an invalid pointer", ErrorNode);
Report->markInteresting(InvalidatedSymbolicBase);
C.emitReport(std::move(Report));
}
@@ -315,6 +326,7 @@ void ento::registerInvalidPtrChecker(CheckerManager &Mgr) {
Checker->InvalidatingGetEnv =
Mgr.getAnalyzerOptions().getCheckerBooleanOption(Checker,
"InvalidatingGetEnv");
+ Checker->InitInvalidPtrBugType();
}
bool ento::shouldRegisterInvalidPtrChecker(const CheckerManager &) {
>From 23cd14b0a9fe3b4f9d7ae952e25cfaa21de93b7f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.se>
Date: Thu, 21 Sep 2023 16:36:06 +0200
Subject: [PATCH 04/12] Mark region uninteresing after placing Note
---
clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
index 4a8e71743f3a845..e394705cee85561 100644
--- a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
@@ -135,6 +135,7 @@ void InvalidPtrChecker::EnvpInvalidatingCall(const CallEvent &Call,
&BR.getBugType() != InvalidPtrBugType)
return;
Out << '\'' << FunctionName << "' " << Message;
+ BR.markNotInteresting(Region);
});
return C.addTransition(State, Pred, Note);
};
>From 84d96acbae28987481e8183e6e98cc44be658678 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.se>
Date: Mon, 25 Sep 2023 10:01:31 +0200
Subject: [PATCH 05/12] Add documentation and example of the option
---
clang/docs/analyzer/checkers.rst | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 597ffcc4a10a25b..43137f4b020f9f7 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -2520,13 +2520,34 @@ pointer. These functions include: getenv, localeconv, asctime, setlocale, strerr
char *p, *pp;
p = getenv("VAR");
- pp = getenv("VAR2");
- // subsequent call to 'getenv' invalidated previous one
+ setenv("SOMEVAR", "VALUE", /*overwrite = */1);
+ // call to 'setenv' may invalidate p
*p;
// dereferencing invalid pointer
}
+
+The ``InvalidatingGetEnv`` option is available for treating getenv calls as
+invalidating. When enabled, the checker issues a warning if getenv is called
+multiple times and their results are used without first creating a copy.
+This level of strictness might be considered overly pedantic for the commonly
+used getenv implementations.
+
+To enable this option, use:
+``-analyzer-config alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=true``.
+
+By default, this option is set to *false*.
+
+When this option is enabled, warnings will be generated for scenarios like the
+following:
+
+.. code-block:: c
+
+ char* p = getenv("VAR");
+ char* pp = getenv("VAR2"); // assumes this call can invalidate `env`
+ strlen(p); // warns about accessing invalid ptr
+
alpha.security.taint
^^^^^^^^^^^^^^^^^^^^
>From 51a8ae41b44475e531ea3091eca30ce417b86c3b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.se>
Date: Tue, 26 Sep 2023 11:00:06 +0200
Subject: [PATCH 06/12] Remove extra quote in note tag message
---
clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
index e394705cee85561..b233d2197c3a3a4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
@@ -196,7 +196,7 @@ void InvalidPtrChecker::postPreviousReturnInvalidatingCall(
C.getNoteTag([MR](PathSensitiveBugReport &BR, llvm::raw_ostream &Out) {
if (!BR.isInteresting(MR) || &BR.getBugType() != InvalidPtrBugType)
return;
- Out << '\'' << "'previous function call was here" << '\'';
+ Out << "previous function call was here";
});
C.addTransition(State, Node, PreviousCallNote);
>From 5af96f79ecc1d956f0b3a79f3de564f57eb80172 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.se>
Date: Mon, 2 Oct 2023 10:29:53 +0200
Subject: [PATCH 07/12] Return to member variable BugType, as suggested by
@DonatNagyE
---
.../Checkers/cert/InvalidPtrChecker.cpp | 40 ++++++++-----------
1 file changed, 16 insertions(+), 24 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
index b233d2197c3a3a4..be6657bb28381fc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
@@ -25,20 +25,15 @@
using namespace clang;
using namespace ento;
-
namespace {
-
class InvalidPtrChecker
: public Checker<check::Location, check::BeginFunction, check::PostCall> {
private:
- static const BugType *InvalidPtrBugType;
// For accurate emission of NoteTags, the BugType of this checker should have
// a unique address.
- void InitInvalidPtrBugType() {
- InvalidPtrBugType = new BugType(this, "Use of invalidated pointer",
- categories::MemoryError);
- }
+ BugType InvalidPtrBugType{this, "Use of invalidated pointer",
+ categories::MemoryError};
void EnvpInvalidatingCall(const CallEvent &Call, CheckerContext &C) const;
@@ -98,8 +93,6 @@ class InvalidPtrChecker
CheckerContext &C) const;
};
-const BugType *InvalidPtrChecker::InvalidPtrBugType;
-
} // namespace
// Set of memory regions that were invalidated
@@ -120,19 +113,19 @@ void InvalidPtrChecker::EnvpInvalidatingCall(const CallEvent &Call,
CheckerContext &C) const {
StringRef FunctionName = Call.getCalleeIdentifier()->getName();
- auto PlaceInvalidationNote = [&C, FunctionName](ProgramStateRef State,
- const MemRegion *Region,
- StringRef Message,
- ExplodedNode *Pred) {
+ auto PlaceInvalidationNote = [this, &C, FunctionName](ProgramStateRef State,
+ const MemRegion *Region,
+ StringRef Message,
+ ExplodedNode *Pred) {
State = State->add<InvalidMemoryRegions>(Region);
// Make copy of string data for the time when notes are *actually* created.
const NoteTag *Note =
- C.getNoteTag([Region, FunctionName = std::string{FunctionName},
+ C.getNoteTag([this, Region, FunctionName = std::string{FunctionName},
Message = std::string{Message}](
PathSensitiveBugReport &BR, llvm::raw_ostream &Out) {
if (!BR.isInteresting(Region) ||
- &BR.getBugType() != InvalidPtrBugType)
+ &BR.getBugType() != &InvalidPtrBugType)
return;
Out << '\'' << FunctionName << "' " << Message;
BR.markNotInteresting(Region);
@@ -166,9 +159,9 @@ void InvalidPtrChecker::postPreviousReturnInvalidatingCall(
if (const MemRegion *const *Reg = State->get<PreviousCallResultMap>(FD)) {
const MemRegion *PrevReg = *Reg;
State = State->add<InvalidMemoryRegions>(PrevReg);
- Note = C.getNoteTag([PrevReg, FD](PathSensitiveBugReport &BR,
- llvm::raw_ostream &Out) {
- if (!BR.isInteresting(PrevReg) || &BR.getBugType() != InvalidPtrBugType)
+ Note = C.getNoteTag([this, PrevReg, FD](PathSensitiveBugReport &BR,
+ llvm::raw_ostream &Out) {
+ if (!BR.isInteresting(PrevReg) || &BR.getBugType() != &InvalidPtrBugType)
return;
Out << '\'';
FD->getNameForDiagnostic(Out, FD->getASTContext().getLangOpts(), true);
@@ -192,9 +185,9 @@ void InvalidPtrChecker::postPreviousReturnInvalidatingCall(
State = State->set<PreviousCallResultMap>(FD, MR);
ExplodedNode *Node = C.addTransition(State, Note);
- const NoteTag *PreviousCallNote =
- C.getNoteTag([MR](PathSensitiveBugReport &BR, llvm::raw_ostream &Out) {
- if (!BR.isInteresting(MR) || &BR.getBugType() != InvalidPtrBugType)
+ const NoteTag *PreviousCallNote = C.getNoteTag(
+ [this, MR](PathSensitiveBugReport &BR, llvm::raw_ostream &Out) {
+ if (!BR.isInteresting(MR) || &BR.getBugType() != &InvalidPtrBugType)
return;
Out << "previous function call was here";
});
@@ -274,7 +267,7 @@ void InvalidPtrChecker::checkPostCall(const CallEvent &Call,
Out << "' in a function call";
auto Report = std::make_unique<PathSensitiveBugReport>(
- *InvalidPtrBugType, Out.str(), ErrorNode);
+ InvalidPtrBugType, Out.str(), ErrorNode);
Report->markInteresting(InvalidatedSymbolicBase);
Report->addRange(Call.getArgSourceRange(I));
C.emitReport(std::move(Report));
@@ -317,7 +310,7 @@ void InvalidPtrChecker::checkLocation(SVal Loc, bool isLoad, const Stmt *S,
return;
auto Report = std::make_unique<PathSensitiveBugReport>(
- *InvalidPtrBugType, "dereferencing an invalid pointer", ErrorNode);
+ InvalidPtrBugType, "dereferencing an invalid pointer", ErrorNode);
Report->markInteresting(InvalidatedSymbolicBase);
C.emitReport(std::move(Report));
}
@@ -327,7 +320,6 @@ void ento::registerInvalidPtrChecker(CheckerManager &Mgr) {
Checker->InvalidatingGetEnv =
Mgr.getAnalyzerOptions().getCheckerBooleanOption(Checker,
"InvalidatingGetEnv");
- Checker->InitInvalidPtrBugType();
}
bool ento::shouldRegisterInvalidPtrChecker(const CheckerManager &) {
>From d38b52e42351f96efa38ace7c884466ce931df04 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.se>
Date: Tue, 3 Oct 2023 12:07:53 +0200
Subject: [PATCH 08/12] Create only a single NoteTag per invalidation
---
.../Checkers/cert/InvalidPtrChecker.cpp | 84 ++++++++++++-------
clang/test/Analysis/invalid-ptr-checker.c | 2 +-
2 files changed, 53 insertions(+), 33 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
index be6657bb28381fc..543282fabd3793c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
@@ -91,6 +91,11 @@ class InvalidPtrChecker
// Check if invalidated region is being dereferenced.
void checkLocation(SVal l, bool isLoad, const Stmt *S,
CheckerContext &C) const;
+
+private:
+ const NoteTag *createEnvInvalidationNote(CheckerContext &C,
+ ProgramStateRef State,
+ StringRef FunctionName) const;
};
} // namespace
@@ -109,43 +114,58 @@ REGISTER_SET_WITH_PROGRAMSTATE(GetenvEnvPtrRegions, const MemRegion *)
REGISTER_MAP_WITH_PROGRAMSTATE(PreviousCallResultMap, const FunctionDecl *,
const MemRegion *)
-void InvalidPtrChecker::EnvpInvalidatingCall(const CallEvent &Call,
- CheckerContext &C) const {
- StringRef FunctionName = Call.getCalleeIdentifier()->getName();
+const NoteTag *InvalidPtrChecker::createEnvInvalidationNote(
+ CheckerContext &C, ProgramStateRef State, StringRef FunctionName) const {
- auto PlaceInvalidationNote = [this, &C, FunctionName](ProgramStateRef State,
- const MemRegion *Region,
- StringRef Message,
- ExplodedNode *Pred) {
- State = State->add<InvalidMemoryRegions>(Region);
-
- // Make copy of string data for the time when notes are *actually* created.
- const NoteTag *Note =
- C.getNoteTag([this, Region, FunctionName = std::string{FunctionName},
- Message = std::string{Message}](
- PathSensitiveBugReport &BR, llvm::raw_ostream &Out) {
- if (!BR.isInteresting(Region) ||
- &BR.getBugType() != &InvalidPtrBugType)
- return;
- Out << '\'' << FunctionName << "' " << Message;
- BR.markNotInteresting(Region);
- });
- return C.addTransition(State, Pred, Note);
- };
+ const MemRegion *MainRegion = State->get<MainEnvPtrRegion>();
+ const auto GetenvRegions = State->get<GetenvEnvPtrRegions>();
- ProgramStateRef State = C.getState();
- ExplodedNode *CurrentChainEnd = C.getPredecessor();
+ return C.getNoteTag([this, MainRegion, GetenvRegions,
+ FunctionName = std::string{FunctionName}](
+ PathSensitiveBugReport &BR, llvm::raw_ostream &Out) {
+ auto IsInterestingForInvalidation = [this, &BR](const MemRegion *R) {
+ return R && &BR.getBugType() == &InvalidPtrBugType && BR.isInteresting(R);
+ };
- if (const MemRegion *MainEnvPtr = State->get<MainEnvPtrRegion>())
- CurrentChainEnd = PlaceInvalidationNote(
- State, MainEnvPtr,
- "call may invalidate the environment parameter of 'main'",
- CurrentChainEnd);
+ // Craft the note tag message.
+ llvm::SmallVector<std::string, 2> InvalidLocationNames;
+ if (IsInterestingForInvalidation(MainRegion)) {
+ InvalidLocationNames.push_back("the environment parameter of 'main'");
+ }
+ if (llvm::any_of(GetenvRegions, IsInterestingForInvalidation))
+ InvalidLocationNames.push_back("the environment returned by 'getenv'");
+
+ if (InvalidLocationNames.size() >= 1)
+ Out << '\'' << FunctionName << "' call may invalidate "
+ << InvalidLocationNames[0];
+ if (InvalidLocationNames.size() == 2)
+ Out << ", and " << InvalidLocationNames[1];
+
+ // Mark all regions that were interesting before as NOT interesting now
+ // to avoid extra notes coming from other checkers.
+ if (IsInterestingForInvalidation(MainRegion))
+ BR.markNotInteresting(MainRegion);
+ for (const MemRegion *GetenvRegion : GetenvRegions)
+ if (IsInterestingForInvalidation(GetenvRegion))
+ BR.markNotInteresting(GetenvRegion);
+ });
+}
+void InvalidPtrChecker::EnvpInvalidatingCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ // This callevent invalidates all previously generated pointers to the
+ // environment.
+ ProgramStateRef State = C.getState();
+ if (const MemRegion *MainEnvPtr = State->get<MainEnvPtrRegion>())
+ State = State->add<InvalidMemoryRegions>(MainEnvPtr);
for (const MemRegion *EnvPtr : State->get<GetenvEnvPtrRegions>())
- CurrentChainEnd = PlaceInvalidationNote(
- State, EnvPtr, "call may invalidate the environment returned by getenv",
- CurrentChainEnd);
+ State = State->add<InvalidMemoryRegions>(EnvPtr);
+
+ StringRef FunctionName = Call.getCalleeIdentifier()->getName();
+ const NoteTag *InvalidationNote =
+ createEnvInvalidationNote(C, State, FunctionName);
+
+ C.addTransition(State, InvalidationNote);
}
void InvalidPtrChecker::postPreviousReturnInvalidatingCall(
diff --git a/clang/test/Analysis/invalid-ptr-checker.c b/clang/test/Analysis/invalid-ptr-checker.c
index d2250e03f3d3d2f..4b257f9e78003fc 100644
--- a/clang/test/Analysis/invalid-ptr-checker.c
+++ b/clang/test/Analysis/invalid-ptr-checker.c
@@ -33,7 +33,7 @@ void setenv_after_getenv(void) {
char *v1 = getenv("VAR1");
setenv("VAR2", "...", 1);
- // expected-note at -1{{'setenv' call may invalidate the environment returned by getenv}}
+ // expected-note at -1{{'setenv' call may invalidate the environment returned by 'getenv'}}
strcmp(v1, "");
// expected-warning at -1{{use of invalidated pointer 'v1' in a function call}}
>From 86e1151c589aaaa53e623cc9cc43dafbba949026 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.se>
Date: Mon, 9 Oct 2023 10:00:36 +0200
Subject: [PATCH 09/12] Add test to ensure no extra notes are emitted for
multiple invalidation points
---
.../Checkers/cert/InvalidPtrChecker.cpp | 4 +++-
clang/test/Analysis/invalid-ptr-checker.c | 13 +++++++++++++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
index 543282fabd3793c..9b9019ed50f7b44 100644
--- a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
@@ -142,7 +142,9 @@ const NoteTag *InvalidPtrChecker::createEnvInvalidationNote(
Out << ", and " << InvalidLocationNames[1];
// Mark all regions that were interesting before as NOT interesting now
- // to avoid extra notes coming from other checkers.
+ // to avoid extra notes coming from invalidation points higher up the
+ // bugpath. This ensures, that only the last invalidation point is marked
+ // with a note tag.
if (IsInterestingForInvalidation(MainRegion))
BR.markNotInteresting(MainRegion);
for (const MemRegion *GetenvRegion : GetenvRegions)
diff --git a/clang/test/Analysis/invalid-ptr-checker.c b/clang/test/Analysis/invalid-ptr-checker.c
index 4b257f9e78003fc..e8ffee7fb479dc9 100644
--- a/clang/test/Analysis/invalid-ptr-checker.c
+++ b/clang/test/Analysis/invalid-ptr-checker.c
@@ -48,3 +48,16 @@ int main(int argc, const char *argv[], const char *envp[]) {
// expected-warning at -1 2 {{dereferencing an invalid pointer}}
// expected-note at -2 2 {{dereferencing an invalid pointer}}
}
+
+void multiple_invalidation_no_duplicate_notes(void) {
+ char *v1 = getenv("VAR1");
+
+ setenv("VAR2", "...", 1); // no note here
+
+ setenv("VAR3", "...", 1);
+ // expected-note at -1{{'setenv' call may invalidate the environment returned by 'getenv'}}
+
+ strcmp(v1, "");
+ // expected-warning at -1{{use of invalidated pointer 'v1' in a function call}}
+ // expected-note at -2{{use of invalidated pointer 'v1' in a function call}}
+}
>From 6494af8873c459c9431b61f91ff19557ac1c3307 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.se>
Date: Mon, 9 Oct 2023 10:15:03 +0200
Subject: [PATCH 10/12] Make testcase less verbose by using
'-verify=expected,pedantic'
---
clang/test/Analysis/cert/env34-c-cert-examples.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/clang/test/Analysis/cert/env34-c-cert-examples.c b/clang/test/Analysis/cert/env34-c-cert-examples.c
index 7e5b9be3cba6bb0..6d6659e55d86b93 100644
--- a/clang/test/Analysis/cert/env34-c-cert-examples.c
+++ b/clang/test/Analysis/cert/env34-c-cert-examples.c
@@ -13,7 +13,7 @@
// RUN: %clang_analyze_cc1 \
// RUN: -analyzer-checker=core,alpha.security.cert.env.InvalidPtr \
// RUN: -analyzer-config alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
-// RUN: -verify=pedantic -Wno-unused %s
+// RUN: -verify=expected,pedantic -Wno-unused %s
#include "../Inputs/system-header-simulator.h"
char *getenv(const char *name);
@@ -39,7 +39,6 @@ void incorrect_usage_setenv_getenv_invalidation(void) {
if (strcmp(tmpvar, "") == 0) { // body of strcmp is unknown
// expected-warning at -1{{use of invalidated pointer 'tmpvar' in a function call}}
- // pedantic-warning at -2{{use of invalidated pointer 'tmpvar' in a function call}}
}
}
>From ffe388170c40e7b0b2590e8bc700c56353d5b954 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.se>
Date: Tue, 17 Oct 2023 14:26:04 +0200
Subject: [PATCH 11/12] Refactor lambda, inline interesting handling
---
.../Checkers/cert/InvalidPtrChecker.cpp | 38 ++++++++++---------
1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
index 9b9019ed50f7b44..f00360f7e43b5d4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
@@ -123,33 +123,37 @@ const NoteTag *InvalidPtrChecker::createEnvInvalidationNote(
return C.getNoteTag([this, MainRegion, GetenvRegions,
FunctionName = std::string{FunctionName}](
PathSensitiveBugReport &BR, llvm::raw_ostream &Out) {
- auto IsInterestingForInvalidation = [this, &BR](const MemRegion *R) {
- return R && &BR.getBugType() == &InvalidPtrBugType && BR.isInteresting(R);
- };
+ // Only handle the BugType of this checker.
+ if (&BR.getBugType() != &InvalidPtrBugType)
+ return;
- // Craft the note tag message.
+ // Mark all regions that were interesting before as NOT interesting now
+ // to avoid extra notes coming from invalidation points higher up the
+ // bugpath. This ensures, that only the last invalidation point is marked
+ // with a note tag.
llvm::SmallVector<std::string, 2> InvalidLocationNames;
- if (IsInterestingForInvalidation(MainRegion)) {
+ if (BR.isInteresting(MainRegion)) {
+ BR.markNotInteresting(MainRegion);
InvalidLocationNames.push_back("the environment parameter of 'main'");
}
- if (llvm::any_of(GetenvRegions, IsInterestingForInvalidation))
- InvalidLocationNames.push_back("the environment returned by 'getenv'");
+ bool InterestingGetenvFound = false;
+ for (const MemRegion *MR : GetenvRegions) {
+ if (BR.isInteresting(MR)) {
+ BR.markNotInteresting(MR);
+ if (!InterestingGetenvFound) {
+ InterestingGetenvFound = true;
+ InvalidLocationNames.push_back(
+ "the environment returned by 'getenv'");
+ }
+ }
+ }
+ // Emit note tag message.
if (InvalidLocationNames.size() >= 1)
Out << '\'' << FunctionName << "' call may invalidate "
<< InvalidLocationNames[0];
if (InvalidLocationNames.size() == 2)
Out << ", and " << InvalidLocationNames[1];
-
- // Mark all regions that were interesting before as NOT interesting now
- // to avoid extra notes coming from invalidation points higher up the
- // bugpath. This ensures, that only the last invalidation point is marked
- // with a note tag.
- if (IsInterestingForInvalidation(MainRegion))
- BR.markNotInteresting(MainRegion);
- for (const MemRegion *GetenvRegion : GetenvRegions)
- if (IsInterestingForInvalidation(GetenvRegion))
- BR.markNotInteresting(GetenvRegion);
});
}
>From 2d0d8faef911bc8446563b085a0e08fdbf8e4074 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.se>
Date: Tue, 24 Oct 2023 10:29:40 +0200
Subject: [PATCH 12/12] Fix comment grammar
---
clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
index f00360f7e43b5d4..e5dd907c660d8ea 100644
--- a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
@@ -129,7 +129,7 @@ const NoteTag *InvalidPtrChecker::createEnvInvalidationNote(
// Mark all regions that were interesting before as NOT interesting now
// to avoid extra notes coming from invalidation points higher up the
- // bugpath. This ensures, that only the last invalidation point is marked
+ // bugpath. This ensures that only the last invalidation point is marked
// with a note tag.
llvm::SmallVector<std::string, 2> InvalidLocationNames;
if (BR.isInteresting(MainRegion)) {
More information about the cfe-commits
mailing list