[clang] 914f6c4 - [StaticAnalyzer] Support struct annotations in FuchsiaHandleChecker
Haowei Wu via cfe-commits
cfe-commits at lists.llvm.org
Sat Nov 21 20:02:55 PST 2020
Author: Haowei Wu
Date: 2020-11-21T19:59:51-08:00
New Revision: 914f6c4ff8a42d384cad0bbb07de4dd1a96c78d4
URL: https://github.com/llvm/llvm-project/commit/914f6c4ff8a42d384cad0bbb07de4dd1a96c78d4
DIFF: https://github.com/llvm/llvm-project/commit/914f6c4ff8a42d384cad0bbb07de4dd1a96c78d4.diff
LOG: [StaticAnalyzer] Support struct annotations in FuchsiaHandleChecker
Support adding handle annotations to sturucture that contains
handles. All the handles referenced by the structure (direct
value or ptr) would be treated as containing the
release/use/acquire annotations directly.
Patch by Yu Shan
Differential Revision: https://reviews.llvm.org/D91223
Added:
Modified:
clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
clang/test/Analysis/fuchsia_handle.cpp
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
index b2822e5307f3..d4901eb0abbb 100644
--- a/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -53,7 +53,7 @@
//
// Note that, the analyzer does not always know for sure if a function failed
// or succeeded. In those cases we use the state MaybeAllocated.
-// Thus, the diagramm above captures the intent, not implementation details.
+// Thus, the diagram above captures the intent, not implementation details.
//
// Due to the fact that the number of handle related syscalls in Fuchsia
// is large, we adopt the annotation attributes to descript syscalls'
@@ -226,32 +226,70 @@ static const ExplodedNode *getAcquireSite(const ExplodedNode *N, SymbolRef Sym,
return nullptr;
}
-/// Returns the symbols extracted from the argument or null if it cannot be
-/// found.
-static SymbolRef getFuchsiaHandleSymbol(QualType QT, SVal Arg,
- ProgramStateRef State) {
+namespace {
+class FuchsiaHandleSymbolVisitor final : public SymbolVisitor {
+public:
+ FuchsiaHandleSymbolVisitor(ProgramStateRef State) : State(std::move(State)) {}
+ ProgramStateRef getState() const { return State; }
+
+ bool VisitSymbol(SymbolRef S) override {
+ if (const auto *HandleType = S->getType()->getAs<TypedefType>())
+ if (HandleType->getDecl()->getName() == HandleTypeName)
+ Symbols.push_back(S);
+ return true;
+ }
+
+ SmallVector<SymbolRef, 1024> GetSymbols() { return Symbols; }
+
+private:
+ SmallVector<SymbolRef, 1024> Symbols;
+ ProgramStateRef State;
+};
+} // end anonymous namespace
+
+/// Returns the symbols extracted from the argument or empty vector if it cannot
+/// be found. It is unlikely to have over 1024 symbols in one argument.
+static SmallVector<SymbolRef, 1024>
+getFuchsiaHandleSymbols(QualType QT, SVal Arg, ProgramStateRef State) {
int PtrToHandleLevel = 0;
while (QT->isAnyPointerType() || QT->isReferenceType()) {
++PtrToHandleLevel;
QT = QT->getPointeeType();
}
+ if (QT->isStructureType()) {
+ // If we see a structure, see if there is any handle referenced by the
+ // structure.
+ FuchsiaHandleSymbolVisitor Visitor(State);
+ State->scanReachableSymbols(Arg, Visitor);
+ return Visitor.GetSymbols();
+ }
if (const auto *HandleType = QT->getAs<TypedefType>()) {
if (HandleType->getDecl()->getName() != HandleTypeName)
- return nullptr;
- if (PtrToHandleLevel > 1) {
+ return {};
+ if (PtrToHandleLevel > 1)
// Not supported yet.
- return nullptr;
- }
+ return {};
if (PtrToHandleLevel == 0) {
- return Arg.getAsSymbol();
+ SymbolRef Sym = Arg.getAsSymbol();
+ if (Sym) {
+ return {Sym};
+ } else {
+ return {};
+ }
} else {
assert(PtrToHandleLevel == 1);
- if (Optional<Loc> ArgLoc = Arg.getAs<Loc>())
- return State->getSVal(*ArgLoc).getAsSymbol();
+ if (Optional<Loc> ArgLoc = Arg.getAs<Loc>()) {
+ SymbolRef Sym = State->getSVal(*ArgLoc).getAsSymbol();
+ if (Sym) {
+ return {Sym};
+ } else {
+ return {};
+ }
+ }
}
}
- return nullptr;
+ return {};
}
void FuchsiaHandleChecker::checkPreCall(const CallEvent &Call,
@@ -273,30 +311,31 @@ void FuchsiaHandleChecker::checkPreCall(const CallEvent &Call,
if (Arg >= FuncDecl->getNumParams())
break;
const ParmVarDecl *PVD = FuncDecl->getParamDecl(Arg);
- SymbolRef Handle =
- getFuchsiaHandleSymbol(PVD->getType(), Call.getArgSVal(Arg), State);
- if (!Handle)
- continue;
+ SmallVector<SymbolRef, 1024> Handles =
+ getFuchsiaHandleSymbols(PVD->getType(), Call.getArgSVal(Arg), State);
// Handled in checkPostCall.
if (hasFuchsiaAttr<ReleaseHandleAttr>(PVD) ||
hasFuchsiaAttr<AcquireHandleAttr>(PVD))
continue;
- const HandleState *HState = State->get<HStateMap>(Handle);
- if (!HState || HState->isEscaped())
- continue;
+ for (SymbolRef Handle : Handles) {
+ const HandleState *HState = State->get<HStateMap>(Handle);
+ if (!HState || HState->isEscaped())
+ continue;
- if (hasFuchsiaAttr<UseHandleAttr>(PVD) || PVD->getType()->isIntegerType()) {
- if (HState->isReleased()) {
- reportUseAfterFree(Handle, Call.getArgSourceRange(Arg), C);
- return;
+ if (hasFuchsiaAttr<UseHandleAttr>(PVD) ||
+ PVD->getType()->isIntegerType()) {
+ if (HState->isReleased()) {
+ reportUseAfterFree(Handle, Call.getArgSourceRange(Arg), C);
+ return;
+ }
+ }
+ if (!hasFuchsiaAttr<UseHandleAttr>(PVD) &&
+ PVD->getType()->isIntegerType()) {
+ // Working around integer by-value escapes.
+ State = State->set<HStateMap>(Handle, HandleState::getEscaped());
}
- }
- if (!hasFuchsiaAttr<UseHandleAttr>(PVD) &&
- PVD->getType()->isIntegerType()) {
- // Working around integer by-value escapes.
- State = State->set<HStateMap>(Handle, HandleState::getEscaped());
}
}
C.addTransition(State);
@@ -339,63 +378,63 @@ void FuchsiaHandleChecker::checkPostCall(const CallEvent &Call,
break;
const ParmVarDecl *PVD = FuncDecl->getParamDecl(Arg);
unsigned ParamDiagIdx = PVD->getFunctionScopeIndex() + 1;
- SymbolRef Handle =
- getFuchsiaHandleSymbol(PVD->getType(), Call.getArgSVal(Arg), State);
- if (!Handle)
- continue;
+ SmallVector<SymbolRef, 1024> Handles =
+ getFuchsiaHandleSymbols(PVD->getType(), Call.getArgSVal(Arg), State);
- const HandleState *HState = State->get<HStateMap>(Handle);
- if (HState && HState->isEscaped())
- continue;
- if (hasFuchsiaAttr<ReleaseHandleAttr>(PVD)) {
- if (HState && HState->isReleased()) {
- reportDoubleRelease(Handle, Call.getArgSourceRange(Arg), C);
- return;
- } else {
+ for (SymbolRef Handle : Handles) {
+ const HandleState *HState = State->get<HStateMap>(Handle);
+ if (HState && HState->isEscaped())
+ continue;
+ if (hasFuchsiaAttr<ReleaseHandleAttr>(PVD)) {
+ if (HState && HState->isReleased()) {
+ reportDoubleRelease(Handle, Call.getArgSourceRange(Arg), C);
+ return;
+ } else {
+ Notes.push_back([Handle, ParamDiagIdx](BugReport &BR) -> std::string {
+ auto *PathBR = static_cast<PathSensitiveBugReport *>(&BR);
+ if (auto IsInteresting = PathBR->getInterestingnessKind(Handle)) {
+ std::string SBuf;
+ llvm::raw_string_ostream OS(SBuf);
+ OS << "Handle released through " << ParamDiagIdx
+ << llvm::getOrdinalSuffix(ParamDiagIdx) << " parameter";
+ return OS.str();
+ } else
+ return "";
+ });
+ State = State->set<HStateMap>(Handle, HandleState::getReleased());
+ }
+ } else if (hasFuchsiaAttr<AcquireHandleAttr>(PVD)) {
Notes.push_back([Handle, ParamDiagIdx](BugReport &BR) -> std::string {
auto *PathBR = static_cast<PathSensitiveBugReport *>(&BR);
if (auto IsInteresting = PathBR->getInterestingnessKind(Handle)) {
std::string SBuf;
llvm::raw_string_ostream OS(SBuf);
- OS << "Handle released through " << ParamDiagIdx
+ OS << "Handle allocated through " << ParamDiagIdx
<< llvm::getOrdinalSuffix(ParamDiagIdx) << " parameter";
return OS.str();
} else
return "";
});
- State = State->set<HStateMap>(Handle, HandleState::getReleased());
+ State = State->set<HStateMap>(
+ Handle, HandleState::getMaybeAllocated(ResultSymbol));
}
- } else if (hasFuchsiaAttr<AcquireHandleAttr>(PVD)) {
- Notes.push_back([Handle, ParamDiagIdx](BugReport &BR) -> std::string {
- auto *PathBR = static_cast<PathSensitiveBugReport *>(&BR);
- if (auto IsInteresting = PathBR->getInterestingnessKind(Handle)) {
- std::string SBuf;
- llvm::raw_string_ostream OS(SBuf);
- OS << "Handle allocated through " << ParamDiagIdx
- << llvm::getOrdinalSuffix(ParamDiagIdx) << " parameter";
- return OS.str();
- } else
- return "";
- });
- State = State->set<HStateMap>(
- Handle, HandleState::getMaybeAllocated(ResultSymbol));
}
}
const NoteTag *T = nullptr;
if (!Notes.empty()) {
T = C.getNoteTag([this, Notes{std::move(Notes)}](
PathSensitiveBugReport &BR) -> std::string {
- if (&BR.getBugType() != &UseAfterReleaseBugType &&
- &BR.getBugType() != &LeakBugType &&
- &BR.getBugType() != &DoubleReleaseBugType)
- return "";
- for (auto &Note : Notes) {
- std::string Text = Note(BR);
- if (!Text.empty())
- return Text;
- }
- return "";
- });
+ if (&BR.getBugType() != &UseAfterReleaseBugType &&
+ &BR.getBugType() != &LeakBugType &&
+ &BR.getBugType() != &DoubleReleaseBugType)
+ return "";
+ for (auto &Note : Notes) {
+ std::string Text = Note(BR);
+ if (!Text.empty())
+ return Text;
+ }
+ return "";
+ });
}
C.addTransition(State, T);
}
@@ -481,13 +520,14 @@ ProgramStateRef FuchsiaHandleChecker::checkPointerEscape(
if (Arg >= FuncDecl->getNumParams())
break;
const ParmVarDecl *PVD = FuncDecl->getParamDecl(Arg);
- SymbolRef Handle =
- getFuchsiaHandleSymbol(PVD->getType(), Call->getArgSVal(Arg), State);
- if (!Handle)
- continue;
- if (hasFuchsiaAttr<UseHandleAttr>(PVD) ||
- hasFuchsiaAttr<ReleaseHandleAttr>(PVD))
- UnEscaped.insert(Handle);
+ SmallVector<SymbolRef, 1024> Handles =
+ getFuchsiaHandleSymbols(PVD->getType(), Call->getArgSVal(Arg), State);
+ for (SymbolRef Handle : Handles) {
+ if (hasFuchsiaAttr<UseHandleAttr>(PVD) ||
+ hasFuchsiaAttr<ReleaseHandleAttr>(PVD)) {
+ UnEscaped.insert(Handle);
+ }
+ }
}
}
diff --git a/clang/test/Analysis/fuchsia_handle.cpp b/clang/test/Analysis/fuchsia_handle.cpp
index d104b13c77ab..911ab7adaaa9 100644
--- a/clang/test/Analysis/fuchsia_handle.cpp
+++ b/clang/test/Analysis/fuchsia_handle.cpp
@@ -9,9 +9,9 @@ typedef unsigned int uint32_t;
#define ZX_HANDLE_INVALID 0
#if defined(__clang__)
-#define ZX_HANDLE_ACQUIRE __attribute__((acquire_handle("Fuchsia")))
-#define ZX_HANDLE_RELEASE __attribute__((release_handle("Fuchsia")))
-#define ZX_HANDLE_USE __attribute__((use_handle("Fuchsia")))
+#define ZX_HANDLE_ACQUIRE __attribute__((acquire_handle("Fuchsia")))
+#define ZX_HANDLE_RELEASE __attribute__((release_handle("Fuchsia")))
+#define ZX_HANDLE_USE __attribute__((use_handle("Fuchsia")))
#else
#define ZX_HANDLE_ACQUIRE
#define ZX_HANDLE_RELEASE
@@ -31,7 +31,7 @@ zx_handle_t return_handle();
void escape1(zx_handle_t *in);
void escape2(zx_handle_t in);
-void (*escape3)(zx_handle_t) = escape2;
+void (*escape3)(zx_handle_t) = escape2;
void use1(const zx_handle_t *in ZX_HANDLE_USE);
void use2(zx_handle_t in ZX_HANDLE_USE);
@@ -82,8 +82,8 @@ void handleDieBeforeErrorSymbol02() {
// expected-note-re at -3 {{Handle allocated through {{(2nd|3rd)}} parameter}}
if (status == 0) { // expected-note {{Assuming 'status' is equal to 0}}
// expected-note at -1 {{Taking true branch}}
- return; // expected-warning {{Potential leak of handle}}
- // expected-note at -1 {{Potential leak of handle}}
+ return; // expected-warning {{Potential leak of handle}}
+ // expected-note at -1 {{Potential leak of handle}}
}
__builtin_trap();
}
@@ -138,7 +138,7 @@ void checkNoLeak06() {
return;
zx_handle_close(sa);
zx_handle_close(sb);
-}
+}
void checkLeak01(int tag) {
zx_handle_t sa, sb;
@@ -158,7 +158,7 @@ void checkLeakFromReturn01(int tag) {
zx_handle_t sa = return_handle(); // expected-note {{Function 'return_handle' returns an open handle}}
(void)sa;
} // expected-note {{Potential leak of handle}}
- // expected-warning at -1 {{Potential leak of handle}}
+// expected-warning at -1 {{Potential leak of handle}}
void checkReportLeakOnOnePath(int tag) {
zx_handle_t sa, sb;
@@ -166,26 +166,26 @@ void checkReportLeakOnOnePath(int tag) {
return; // expected-note at -1 {{Assuming the condition is false}}
// expected-note at -2 {{Taking false branch}}
zx_handle_close(sb);
- switch(tag) { // expected-note {{Control jumps to the 'default' case at line}}
- case 0:
- use2(sa);
- return;
- case 1:
- use2(sa);
- return;
- case 2:
- use2(sa);
- return;
- case 3:
- use2(sa);
- return;
- case 4:
- use2(sa);
- return;
- default:
- use2(sa);
- return; // expected-warning {{Potential leak of handle}}
- // expected-note at -1 {{Potential leak of handle}}
+ switch (tag) { // expected-note {{Control jumps to the 'default' case at line}}
+ case 0:
+ use2(sa);
+ return;
+ case 1:
+ use2(sa);
+ return;
+ case 2:
+ use2(sa);
+ return;
+ case 3:
+ use2(sa);
+ return;
+ case 4:
+ use2(sa);
+ return;
+ default:
+ use2(sa);
+ return; // expected-warning {{Potential leak of handle}}
+ // expected-note at -1 {{Potential leak of handle}}
}
}
@@ -193,7 +193,7 @@ void checkDoubleRelease01(int tag) {
zx_handle_t sa, sb;
zx_channel_create(0, &sa, &sb);
// expected-note at -1 {{Handle allocated through 2nd parameter}}
- if (tag) // expected-note {{Assuming 'tag' is not equal to 0}}
+ if (tag) // expected-note {{Assuming 'tag' is not equal to 0}}
zx_handle_close(sa); // expected-note {{Handle released through 1st parameter}}
// expected-note at -2 {{Taking true branch}}
zx_handle_close(sa); // expected-warning {{Releasing a previously released handle}}
@@ -211,12 +211,12 @@ void checkUseAfterFree01(int tag) {
if (tag) {
// expected-note at -1 {{Assuming 'tag' is not equal to 0}}
zx_handle_close(sa); // expected-note {{Handle released through 1st parameter}}
- use1(&sa); // expected-warning {{Using a previously released handle}}
+ use1(&sa); // expected-warning {{Using a previously released handle}}
// expected-note at -1 {{Using a previously released handle}}
}
// expected-note at -6 {{Assuming 'tag' is 0}}
zx_handle_close(sb); // expected-note {{Handle released through 1st parameter}}
- use2(sb); // expected-warning {{Using a previously released handle}}
+ use2(sb); // expected-warning {{Using a previously released handle}}
// expected-note at -1 {{Using a previously released handle}}
}
@@ -229,6 +229,92 @@ void checkMemberOperatorIndices() {
zx_handle_close(sc);
}
+struct HandleStruct {
+ zx_handle_t h;
+};
+
+void close_handle_struct(HandleStruct hs ZX_HANDLE_RELEASE);
+
+void use_handle_struct(HandleStruct hs ZX_HANDLE_USE);
+
+void checkHandleInStructureUseAfterFree() {
+ zx_handle_t sa, sb;
+ zx_channel_create(0, &sa, &sb); // expected-note {{Handle allocated through 3rd parameter}}
+ HandleStruct hs;
+ hs.h = sb;
+ use_handle_struct(hs);
+ close_handle_struct(hs); // expected-note {{Handle released through 1st parameter}}
+ zx_handle_close(sa);
+
+ use2(sb); // expected-warning {{Using a previously released handle}}
+ // expected-note at -1 {{Using a previously released handle}}
+}
+
+void checkHandleInStructureUseAfterFree2() {
+ zx_handle_t sa, sb;
+ zx_channel_create(0, &sa, &sb); // expected-note {{Handle allocated through 3rd parameter}}
+ HandleStruct hs;
+ hs.h = sb;
+ use_handle_struct(hs);
+ zx_handle_close(sb); // expected-note {{Handle released through 1st parameter}}
+ zx_handle_close(sa);
+
+ use_handle_struct(hs); // expected-warning {{Using a previously released handle}}
+ // expected-note at -1 {{Using a previously released handle}}
+}
+
+void checkHandleInStructureLeak() {
+ zx_handle_t sa, sb;
+ zx_channel_create(0, &sa, &sb); // expected-note {{Handle allocated through 3rd parameter}}
+ HandleStruct hs;
+ hs.h = sb;
+ zx_handle_close(sa); // expected-warning {{Potential leak of handle}}
+ // expected-note at -1 {{Potential leak of handle}}
+}
+
+struct HandlePtrStruct {
+ zx_handle_t *h;
+};
+
+void close_handle_struct(HandlePtrStruct hs ZX_HANDLE_RELEASE);
+
+void use_handle_struct(HandlePtrStruct hs ZX_HANDLE_USE);
+
+void checkHandlePtrInStructureUseAfterFree() {
+ zx_handle_t sa, sb;
+ zx_channel_create(0, &sa, &sb);
+ HandlePtrStruct hs;
+ hs.h = &sb;
+ use_handle_struct(hs);
+ close_handle_struct(hs); // expected-note {{Handle released through 1st parameter}}
+ zx_handle_close(sa);
+
+ use2(sb); // expected-warning {{Using a previously released handle}}
+ // expected-note at -1 {{Using a previously released handle}}
+}
+
+void checkHandlePtrInStructureUseAfterFree2() {
+ zx_handle_t sa, sb;
+ zx_channel_create(0, &sa, &sb);
+ HandlePtrStruct hs;
+ hs.h = &sb;
+ use_handle_struct(hs);
+ zx_handle_close(sb); // expected-note {{Handle released through 1st parameter}}
+ zx_handle_close(sa);
+
+ use_handle_struct(hs); // expected-warning {{Using a previously released handle}}
+ // expected-note at -1 {{Using a previously released handle}}
+}
+
+void checkHandlePtrInStructureLeak() {
+ zx_handle_t sa, sb;
+ zx_channel_create(0, &sa, &sb); // expected-note {{Handle allocated through 3rd parameter}}
+ HandlePtrStruct hs;
+ hs.h = &sb;
+ zx_handle_close(sa); // expected-warning {{Potential leak of handle}}
+ // expected-note at -1 {{Potential leak of handle}}
+}
+
// RAII
template <typename T>
@@ -239,6 +325,7 @@ struct HandleWrapper {
zx_handle_close(handle);
}
T *get_handle_address() { return &handle; }
+
private:
T handle;
};
@@ -259,6 +346,7 @@ struct HandleWrapperUnkonwDtor {
zx_handle_close(handle);
}
T *get_handle_address() { return &handle; }
+
private:
T handle;
};
@@ -270,7 +358,7 @@ void doNotWarnOnUnknownDtor() {
return;
zx_handle_close(sb);
}
-
+
// Various escaping scenarios
zx_handle_t *get_handle_address();
More information about the cfe-commits
mailing list