analyzer: fix folding of regions involving unknown ptrs [PR103892]

PR analyzer/103892 reports a false positive from -Wanalyzer-double-free.

The root cause is the analyzer failing to properly handle "unknown"
symbolic regions, and thus confusing two different expressions.

Specifically, the analyzer eventually hits the complexity limit for
symbolic values, and starts using an "unknown" svalue for a pointer.
The analyzer uses
  symbolic_region(unknown_svalue([of ptr type]))
i.e.
  (*UNKNOWN_PTR)
in a few places to mean "we have an lvalue, but we're not going to
attempt to track what it is anymore".

"Unknown" should probably be renamed to "unknowable"; in theory, any
operation on such an unknown svalue should be also an unknown svalue.

The issue is that in various places where we create child regions, we
were failing to check for the parent region being (*UNKNOWN_PTR), and so
were erroneously creating regions based on (*UNKNOWN_PTR), such as
*(UNKNOWN_PTR + OFFSET).  The state-machine handling was erroneously
allowing e.g. INITIAL_VALUE (*(UNKNOWN_PTR + OFFSET)) to have state,
and thus we could record that such a value had had "free" called on it,
and thus eventually false report a double-free when a different
expression incorrectly "simplified" to the same expression.

This patch fixes things by checking when creating the various kinds of
child region for (*UNKNOWN_PTR) as the parent region, and simply
returning another (*UNKNOWN_PTR) for such child regions (using the
appropriate type).

Doing so fixes the false positive, and also fixes a state explosion on
this testcase, as the states at the program points more rapidly reach
a fixed point where everything is unknown.  I checked for other cases
that no longer needed -Wno-analyzer-too-complex; the only other one
seems to be gcc.dg/analyzer/pr96841.c, but that seems to already have
become redundant at some point before this patch.

gcc/analyzer/ChangeLog:
	PR analyzer/103892
	* region-model-manager.cc
	(region_model_manager::get_unknown_symbolic_region): New,
	extracted from...
	(region_model_manager::get_field_region): ...here.
	(region_model_manager::get_element_region): Use it here.
	(region_model_manager::get_offset_region): Likewise.
	(region_model_manager::get_sized_region): Likewise.
	(region_model_manager::get_cast_region): Likewise.
	(region_model_manager::get_bit_range): Likewise.
	* region-model.h
	(region_model_manager::get_unknown_symbolic_region): New decl.
	* region.cc (symbolic_region::symbolic_region): Handle sval_ptr
	having NULL type.
	(symbolic_region::dump_to_pp): Handle having NULL type.

gcc/testsuite/ChangeLog:
	PR analyzer/103892
	* gcc.dg/analyzer/pr103892.c: New test.
	* gcc.dg/analyzer/pr96841.c: Drop redundant
	-Wno-analyzer-too-complex.

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

diff --git a/gcc/analyzer/region-model-manager.cc b/gcc/analyzer/region-model-manager.cc
index 56d60768749ee99be631f904495d7daeeca6f665..4ec275ecd431117ff4233b901e97b41f2d8f56c1 100644 (file)
--- a/gcc/analyzer/region-model-manager.cc
+++ b/gcc/analyzer/region-model-manager.cc
@@ -1362,6 +1362,19 @@ region_model_manager::get_region_for_global (tree expr)
   return reg;
 }
 
+/* Return the region for an unknown access of type REGION_TYPE,
+   creating it if necessary.
+   This is a symbolic_region, where the pointer is an unknown_svalue
+   of type &REGION_TYPE.  */
+
+const region *
+region_model_manager::get_unknown_symbolic_region (tree region_type)
+{
+  tree ptr_type = region_type ? build_pointer_type (region_type) : NULL_TREE;
+  const svalue *unknown_ptr = get_or_create_unknown_svalue (ptr_type);
+  return get_symbolic_region (unknown_ptr);
+}
+
 /* Return the region that describes accessing field FIELD of PARENT,
    creating it if necessary.  */
 
@@ -1372,12 +1385,7 @@ region_model_manager::get_field_region (const region *parent, tree field)
 
   /* (*UNKNOWN_PTR).field is (*UNKNOWN_PTR_OF_&FIELD_TYPE).  */
   if (parent->symbolic_for_unknown_ptr_p ())
-    {
-      tree ptr_to_field_type = build_pointer_type (TREE_TYPE (field));
-      const svalue *unknown_ptr_to_field
-       = get_or_create_unknown_svalue (ptr_to_field_type);
-      return get_symbolic_region (unknown_ptr_to_field);
-    }
+    return get_unknown_symbolic_region (TREE_TYPE (field));
 
   field_region::key_t key (parent, field);
   if (field_region *reg = m_field_regions.get (key))
@@ -1397,6 +1405,10 @@ region_model_manager::get_element_region (const region *parent,
                                          tree element_type,
                                          const svalue *index)
 {
+  /* (UNKNOWN_PTR[IDX]) is (UNKNOWN_PTR).  */
+  if (parent->symbolic_for_unknown_ptr_p ())
+    return get_unknown_symbolic_region (element_type);
+
   element_region::key_t key (parent, element_type, index);
   if (element_region *reg = m_element_regions.get (key))
     return reg;
@@ -1416,6 +1428,10 @@ region_model_manager::get_offset_region (const region *parent,
                                         tree type,
                                         const svalue *byte_offset)
 {
+  /* (UNKNOWN_PTR + OFFSET) is (UNKNOWN_PTR).  */
+  if (parent->symbolic_for_unknown_ptr_p ())
+    return get_unknown_symbolic_region (type);
+
   /* If BYTE_OFFSET is zero, return PARENT.  */
   if (tree cst_offset = byte_offset->maybe_get_constant ())
     if (zerop (cst_offset))
@@ -1451,6 +1467,9 @@ region_model_manager::get_sized_region (const region *parent,
                                        tree type,
                                        const svalue *byte_size_sval)
 {
+  if (parent->symbolic_for_unknown_ptr_p ())
+    return get_unknown_symbolic_region (type);
+
   if (byte_size_sval->get_type () != size_type_node)
     byte_size_sval = get_or_create_cast (size_type_node, byte_size_sval);
 
@@ -1486,6 +1505,9 @@ region_model_manager::get_cast_region (const region *original_region,
   if (type == original_region->get_type ())
     return original_region;
 
+  if (original_region->symbolic_for_unknown_ptr_p ())
+    return get_unknown_symbolic_region (type);
+
   cast_region::key_t key (original_region, type);
   if (cast_region *reg = m_cast_regions.get (key))
     return reg;
@@ -1558,6 +1580,9 @@ region_model_manager::get_bit_range (const region *parent, tree type,
 {
   gcc_assert (parent);
 
+  if (parent->symbolic_for_unknown_ptr_p ())
+    return get_unknown_symbolic_region (type);
+
   bit_range_region::key_t key (parent, type, bits);
   if (bit_range_region *reg = m_bit_range_regions.get (key))
     return reg;

diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 23841718b5c2662377c2adfa16f5c84cca83df28..eff3d4930f9b123a62a9eb4356307803f5ba05a0 100644 (file)
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -327,6 +327,8 @@ public:
   const region *get_bit_range (const region *parent, tree type,
                               const bit_range &bits);
 
+  const region *get_unknown_symbolic_region (tree region_type);
+
   const region *
   get_region_for_unexpected_tree_code (region_model_context *ctxt,
                                       tree t,

diff --git a/gcc/analyzer/region.cc b/gcc/analyzer/region.cc
index 749e6182b06fe31cc9a0f69e75ea3b65171431eb..1a7949b3f097b86498ce23f9a2ceb5af833fa45d 100644 (file)
--- a/gcc/analyzer/region.cc
+++ b/gcc/analyzer/region.cc
@@ -1016,7 +1016,9 @@ root_region::dump_to_pp (pretty_printer *pp, bool simple) const
 symbolic_region::symbolic_region (unsigned id, region *parent,
                                  const svalue *sval_ptr)
 : region (complexity::from_pair (parent, sval_ptr), id, parent,
-         TREE_TYPE (sval_ptr->get_type ())),
+         (sval_ptr->get_type ()
+          ? TREE_TYPE (sval_ptr->get_type ())
+          : NULL_TREE)),
   m_sval_ptr (sval_ptr)
 {
 }
@@ -1045,8 +1047,11 @@ symbolic_region::dump_to_pp (pretty_printer *pp, bool simple) const
     {
       pp_string (pp, "symbolic_region(");
       get_parent_region ()->dump_to_pp (pp, simple);
-      pp_string (pp, ", ");
-      print_quoted_type (pp, get_type ());
+      if (get_type ())
+       {
+         pp_string (pp, ", ");
+         print_quoted_type (pp, get_type ());
+       }
       pp_string (pp, ", ");
       m_sval_ptr->dump_to_pp (pp, simple);
       pp_string (pp, ")");

diff --git a/gcc/testsuite/gcc.dg/analyzer/pr103892.c b/gcc/testsuite/gcc.dg/analyzer/pr103892.c
new file mode 100644 (file)
index 0000000..e9775b6
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pr103892.c
@@ -0,0 +1,75 @@
+/* { dg-additional-options "-O2" } */
+
+extern void free (void *__ptr) __attribute__ ((__nothrow__ , __leaf__));
+
+enum pipecmd_tag
+{
+ PIPECMD_PROCESS,
+ PIPECMD_SEQUENCE
+};
+
+struct pipecmd {
+ enum pipecmd_tag tag;
+ union {
+  struct pipecmd_process {
+   int argc;
+   int argv_max;
+   char **argv;
+  } process;
+  struct pipecmd_sequence {
+   int ncommands;
+   int commands_max;
+   struct pipecmd **commands;
+  } sequence;
+ } u;
+};
+
+static char *argstr_get_word (const char **argstr)
+{
+ while (**argstr) {
+  switch (**argstr) {
+   case ' ':
+   case '\t':
+    return (void *) 0;
+  }
+ }
+ return (void *) 0;
+}
+
+struct pipecmd *pipecmd_new_argstr (const char *argstr)
+{
+ argstr_get_word (&argstr);
+ return (void *) 0;
+}
+
+void pipecmd_free (struct pipecmd *cmd)
+{
+ int i;
+
+ if (!cmd)
+  return;
+
+ switch (cmd->tag) {
+  case PIPECMD_PROCESS: {
+   struct pipecmd_process *cmdp = &cmd->u.process;
+
+   for (i = 0; i < cmdp->argc; ++i)
+    free (cmdp->argv[i]);
+   free (cmdp->argv);
+
+   break;
+  }
+
+  case PIPECMD_SEQUENCE: {
+   struct pipecmd_sequence *cmds = &cmd->u.sequence;
+
+   for (i = 0; i < cmds->ncommands; ++i)
+    pipecmd_free (cmds->commands[i]);
+   free (cmds->commands);
+
+   break;
+  }
+ }
+
+ free (cmd);
+}

diff --git a/gcc/testsuite/gcc.dg/analyzer/pr96841.c b/gcc/testsuite/gcc.dg/analyzer/pr96841.c
index c76658288b711b326c4160963b232a7e8d1541a4..14f3f7a86a338686ec660fb9970ecafd2f425097 100644 (file)
--- a/gcc/testsuite/gcc.dg/analyzer/pr96841.c
+++ b/gcc/testsuite/gcc.dg/analyzer/pr96841.c
@@ -1,4 +1,4 @@
-/* { dg-additional-options "-Wno-analyzer-too-complex -O1 -Wno-builtin-declaration-mismatch" } */
+/* { dg-additional-options "-O1 -Wno-builtin-declaration-mismatch" } */
 
 int
 l8 (void);