diff mbox series

[committed] analyzer: fix ICE with -fsanitize=undefined [PR98293]

Message ID 20210105002258.3038219-1-dmalcolm@redhat.com
State New
Headers show
Series [committed] analyzer: fix ICE with -fsanitize=undefined [PR98293] | expand

Commit Message

David Malcolm Jan. 5, 2021, 12:22 a.m. UTC 
-fsanitize=undefined with calls to nonnull functions
creates struct __ubsan_nonnull_arg_data instances
with CONSTRUCTORs for RECORD_TYPEs with NULL index values.
The analyzer was mistakenly using INTEGER_CST for these
fields, leading to ICEs.

Fix the issue by iterating through the fields in the type
for such cases, imitating similar logic in varasm.c's
output_constructor.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to master as r11-6452-g15af33a88065f983181550fc53821f1c6e14c5c7.

gcc/analyzer/ChangeLog:
	PR analyzer/98293
	* store.cc (binding_map::apply_ctor_to_region): When "index" is
	NULL, iterate through the fields for RECORD_TYPEs, rather than
	creating an INTEGER_CST index.

gcc/testsuite/ChangeLog:
	PR analyzer/98293
	* gcc.dg/analyzer/pr98293.c: New test.
---
 gcc/analyzer/store.cc                   | 19 ++++++++++++++++++-
 gcc/testsuite/gcc.dg/analyzer/pr98293.c |  2 ++
 2 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr98293.c

Comments

Jakub Jelinek Jan. 5, 2021, 12:41 a.m. UTC | #1 
On Mon, Jan 04, 2021 at 07:22:58PM -0500, David Malcolm via Gcc-patches wrote:
> --- a/gcc/analyzer/store.cc
> +++ b/gcc/analyzer/store.cc
> @@ -524,10 +524,27 @@ binding_map::apply_ctor_to_region (const region *parent_reg, tree ctor,
>    unsigned ix;
>    tree index;
>    tree val;
> +  tree parent_type = parent_reg->get_type ();
> +  tree field;
> +  if (TREE_CODE (parent_type) == RECORD_TYPE)
> +    field = TYPE_FIELDS (parent_type);
> +  else
> +    field = NULL_TREE;
>    FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (ctor), ix, index, val)
>      {
>        if (!index)
> -	index = build_int_cst (integer_type_node, ix);
> +	{
> +	  /* If index is NULL, then iterate through the fields for
> +	     a RECORD_TYPE, or use an INTEGER_CST otherwise.
> +	     Compare with similar logic in output_constructor.  */
> +	  if (field)
> +	    {
> +	      index = field;
> +	      field = DECL_CHAIN (field);
> +	    }

The TYPE_FIELDS chain doesn't contain only FIELD_DECLs, can contain other
decls (FUNCTION_DECLs, USING_DECLs, TYPE_DECLs, ...).
So this should be really skipping chain elts other than FIELD_DECLs.
E.g. C++ FE has next_initializable_field function for that, unfortunately
the middle-end doesn't.

	Jakub
diff mbox series

Patch

diff --git a/gcc/analyzer/store.cc b/gcc/analyzer/store.cc
index 35a7e2985cd..b4a4d5f3bb2 100644
--- a/gcc/analyzer/store.cc
+++ b/gcc/analyzer/store.cc
@@ -524,10 +524,27 @@  binding_map::apply_ctor_to_region (const region *parent_reg, tree ctor,
   unsigned ix;
   tree index;
   tree val;
+  tree parent_type = parent_reg->get_type ();
+  tree field;
+  if (TREE_CODE (parent_type) == RECORD_TYPE)
+    field = TYPE_FIELDS (parent_type);
+  else
+    field = NULL_TREE;
   FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (ctor), ix, index, val)
     {
       if (!index)
-	index = build_int_cst (integer_type_node, ix);
+	{
+	  /* If index is NULL, then iterate through the fields for
+	     a RECORD_TYPE, or use an INTEGER_CST otherwise.
+	     Compare with similar logic in output_constructor.  */
+	  if (field)
+	    {
+	      index = field;
+	      field = DECL_CHAIN (field);
+	    }
+	  else
+	    index = build_int_cst (integer_type_node, ix);
+	}
       else if (TREE_CODE (index) == RANGE_EXPR)
 	{
 	  tree min_index = TREE_OPERAND (index, 0);
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr98293.c b/gcc/testsuite/gcc.dg/analyzer/pr98293.c
new file mode 100644
index 00000000000..f750c902440
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pr98293.c
@@ -0,0 +1,2 @@ 
+/* { dg-additional-options "-fsanitize=undefined" } */
+#include "../pr93399.c"