[PATCH] D52848: [analyzer] Do not crash if the assumption added in TrustNonNullChecker is enough to make the state unfeasible

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC343735: [analyzer] Do not crash if the assumption added in 
TrustNonNullChecker is… (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52848?vs=168178&id=168191#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52848

Files:
  lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
  test/Analysis/trustnonnullchecker_test.m


Index: test/Analysis/trustnonnullchecker_test.m
===================================================================
--- test/Analysis/trustnonnullchecker_test.m
+++ test/Analysis/trustnonnullchecker_test.m
@@ -170,3 +170,25 @@
   if (k) {}
   return k; // no-warning
 }
+
+// Check that we don't crash when the added assumption is enough
+// to make the state unfeasible.
+@class DummyClass;
+@interface DictionarySubclass : NSDictionary {
+  DummyClass *g;
+  DictionarySubclass *d;
+}
+@end
+@implementation DictionarySubclass
+- (id) objectForKey:(id)e {
+  if (e) {}
+  return d;
+}
+- (void) coder {
+  for (id e in g) {
+    id f = [self objectForKey:e];
+    if (f)
+      (void)e;
+  }
+}
+@end
Index: lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
+++ lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
@@ -212,20 +212,26 @@
   /// the negation of \p Antecedent.
   /// Checks NonNullImplicationMap and assumes \p Antecedent otherwise.
   ProgramStateRef addImplication(SymbolRef Antecedent,
-                                 ProgramStateRef State,
+                                 ProgramStateRef InputState,
                                  bool Negated) const {
-    SValBuilder &SVB = State->getStateManager().getSValBuilder();
+    if (!InputState)
+      return nullptr;
+    SValBuilder &SVB = InputState->getStateManager().getSValBuilder();
     const SymbolRef *Consequent =
-        Negated ? State->get<NonNullImplicationMap>(Antecedent)
-                : State->get<NullImplicationMap>(Antecedent);
+        Negated ? InputState->get<NonNullImplicationMap>(Antecedent)
+                : InputState->get<NullImplicationMap>(Antecedent);
     if (!Consequent)
-      return State;
+      return InputState;
 
     SVal AntecedentV = SVB.makeSymbolVal(Antecedent);
-    if ((Negated && State->isNonNull(AntecedentV).isConstrainedTrue())
-        || (!Negated && State->isNull(AntecedentV).isConstrainedTrue())) {
+    ProgramStateRef State = InputState;
+
+    if ((Negated && InputState->isNonNull(AntecedentV).isConstrainedTrue())
+        || (!Negated && InputState->isNull(AntecedentV).isConstrainedTrue())) {
       SVal ConsequentS = SVB.makeSymbolVal(*Consequent);
-      State = State->assume(ConsequentS.castAs<DefinedSVal>(), Negated);
+      State = InputState->assume(ConsequentS.castAs<DefinedSVal>(), Negated);
+      if (!State)
+        return nullptr;
 
       // Drop implications from the map.
       if (Negated) {



Index: test/Analysis/trustnonnullchecker_test.m
===================================================================
--- test/Analysis/trustnonnullchecker_test.m
+++ test/Analysis/trustnonnullchecker_test.m
@@ -170,3 +170,25 @@
   if (k) {}
   return k; // no-warning
 }
+
+// Check that we don't crash when the added assumption is enough
+// to make the state unfeasible.
+@class DummyClass;
+@interface DictionarySubclass : NSDictionary {
+  DummyClass *g;
+  DictionarySubclass *d;
+}
+@end
+@implementation DictionarySubclass
+- (id) objectForKey:(id)e {
+  if (e) {}
+  return d;
+}
+- (void) coder {
+  for (id e in g) {
+    id f = [self objectForKey:e];
+    if (f)
+      (void)e;
+  }
+}
+@end
Index: lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
+++ lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
@@ -212,20 +212,26 @@
   /// the negation of \p Antecedent.
   /// Checks NonNullImplicationMap and assumes \p Antecedent otherwise.
   ProgramStateRef addImplication(SymbolRef Antecedent,
-                                 ProgramStateRef State,
+                                 ProgramStateRef InputState,
                                  bool Negated) const {
-    SValBuilder &SVB = State->getStateManager().getSValBuilder();
+    if (!InputState)
+      return nullptr;
+    SValBuilder &SVB = InputState->getStateManager().getSValBuilder();
     const SymbolRef *Consequent =
-        Negated ? State->get<NonNullImplicationMap>(Antecedent)
-                : State->get<NullImplicationMap>(Antecedent);
+        Negated ? InputState->get<NonNullImplicationMap>(Antecedent)
+                : InputState->get<NullImplicationMap>(Antecedent);
     if (!Consequent)
-      return State;
+      return InputState;
 
     SVal AntecedentV = SVB.makeSymbolVal(Antecedent);
-    if ((Negated && State->isNonNull(AntecedentV).isConstrainedTrue())
-        || (!Negated && State->isNull(AntecedentV).isConstrainedTrue())) {
+    ProgramStateRef State = InputState;
+
+    if ((Negated && InputState->isNonNull(AntecedentV).isConstrainedTrue())
+        || (!Negated && InputState->isNull(AntecedentV).isConstrainedTrue())) {
       SVal ConsequentS = SVB.makeSymbolVal(*Consequent);
-      State = State->assume(ConsequentS.castAs<DefinedSVal>(), Negated);
+      State = InputState->assume(ConsequentS.castAs<DefinedSVal>(), Negated);
+      if (!State)
+        return nullptr;
 
       // Drop implications from the map.
       if (Negated) {

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits