xazax.hun created this revision.
xazax.hun added reviewers: aaron.ballman, NoQ, haowei.
xazax.hun added a project: clang.
Herald added subscribers: Charusso, gamesh411, dkrupp, donat.nagy, Szelethus,
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware.
While the static analyzer is doing a great job modeling functions most of the
time, sometimes we have global dynamic invariants that might be infeasible to
reason about statically (or in some cases just to complex to worth to implement
it).
It would be great to have a way to tell the checkers that certain functions are
not worth modeling. In case of fuchsia.HandleCheck, we could achieve the same
by simply removing all the annotations. This is not always desired though. The
annotations are useful on their own, they document the ownership semantics of
the handles. So instead of removing the annotations for these functions, we
think it might be better to introduce yet another annotation for this use case.
What do you think? Is this reasonable or do you have any alternative ideas?
For the curious the syscall I have problem with in Fuchsia is the
`zx_channel_read`:
https://fuchsia.dev/fuchsia-src/reference/syscalls/channel_read.md
The problem is mainly with the `handles` parameter. We might not receive
handles at all. And we might know that in advance that we will not receive
handles, so we do not need to check the `actual_handles`. So assuming `handles`
contains open handles will end up producing spurious handle leak errors.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D72018
Files:
clang/include/clang/Basic/Attr.td
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
clang/test/Analysis/fuchsia_handle.cpp
clang/test/Misc/pragma-attribute-supported-attributes-list.test
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===================================================================
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -15,6 +15,7 @@
// CHECK-NEXT: AllocSize (SubjectMatchRule_function)
// CHECK-NEXT: AlwaysDestroy (SubjectMatchRule_variable)
// CHECK-NEXT: AlwaysInline (SubjectMatchRule_function)
+// CHECK-NEXT: AnalyzerCheckerIgnore (SubjectMatchRule_function)
// CHECK-NEXT: Annotate ()
// CHECK-NEXT: AnyX86NoCfCheck (SubjectMatchRule_hasType_functionType)
// CHECK-NEXT: ArcWeakrefUnavailable (SubjectMatchRule_objc_interface)
Index: clang/test/Analysis/fuchsia_handle.cpp
===================================================================
--- clang/test/Analysis/fuchsia_handle.cpp
+++ clang/test/Analysis/fuchsia_handle.cpp
@@ -12,10 +12,12 @@
#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_IGNORE __attribute__((analyzer_checker_ignore("fuchsia.HandleChecker")))
#else
#define ZX_HANDLE_ACQUIRE
#define ZX_HANDLE_RELEASE
#define ZX_HANDLE_USE
+#define ZX_HANDLE_IGNORE
#endif
zx_status_t zx_channel_create(
@@ -23,6 +25,12 @@
zx_handle_t *out0 ZX_HANDLE_ACQUIRE,
zx_handle_t *out1 ZX_HANDLE_ACQUIRE);
+ZX_HANDLE_IGNORE
+zx_status_t zx_channel_create_complicated(
+ uint32_t options,
+ zx_handle_t *out0 ZX_HANDLE_ACQUIRE,
+ zx_handle_t *out1 ZX_HANDLE_ACQUIRE);
+
zx_status_t zx_handle_close(
zx_handle_t handle ZX_HANDLE_RELEASE);
@@ -327,3 +335,11 @@
return;
zx_handle_close(h2);
} // *h should be escaped here. Right?
+
+// Ignored functions
+
+void ignore_function() {
+ zx_handle_t sa, sb;
+ if (zx_channel_create_complicated(0, &sa, &sb))
+ return;
+}
Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -302,6 +302,12 @@
if (!FuncDecl)
return;
+ if (const auto *Attr = FuncDecl->getAttr<AnalyzerCheckerIgnoreAttr>())
+ if (llvm::any_of(Attr->checkers(), [this](StringRef Checker) {
+ return Checker == getCheckerName();
+ }))
+ return;
+
ProgramStateRef State = C.getState();
std::vector<std::function<std::string(BugReport & BR)>> Notes;
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -2065,6 +2065,25 @@
D->addAttr(::new (S.Context) AnalyzerNoReturnAttr(S.Context, AL));
}
+static void HandleAnalyzerCheckerIgnoreAttr(Sema &S, Decl *D,
+ const ParsedAttr &AL) {
+ if (!checkAttributeAtLeastNumArgs(S, AL, 1))
+ return;
+
+ SmallVector<StringRef, 4> Checkers;
+ for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) {
+ StringRef CheckerName;
+
+ if (!S.checkStringLiteralArgumentAttr(AL, I, CheckerName, nullptr))
+ return;
+
+ Checkers.push_back(CheckerName);
+ }
+
+ D->addAttr(AnalyzerCheckerIgnoreAttr::Create(S.Context, Checkers.data(),
+ Checkers.size()));
+}
+
// PS3 PPU-specific.
static void handleVecReturnAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
/*
@@ -6763,6 +6782,9 @@
case ParsedAttr::AT_AnalyzerNoReturn:
handleAnalyzerNoReturnAttr(S, D, AL);
break;
+ case ParsedAttr::AT_AnalyzerCheckerIgnore:
+ HandleAnalyzerCheckerIgnoreAttr(S, D, AL);
+ break;
case ParsedAttr::AT_TLSModel:
handleTLSModelAttr(S, D, AL);
break;
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -699,6 +699,13 @@
let Documentation = [Undocumented];
}
+def AnalyzerCheckerIgnore : InheritableAttr {
+ let Spellings = [Clang<"analyzer_checker_ignore">];
+ let Args = [VariadicStringArgument<"Checkers">];
+ let Subjects = SubjectList<[Function]>;
+ let Documentation = [Undocumented];
+}
+
def Annotate : InheritableParamAttr {
let Spellings = [Clang<"annotate">];
let Args = [StringArgument<"Annotation">];
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
