[committed] analyzer: fix ICE with -Wanalyzer-null-dereference [PR 93950]

Message ID 20200227020845.30275-1-dmalcolm@redhat.com
State New
Headers show
Series
  • [committed] analyzer: fix ICE with -Wanalyzer-null-dereference [PR 93950]
Related show

Commit Message

David Malcolm Feb. 27, 2020, 2:08 a.m.
PR analyzer/93950 reports an ICE when pruning the path of a
-Wanalyzer-null-dereference diagnostic.

The root cause is a bug in the state-tracking code, in which the
variable of interest is tracked from the callee to a "nullptr" param
at the caller, whereupon we have an INTEGER_CST "variable", and
the attempt to look up its lvalue fails.

This code could use a rewrite; in the meantime this patch extends
the bulletproofing from g:8525d1f5f57b11fe04a97674cc2fc2b7727621d0
for PR analyzer/93544 to all of the various places where var can
be updated, fixing the ICE.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to master as r10-6880-g71b633aaea3aac2d983da7b1b99da8c9a8c80d1a.

gcc/analyzer/ChangeLog:
	PR analyzer/93950
	* diagnostic-manager.cc
	(diagnostic_manager::prune_for_sm_diagnostic): Assert that var is
	either NULL or not a constant.  When updating var, bulletproof
	against constant values.

gcc/testsuite/ChangeLog:
	PR analyzer/93950
	* g++.dg/analyzer/pr93950.C: New test.
---
 gcc/analyzer/diagnostic-manager.cc      | 16 ++++++++++++++
 gcc/testsuite/g++.dg/analyzer/pr93950.C | 28 +++++++++++++++++++++++++
 2 files changed, 44 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/analyzer/pr93950.C

-- 
2.21.0

Patch

diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index 78c5890054f..b8e59334374 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -1105,6 +1105,7 @@  diagnostic_manager::prune_for_sm_diagnostic (checker_path *path,
 	  else
 	    log ("considering event %i", idx);
 	}
+      gcc_assert (var == NULL || !CONSTANT_CLASS_P (var));
       switch (base_event->m_kind)
 	{
 	default:
@@ -1164,6 +1165,11 @@  diagnostic_manager::prune_for_sm_diagnostic (checker_path *path,
 		    log ("event %i: switching var of interest from %qE to %qE",
 			 idx, var, state_change->m_origin);
 		    var = state_change->m_origin;
+		    if (var && CONSTANT_CLASS_P (var))
+		      {
+			log ("new var is a constant; setting var to NULL");
+			var = NULL_TREE;
+		      }
 		  }
 		log ("event %i: switching state of interest from %qs to %qs",
 		     idx, sm->get_state_name (state_change->m_to),
@@ -1260,6 +1266,11 @@  diagnostic_manager::prune_for_sm_diagnostic (checker_path *path,
 		var = caller_var;
 		if (expr.param_p ())
 		  event->record_critical_state (var, state);
+		if (var && CONSTANT_CLASS_P (var))
+		  {
+		    log ("new var is a constant; setting var to NULL");
+		    var = NULL_TREE;
+		  }
 	      }
 	  }
 	  break;
@@ -1285,6 +1296,11 @@  diagnostic_manager::prune_for_sm_diagnostic (checker_path *path,
 		    var = callee_var;
 		    if (expr.return_value_p ())
 		      event->record_critical_state (var, state);
+		    if (var && CONSTANT_CLASS_P (var))
+		      {
+			log ("new var is a constant; setting var to NULL");
+			var = NULL_TREE;
+		      }
 		  }
 	      }
 	  }
diff --git a/gcc/testsuite/g++.dg/analyzer/pr93950.C b/gcc/testsuite/g++.dg/analyzer/pr93950.C
new file mode 100644
index 00000000000..e2808173407
--- /dev/null
+++ b/gcc/testsuite/g++.dg/analyzer/pr93950.C
@@ -0,0 +1,28 @@ 
+// { dg-do compile { target c++11 } }
+
+struct d
+{
+  struct e
+  {
+    int f;
+    int *g;
+  };
+  void h (e * i)
+  {
+    void *j = nullptr; // { dg-bogus "NULL" "" { xfail *-*-* } }
+    // TODO(xfail): we report "'i' is NULL" above, which is the wrong location
+    
+    i->f = *i->g; // { dg-warning "dereference of NULL 'i'" }
+  }
+  virtual void c (int, int)
+  {
+    int *j = nullptr;
+    h (nullptr);
+  }
+};
+
+void
+foo ()
+{
+  d ();
+}