[gcc r12-8071] analyzer: fix folding of regions involving unknown ptrs [PR103892]

[gcc r12-8071] analyzer: fix folding of regions involving unknown ptrs [PR103892]

[David Malcolm]

https://gcc.gnu.org/g:3d41408c5d28105e7a3ea2eb2529431a70b96369

commit r12-8071-g3d41408c5d28105e7a3ea2eb2529431a70b96369
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Sat Apr 9 18:12:57 2022 -0400

    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:
---
 gcc/analyzer/region-model-manager.cc     | 37 +++++++++++++---
 gcc/analyzer/region-model.h              |  2 +
 gcc/analyzer/region.cc                   | 11 +++--
 gcc/testsuite/gcc.dg/analyzer/pr103892.c | 75 ++++++++++++++++++++++++++++++++
 gcc/testsuite/gcc.dg/analyzer/pr96841.c  |  2 +-
 5 files changed, 117 insertions(+), 10 deletions(-)

diff --git a/gcc/analyzer/region-model-manager.cc b/gcc/analyzer/region-model-manager.cc
index 56d60768749..4ec275ecd43 100644
--- 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 23841718b5c..eff3d4930f9 100644
--- 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 749e6182b06..1a7949b3f09 100644
--- 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
index 00000000000..e9775b69ad0
--- /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 c76658288b7..14f3f7a86a3 100644
--- 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);