aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--include/clang/StaticAnalyzer/Core/AnalyzerOptions.h15
-rw-r--r--lib/StaticAnalyzer/Core/AnalyzerOptions.cpp8
-rw-r--r--lib/StaticAnalyzer/Core/BugReporterVisitors.cpp23
-rw-r--r--test/Analysis/inline-plist.c4
-rw-r--r--test/Analysis/inlining/false-positive-suppression.c95
-rw-r--r--test/Analysis/inlining/path-notes.c4
-rw-r--r--test/Analysis/inlining/path-notes.m4
7 files changed, 140 insertions, 13 deletions
diff --git a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index 924bbf9d1c..7657736c2b 100644
--- a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -184,7 +184,10 @@ private:
// Cache of the "ipa-always-inline-size" setting.
// \sa getAlwaysInlineSize
llvm::Optional<unsigned> AlwaysInlineSize;
-
+
+ /// \sa shouldPruneNullReturnPaths
+ llvm::Optional<bool> PruneNullReturnPaths;
+
/// Interprets an option's string value as a boolean.
///
/// Accepts the strings "true" and "false".
@@ -226,6 +229,16 @@ public:
/// accepts the values "true" and "false".
bool mayInlineTemplateFunctions() const;
+ /// Returns whether or not paths that go through null returns should be
+ /// suppressed.
+ ///
+ /// This is a heuristic for avoiding bug reports with paths that go through
+ /// inlined functions that are more defensive than their callers.
+ ///
+ /// This is controlled by the 'suppress-null-return-paths' config option,
+ /// which accepts the values "true" and "false".
+ bool shouldPruneNullReturnPaths() const;
+
// Returns the size of the functions (in basic blocks), which should be
// considered to be small enough to always inline.
//
diff --git a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
index 04eb0ad97b..1ffd105766 100644
--- a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -90,6 +90,14 @@ bool AnalyzerOptions::mayInlineObjCMethod() const {
return *ObjCInliningMode;
}
+bool AnalyzerOptions::shouldPruneNullReturnPaths() const {
+ if (!PruneNullReturnPaths.hasValue())
+ const_cast<llvm::Optional<bool> &>(PruneNullReturnPaths) =
+ getBooleanOption("suppress-null-return-paths", /*Default=*/true);
+
+ return *PruneNullReturnPaths;
+}
+
int AnalyzerOptions::getOptionAsInteger(StringRef Name, int DefaultVal) const {
std::string OptStr = Config.lookup(Name);
if (OptStr.empty())
diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 445500ba31..00d7d3becf 100644
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -140,9 +140,13 @@ public:
ReturnVisitor(const StackFrameContext *Frame)
: StackFrame(Frame), Satisfied(false) {}
- virtual void Profile(llvm::FoldingSetNodeID &ID) const {
+ static void *getTag() {
static int Tag = 0;
- ID.AddPointer(&Tag);
+ return static_cast<void *>(&Tag);
+ }
+
+ virtual void Profile(llvm::FoldingSetNodeID &ID) const {
+ ID.AddPointer(ReturnVisitor::getTag());
ID.AddPointer(StackFrame);
}
@@ -215,10 +219,6 @@ public:
// Don't print any more notes after this one.
Satisfied = true;
- // Build an appropriate message based on the return value.
- SmallString<64> Msg;
- llvm::raw_svector_ostream Out(Msg);
-
const Expr *RetE = Ret->getRetValue();
assert(RetE && "Tracking a return value for a void function");
RetE = RetE->IgnoreParenCasts();
@@ -234,7 +234,18 @@ public:
// If we're returning 0, we should track where that 0 came from.
bugreporter::trackNullOrUndefValue(N, RetE, BR);
+ // Build an appropriate message based on the return value.
+ SmallString<64> Msg;
+ llvm::raw_svector_ostream Out(Msg);
+
if (isa<Loc>(V)) {
+ // If we are pruning null-return paths as unlikely error paths, mark the
+ // report invalid. We still want to emit a path note, however, in case
+ // the report is resurrected as valid later on.
+ ExprEngine &Eng = BRC.getBugReporter().getEngine();
+ if (Eng.getAnalysisManager().options.shouldPruneNullReturnPaths())
+ BR.markInvalid(ReturnVisitor::getTag(), StackFrame);
+
if (RetE->getType()->isObjCObjectPointerType())
Out << "Returning nil";
else
diff --git a/test/Analysis/inline-plist.c b/test/Analysis/inline-plist.c
index 6c8daf5d5e..1ccef6c2ca 100644
--- a/test/Analysis/inline-plist.c
+++ b/test/Analysis/inline-plist.c
@@ -1,5 +1,5 @@
-// RUN: %clang --analyze %s -fblocks -Xanalyzer -analyzer-output=text -Xclang -verify %s
-// RUN: %clang --analyze %s -fblocks -o %t
+// RUN: %clang --analyze %s -fblocks -Xanalyzer -analyzer-output=text -Xanalyzer -analyzer-config -Xanalyzer suppress-null-return-paths=false -Xclang -verify %s
+// RUN: %clang --analyze %s -fblocks -Xanalyzer -analyzer-config -Xanalyzer suppress-null-return-paths=false -o %t
// RUN: FileCheck -input-file %t %s
// <rdar://problem/10967815>
diff --git a/test/Analysis/inlining/false-positive-suppression.c b/test/Analysis/inlining/false-positive-suppression.c
new file mode 100644
index 0000000000..be485e0c1c
--- /dev/null
+++ b/test/Analysis/inlining/false-positive-suppression.c
@@ -0,0 +1,95 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-config suppress-null-return-paths=false -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -verify -DSUPPRESSED %s
+
+int opaquePropertyCheck(void *object);
+int coin();
+
+int *dynCastToInt(void *ptr) {
+ if (opaquePropertyCheck(ptr))
+ return (int *)ptr;
+ return 0;
+}
+
+int *dynCastOrNull(void *ptr) {
+ if (!ptr)
+ return 0;
+ if (opaquePropertyCheck(ptr))
+ return (int *)ptr;
+ return 0;
+}
+
+
+void testDynCast(void *p) {
+ int *casted = dynCastToInt(p);
+ *casted = 1;
+#ifndef SUPPRESSED
+ // expected-warning@-2 {{Dereference of null pointer}}
+#endif
+}
+
+void testDynCastOrNull(void *p) {
+ int *casted = dynCastOrNull(p);
+ *casted = 1;
+#ifndef SUPPRESSED
+ // expected-warning@-2 {{Dereference of null pointer}}
+#endif
+}
+
+
+void testBranch(void *p) {
+ int *casted;
+
+ // Although the report will be suppressed on one branch, it should still be
+ // valid on the other.
+ if (coin()) {
+ casted = dynCastToInt(p);
+ } else {
+ if (p)
+ return;
+ casted = (int *)p;
+ }
+
+ *casted = 1; // expected-warning {{Dereference of null pointer}}
+}
+
+void testBranchReversed(void *p) {
+ int *casted;
+
+ // Although the report will be suppressed on one branch, it should still be
+ // valid on the other.
+ if (coin()) {
+ if (p)
+ return;
+ casted = (int *)p;
+ } else {
+ casted = dynCastToInt(p);
+ }
+
+ *casted = 1; // expected-warning {{Dereference of null pointer}}
+}
+
+
+// ---------------------------------------
+// FALSE NEGATIVES (over-suppression)
+// ---------------------------------------
+
+void testDynCastOrNullOfNull() {
+ // In this case we have a known value for the argument, and thus the path
+ // through the function doesn't ever split.
+ int *casted = dynCastOrNull(0);
+ *casted = 1;
+#ifndef SUPPRESSED
+ // expected-warning@-2 {{Dereference of null pointer}}
+#endif
+}
+
+void testDynCastOfNull() {
+ // In this case all paths out of the function return 0, but they are all
+ // dominated by a branch whose condition we don't know!
+ int *casted = dynCastToInt(0);
+ *casted = 1;
+#ifndef SUPPRESSED
+ // expected-warning@-2 {{Dereference of null pointer}}
+#endif
+}
+
diff --git a/test/Analysis/inlining/path-notes.c b/test/Analysis/inlining/path-notes.c
index bd44f0c895..763614d9aa 100644
--- a/test/Analysis/inlining/path-notes.c
+++ b/test/Analysis/inlining/path-notes.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=text -verify %s
-// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=plist-multi-file %s -o %t.plist
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=text -analyzer-config suppress-null-return-paths=false -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=plist-multi-file -analyzer-config suppress-null-return-paths=false %s -o %t.plist
// RUN: FileCheck --input-file=%t.plist %s
void zero(int **p) {
diff --git a/test/Analysis/inlining/path-notes.m b/test/Analysis/inlining/path-notes.m
index 1b67051bfa..b15b869682 100644
--- a/test/Analysis/inlining/path-notes.m
+++ b/test/Analysis/inlining/path-notes.m
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=text -verify %s
-// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=plist-multi-file %s -o %t.plist
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=text -analyzer-config suppress-null-return-paths=false -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=plist-multi-file -analyzer-config suppress-null-return-paths=false %s -o %t.plist
// RUN: FileCheck --input-file=%t.plist %s
@interface Test
generated by cgit v1.2.3-18-g5258 (git 2.32.0) at 2025-08-03 22:38:43 +0000