[PATCH] D67932: [analyzer] Fix accidentally skipping the call during inlined defensive check suppression.

[Artem Dergachev via Phabricator via cfe-commits]

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>(


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D67932.228552.patch
Type: text/x-patch
Size: 3301 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191109/cbf7e3b8/attachment-0001.bin>