aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Malcolm <dmalcolm@redhat.com>2022-04-09 18:12:57 -0400
committerDavid Malcolm <dmalcolm@redhat.com>2022-04-09 18:12:57 -0400
commit3d41408c5d28105e7a3ea2eb2529431a70b96369 (patch)
tree1b9f55531c36d80333f3556df8f2a71a8221243d
parent4943b75e9f06f0b64ed541430bb7fbccf55fc552 (diff)
downloadgcc-3d41408c5d28105e7a3ea2eb2529431a70b96369.zip
gcc-3d41408c5d28105e7a3ea2eb2529431a70b96369.tar.gz
gcc-3d41408c5d28105e7a3ea2eb2529431a70b96369.tar.bz2
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>
-rw-r--r--gcc/analyzer/region-model-manager.cc37
-rw-r--r--gcc/analyzer/region-model.h2
-rw-r--r--gcc/analyzer/region.cc11
-rw-r--r--gcc/testsuite/gcc.dg/analyzer/pr103892.c75
-rw-r--r--gcc/testsuite/gcc.dg/analyzer/pr96841.c2
5 files changed, 117 insertions, 10 deletions
diff --git a/gcc/analyzer/region-model-manager.cc b/gcc/analyzer/region-model-manager.cc
index 56d6076..4ec275e 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 2384171..eff3d49 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 749e618..1a7949b3f 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 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 c766582..14f3f7a 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);