This revision was automatically updated to reflect the committed changes.
Closed by commit rGe4da37e8a0a3: [analyzer] Fix skipping the call during
inlined defensive check suppression. (authored by dergachev.a).
Changed prior to commit:
https://reviews.llvm.org/D67932?vs=221408&id=228552#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67932/new/
https://reviews.llvm.org/D67932
Files:
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
clang/test/Analysis/NSContainers.m
Index: clang/test/Analysis/NSContainers.m
===================================================================
--- clang/test/Analysis/NSContainers.m
+++ clang/test/Analysis/NSContainers.m
@@ -2,6 +2,8 @@
void clang_analyzer_eval(int);
+#define nil ((id)0)
+
typedef unsigned long NSUInteger;
typedef signed char BOOL;
typedef struct _NSZone NSZone;
@@ -310,3 +312,14 @@
// here either.
[subviews addObject:view]; // no-warning
}
+
+NSString *getStringFromString(NSString *string) {
+ if (!string)
+ return nil;
+ return @"New String";
+}
+void testInlinedDefensiveCheck(NSMutableDictionary *dict, id obj) {
+ // The check in getStringFromString() is not a good indication
+ // that 'obj' can be nil in this context.
+ dict[obj] = getStringFromString(obj); // no-warning
+}
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1606,9 +1606,6 @@
AnalyzerOptions &Options = N->getState()->getAnalysisManager().options;
if (!Options.ShouldSuppressInlinedDefensiveChecks)
IsSatisfied = true;
-
- assert(N->getState()->isNull(V).isConstrainedTrue() &&
- "The visitor only tracks the cases where V is constrained to 0");
}
void SuppressInlineDefensiveChecksVisitor::Profile(
@@ -1639,13 +1636,12 @@
// Check if in the previous state it was feasible for this value
// to *not* be null.
- if (!Pred->getState()->isNull(V).isConstrainedTrue()) {
+ if (!Pred->getState()->isNull(V).isConstrainedTrue() &&
+ Succ->getState()->isNull(V).isConstrainedTrue()) {
IsSatisfied = true;
- assert(Succ->getState()->isNull(V).isConstrainedTrue());
-
// Check if this is inlined defensive checks.
- const LocationContext *CurLC =Succ->getLocationContext();
+ const LocationContext *CurLC = Succ->getLocationContext();
const LocationContext *ReportLC = BR.getErrorNode()->getLocationContext();
if (CurLC != ReportLC && !CurLC->isParentOf(ReportLC)) {
BR.markInvalid("Suppress IDC", CurLC);
@@ -2012,11 +2008,16 @@
// Add visitor, which will suppress inline defensive checks.
if (auto DV = V.getAs<DefinedSVal>())
- if (!DV->isZeroConstant() && LVState->isNull(*DV).isConstrainedTrue()
&&
- EnableNullFPSuppression)
+ if (!DV->isZeroConstant() && EnableNullFPSuppression) {
+ // Note that LVNode may be too late (i.e., too far from the
InputNode)
+ // because the lvalue may have been computed before the inlined call
+ // was evaluated. InputNode may as well be too early here, because
+ // the symbol is already dead; this, however, is fine because we can
+ // still find the node in which it collapsed to null previously.
report.addVisitor(
- std::make_unique<SuppressInlineDefensiveChecksVisitor>(*DV,
- LVNode));
+ std::make_unique<SuppressInlineDefensiveChecksVisitor>(
+ *DV, InputNode));
+ }
if (auto KV = V.getAs<KnownSVal>())
report.addVisitor(std::make_unique<FindLastStoreBRVisitor>(
Index: clang/test/Analysis/NSContainers.m
===================================================================
--- clang/test/Analysis/NSContainers.m
+++ clang/test/Analysis/NSContainers.m
@@ -2,6 +2,8 @@
void clang_analyzer_eval(int);
+#define nil ((id)0)
+
typedef unsigned long NSUInteger;
typedef signed char BOOL;
typedef struct _NSZone NSZone;
@@ -310,3 +312,14 @@
// here either.
[subviews addObject:view]; // no-warning
}
+
+NSString *getStringFromString(NSString *string) {
+ if (!string)
+ return nil;
+ return @"New String";
+}
+void testInlinedDefensiveCheck(NSMutableDictionary *dict, id obj) {
+ // The check in getStringFromString() is not a good indication
+ // that 'obj' can be nil in this context.
+ dict[obj] = getStringFromString(obj); // no-warning
+}
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1606,9 +1606,6 @@
AnalyzerOptions &Options = N->getState()->getAnalysisManager().options;
if (!Options.ShouldSuppressInlinedDefensiveChecks)
IsSatisfied = true;
-
- assert(N->getState()->isNull(V).isConstrainedTrue() &&
- "The visitor only tracks the cases where V is constrained to 0");
}
void SuppressInlineDefensiveChecksVisitor::Profile(
@@ -1639,13 +1636,12 @@
// Check if in the previous state it was feasible for this value
// to *not* be null.
- if (!Pred->getState()->isNull(V).isConstrainedTrue()) {
+ if (!Pred->getState()->isNull(V).isConstrainedTrue() &&
+ Succ->getState()->isNull(V).isConstrainedTrue()) {
IsSatisfied = true;
- assert(Succ->getState()->isNull(V).isConstrainedTrue());
-
// Check if this is inlined defensive checks.
- const LocationContext *CurLC =Succ->getLocationContext();
+ const LocationContext *CurLC = Succ->getLocationContext();
const LocationContext *ReportLC = BR.getErrorNode()->getLocationContext();
if (CurLC != ReportLC && !CurLC->isParentOf(ReportLC)) {
BR.markInvalid("Suppress IDC", CurLC);
@@ -2012,11 +2008,16 @@
// Add visitor, which will suppress inline defensive checks.
if (auto DV = V.getAs<DefinedSVal>())
- if (!DV->isZeroConstant() && LVState->isNull(*DV).isConstrainedTrue() &&
- EnableNullFPSuppression)
+ if (!DV->isZeroConstant() && EnableNullFPSuppression) {
+ // Note that LVNode may be too late (i.e., too far from the InputNode)
+ // because the lvalue may have been computed before the inlined call
+ // was evaluated. InputNode may as well be too early here, because
+ // the symbol is already dead; this, however, is fine because we can
+ // still find the node in which it collapsed to null previously.
report.addVisitor(
- std::make_unique<SuppressInlineDefensiveChecksVisitor>(*DV,
- LVNode));
+ std::make_unique<SuppressInlineDefensiveChecksVisitor>(
+ *DV, InputNode));
+ }
if (auto KV = V.getAs<KnownSVal>())
report.addVisitor(std::make_unique<FindLastStoreBRVisitor>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
