[committed] analyzer: add svalue::can_have_associated_state_p [PR101503]

Message ID 20210719194718.297068-1-dmalcolm@redhat.com
State New
Headers show
Series
  • [committed] analyzer: add svalue::can_have_associated_state_p [PR101503]
Related show

Commit Message

Xionghu Luo via Gcc-patches July 19, 2021, 7:47 p.m.
PR analyzer/101503 reports an assertion failure due to an unexpected
"UNKNOWN" value (due to using --param analyzer-max-svalue-depth=0).

This patch fixes this by rejecting attempts to purge state involving
unknown/poisoned svalues (in region_model::purge_state_involving),
as these svalues should not have state associated with them - they
are singletons w.r.t each type.

To be more systematic about this, the patch also introduces a new
svalue::can_have_associated_state_p which returns false for
unknown/poisoned svalues, so that we can reject adding constraints
or sm-state on them, or building various kinds of svalue in terms
of them (e.g. unary ops, binary ops, etc).

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r12-2399-ga113b14398f2a4ad2742e6e9c87e25cac60f263e.

gcc/analyzer/ChangeLog:
	PR analyzer/101503
	* constraint-manager.cc (constraint_manager::add_constraint): Use
	can_have_associated_state_p rather than testing for unknown.
	(constraint_manager::get_or_add_equiv_class): Likewise.
	* program-state.cc (sm_state_map::set_state): Likewise.
	(sm_state_map::impl_set_state): Add assertion.
	* region-model-manager.cc
	(region_model_manager::maybe_fold_unaryop): Handle poisoned
	values.
	(region_model_manager::maybe_fold_binop): Move handling of unknown
	values...
	(region_model_manager::get_or_create_binop): ...to here, and
	generalize to use can_have_associated_state_p.
	(region_model_manager::maybe_fold_sub_svalue): Use
	can_have_associated_state_p rather than testing for unknown.
	(region_model_manager::maybe_fold_repeated_svalue): Use unknown
	when the size or repeated value is "unknown"/"poisoned".
	* region-model.cc (region_model::purge_state_involving): Reject
	attempts to purge unknown/poisoned svalues, as these svalues
	should not have state associated with them.
	* svalue.cc (sub_svalue::sub_svalue): Assert that we're building
	on top of an svalue with can_have_associated_state_p.
	(repeated_svalue::repeated_svalue): Likewise.
	(bits_within_svalue::bits_within_svalue): Likewise.
	* svalue.h (svalue::can_have_associated_state_p): New.
	(unknown_svalue::can_have_associated_state_p): New.
	(poisoned_svalue::can_have_associated_state_p): New.
	(unaryop_svalue::unaryop_svalue): Assert that we're building on
	top of an svalue with can_have_associated_state_p.
	(binop_svalue::binop_svalue): Likewise.
	(widening_svalue::widening_svalue): Likewise.

gcc/testsuite/ChangeLog:
	PR analyzer/101503
	* gcc.dg/analyzer/pr101503.c: New test.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>

---
 gcc/analyzer/constraint-manager.cc       |  8 +++----
 gcc/analyzer/program-state.cc            |  6 +++--
 gcc/analyzer/region-model-manager.cc     | 28 +++++++++++++++++-------
 gcc/analyzer/region-model.cc             |  2 ++
 gcc/analyzer/svalue.cc                   |  4 ++++
 gcc/analyzer/svalue.h                    | 16 ++++++++++++++
 gcc/testsuite/gcc.dg/analyzer/pr101503.c | 11 ++++++++++
 7 files changed, 61 insertions(+), 14 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr101503.c

-- 
2.26.3

Patch

diff --git a/gcc/analyzer/constraint-manager.cc b/gcc/analyzer/constraint-manager.cc
index 5b5a9dec0a9..f59929a75ca 100644
--- a/gcc/analyzer/constraint-manager.cc
+++ b/gcc/analyzer/constraint-manager.cc
@@ -833,9 +833,9 @@  constraint_manager::add_constraint (const svalue *lhs,
   lhs = lhs->unwrap_any_unmergeable ();
   rhs = rhs->unwrap_any_unmergeable ();
 
-  /* Nothing can be known about unknown values.  */
-  if (lhs->get_kind () == SK_UNKNOWN
-      || rhs->get_kind () == SK_UNKNOWN)
+  /* Nothing can be known about unknown/poisoned values.  */
+  if (!lhs->can_have_associated_state_p ()
+      || !rhs->can_have_associated_state_p ())
     /* Not a contradiction.  */
     return true;
 
@@ -1175,7 +1175,7 @@  constraint_manager::get_or_add_equiv_class (const svalue *sval)
 {
   equiv_class_id result (-1);
 
-  gcc_assert (sval->get_kind () != SK_UNKNOWN);
+  gcc_assert (sval->can_have_associated_state_p ());
 
   /* Convert all NULL pointers to (void *) to avoid state explosions
      involving all of the various (foo *)NULL vs (bar *)NULL.  */
diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index ccfe7b019b0..5bb86767873 100644
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -453,8 +453,8 @@  sm_state_map::set_state (region_model *model,
   if (model == NULL)
     return;
 
-  /* Reject attempts to set state on UNKNOWN.  */
-  if (sval->get_kind () == SK_UNKNOWN)
+  /* Reject attempts to set state on UNKNOWN/POISONED.  */
+  if (!sval->can_have_associated_state_p ())
     return;
 
   equiv_class &ec = model->get_constraints ()->get_equiv_class (sval);
@@ -492,6 +492,8 @@  sm_state_map::impl_set_state (const svalue *sval,
   if (get_state (sval, ext_state) == state)
     return false;
 
+  gcc_assert (sval->can_have_associated_state_p ());
+
   /* Special-case state 0 as the default value.  */
   if (state == 0)
     {
diff --git a/gcc/analyzer/region-model-manager.cc b/gcc/analyzer/region-model-manager.cc
index 7a52a64bbb3..fccb93ea5d1 100644
--- a/gcc/analyzer/region-model-manager.cc
+++ b/gcc/analyzer/region-model-manager.cc
@@ -340,6 +340,13 @@  region_model_manager::maybe_fold_unaryop (tree type, enum tree_code op,
   /* Ops on "unknown" are also unknown.  */
   if (arg->get_kind () == SK_UNKNOWN)
     return get_or_create_unknown_svalue (type);
+  /* Likewise for "poisoned".  */
+  else if (const poisoned_svalue *poisoned_sval
+	     = arg->dyn_cast_poisoned_svalue ())
+    return get_or_create_poisoned_svalue (poisoned_sval->get_poison_kind (),
+					  type);
+
+  gcc_assert (arg->can_have_associated_state_p ());
 
   switch (op)
     {
@@ -615,12 +622,6 @@  region_model_manager::maybe_fold_binop (tree type, enum tree_code op,
 	     get_or_create_binop (size_type_node, op,
 				  binop->get_arg1 (), arg1));
 
-  /* Ops on "unknown" are also unknown (unless we can use one of the
-     identities above).  */
-  if (arg0->get_kind () == SK_UNKNOWN
-      || arg1->get_kind () == SK_UNKNOWN)
-    return get_or_create_unknown_svalue (type);
-
   /* etc.  */
 
   return NULL;
@@ -641,6 +642,12 @@  region_model_manager::get_or_create_binop (tree type, enum tree_code op,
   if (const svalue *folded = maybe_fold_binop (type, op, arg0, arg1))
     return folded;
 
+  /* Ops on "unknown"/"poisoned" are unknown (unless we were able to fold
+     it via an identity in maybe_fold_binop).  */
+  if (!arg0->can_have_associated_state_p ()
+      || !arg1->can_have_associated_state_p ())
+    return get_or_create_unknown_svalue (type);
+
   binop_svalue::key_t key (type, op, arg0, arg1);
   if (binop_svalue **slot = m_binop_values_map.get (key))
     return *slot;
@@ -658,8 +665,8 @@  region_model_manager::maybe_fold_sub_svalue (tree type,
 					     const svalue *parent_svalue,
 					     const region *subregion)
 {
-  /* Subvalues of "unknown" are unknown.  */
-  if (parent_svalue->get_kind () == SK_UNKNOWN)
+  /* Subvalues of "unknown"/"poisoned" are unknown.  */
+  if (!parent_svalue->can_have_associated_state_p ())
     return get_or_create_unknown_svalue (type);
 
   /* If we have a subregion of a zero-fill, it's zero.  */
@@ -755,6 +762,11 @@  region_model_manager::maybe_fold_repeated_svalue (tree type,
 						  const svalue *outer_size,
 						  const svalue *inner_svalue)
 {
+  /* Repeated "unknown"/"poisoned" is unknown.  */
+  if (!outer_size->can_have_associated_state_p ()
+      || !inner_svalue->can_have_associated_state_p ())
+    return get_or_create_unknown_svalue (type);
+
   /* If INNER_SVALUE is the same size as OUTER_SIZE,
      turn into simply a cast.  */
   if (tree cst_outer_num_bytes = outer_size->maybe_get_constant ())
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 4fab1ef8427..6d02c60449c 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -1304,6 +1304,8 @@  void
 region_model::purge_state_involving (const svalue *sval,
 				     region_model_context *ctxt)
 {
+  if (!sval->can_have_associated_state_p ())
+    return;
   m_store.purge_state_involving (sval, m_mgr);
   m_constraints->purge_state_involving (sval);
   m_dynamic_extents.purge_state_involving (sval);
diff --git a/gcc/analyzer/svalue.cc b/gcc/analyzer/svalue.cc
index 323df8015fd..094c7256818 100644
--- a/gcc/analyzer/svalue.cc
+++ b/gcc/analyzer/svalue.cc
@@ -1109,6 +1109,7 @@  sub_svalue::sub_svalue (tree type, const svalue *parent_svalue,
 	  type),
   m_parent_svalue (parent_svalue), m_subregion (subregion)
 {
+  gcc_assert (parent_svalue->can_have_associated_state_p ());
 }
 
 /* Implementation of svalue::dump_to_pp vfunc for sub_svalue.  */
@@ -1165,6 +1166,8 @@  repeated_svalue::repeated_svalue (tree type,
   m_outer_size (outer_size),
   m_inner_svalue (inner_svalue)
 {
+  gcc_assert (outer_size->can_have_associated_state_p ());
+  gcc_assert (inner_svalue->can_have_associated_state_p ());
 }
 
 /* Implementation of svalue::dump_to_pp vfunc for repeated_svalue.  */
@@ -1290,6 +1293,7 @@  bits_within_svalue::bits_within_svalue (tree type,
   m_bits (bits),
   m_inner_svalue (inner_svalue)
 {
+  gcc_assert (inner_svalue->can_have_associated_state_p ());
 }
 
 /* Implementation of svalue::dump_to_pp vfunc for bits_within_svalue.  */
diff --git a/gcc/analyzer/svalue.h b/gcc/analyzer/svalue.h
index 15198899cd5..debe4391475 100644
--- a/gcc/analyzer/svalue.h
+++ b/gcc/analyzer/svalue.h
@@ -160,6 +160,11 @@  public:
 
   virtual bool all_zeroes_p () const;
 
+  /* Can this svalue be involved in constraints and sm-state?
+     Most can, but UNKNOWN and POISONED svalues are singletons
+     per-type and thus it's meaningless for them to "have state".  */
+  virtual bool can_have_associated_state_p () const { return true; }
+
  protected:
   svalue (complexity c, tree type)
   : m_complexity (c), m_type (type)
@@ -319,6 +324,9 @@  public:
   maybe_fold_bits_within (tree type,
 			  const bit_range &subrange,
 			  region_model_manager *mgr) const FINAL OVERRIDE;
+
+  /* Unknown values are singletons per-type, so can't have state.  */
+  bool can_have_associated_state_p () const FINAL OVERRIDE { return false; }
 };
 
 /* An enum describing a particular kind of "poisoned" value.  */
@@ -389,6 +397,9 @@  public:
 
   enum poison_kind get_poison_kind () const { return m_kind; }
 
+  /* Poisoned svalues are singletons per-type, so can't have state.  */
+  bool can_have_associated_state_p () const FINAL OVERRIDE { return false; }
+
  private:
   enum poison_kind m_kind;
 };
@@ -602,6 +613,7 @@  public:
   unaryop_svalue (tree type, enum tree_code op, const svalue *arg)
   : svalue (complexity (arg), type), m_op (op), m_arg (arg)
   {
+    gcc_assert (arg->can_have_associated_state_p ());
   }
 
   enum svalue_kind get_kind () const FINAL OVERRIDE { return SK_UNARYOP; }
@@ -694,6 +706,8 @@  public:
 	     type),
     m_op (op), m_arg0 (arg0), m_arg1 (arg1)
   {
+    gcc_assert (arg0->can_have_associated_state_p ());
+    gcc_assert (arg1->can_have_associated_state_p ());
   }
 
   enum svalue_kind get_kind () const FINAL OVERRIDE { return SK_BINOP; }
@@ -1135,6 +1149,8 @@  public:
     m_point (point.get_function_point ()),
     m_base_sval (base_sval), m_iter_sval (iter_sval)
   {
+    gcc_assert (base_sval->can_have_associated_state_p ());
+    gcc_assert (iter_sval->can_have_associated_state_p ());
   }
 
   enum svalue_kind get_kind () const FINAL OVERRIDE { return SK_WIDENING; }
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr101503.c b/gcc/testsuite/gcc.dg/analyzer/pr101503.c
new file mode 100644
index 00000000000..16faf6eac2f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pr101503.c
@@ -0,0 +1,11 @@ 
+/* { dg-additional-options "--param analyzer-max-svalue-depth=0" } */
+
+int val;
+
+int
+fn (void)
+{
+  val = fn ();
+
+  return 0;
+}