The underlying cause of PR 89170 is a bug that appears to have existed
since 2001, when the function decode_addr_const was changed to call
output_constant_def. The problem is that output_constant_def calls
compare_constant, and when compare_constant sees an ADDR_EXPR, it
calls decode_addr_const. So it is possible for output_constant_def to
call itself recursively while adding a value to the hash table. This
only happens if there are already some constants in the hash table
with the same hash code, because that is the only case in which
compare_constant can call decode_addr_constant. This works fine if
the value whose address is taken is already in the hash table. And it
works fine if the address is not in the hash table, but adding the
address does not cause the hash table to grow.
But if we call output_constant_def with a constant that includes an
ADDR_EXPR, and if there is already a constant in the hash table with
the same hash code, and if decode_addr_constant calls
output_constant_def recursively with a constant that is not already in
the hash table, and if adding that constant causes the hash table to
grow, then the outer call to output_constant_def will wind up looking
at the wrong hash bucket. The effect is that output_constant_def will
return an rtx for a different constant.
This unlikely sequence of events actually happened building libgo's
net/http test on PPC64, causing a miscompilation leading to a test
failure filed as PR 89170.
I plan to commit this patch to fix the problem. I didn't add a test
case since the sequence of events is so hard to recreate. I added a
check that will detect any future recursive insertion into the hash
table.
Bootstrapped and tested on x86_64-pc-linux-gnu. Since I haven't
looked at this part of the code in many years, I'll wait a bit to see
if anybody has any comments on the patch.
Ian
2019-02-20 Ian Lance Taylor <i...@golang.org>
PR go/89170
* varasm.c (decode_addr_const): Call lookup_constant_def rather
than output_constant_def.
(const_desc_htab_inserting): New static variable.
(output_constant_def): Call output_addressed_constants. Check and
set const_desc_htab_inserting.
(tree_output_constant_def): Likewise.
Index: varasm.c
===================================================================
--- varasm.c (revision 268949)
+++ varasm.c (working copy)
@@ -2961,7 +2961,9 @@ decode_addr_const (tree exp, struct addr
case COMPLEX_CST:
case CONSTRUCTOR:
case INTEGER_CST:
- x = output_constant_def (target, 1);
+ x = lookup_constant_def (target);
+ /* Should have been added by output_addressed_constants. */
+ gcc_assert (x);
break;
case INDIRECT_REF:
@@ -2989,6 +2991,9 @@ decode_addr_const (tree exp, struct addr
static GTY(()) hash_table<tree_descriptor_hasher> *const_desc_htab;
+/* Used to guard against a recursive insert. */
+static bool const_desc_htab_inserting;
+
static void maybe_output_constant_def_contents (struct
constant_descriptor_tree *, int);
/* Constant pool accessor function. */
@@ -3443,6 +3448,13 @@ output_constant_def (tree exp, int defer
struct constant_descriptor_tree *desc;
struct constant_descriptor_tree key;
+ /* The hash table methods may call output_constant_def recursively
+ for addressed constants, so handle them first. */
+ output_addressed_constants (exp);
+
+ gcc_assert (!const_desc_htab_inserting);
+ const_desc_htab_inserting = true;
+
/* Look up EXP in the table of constant descriptors. If we didn't find
it, create a new one. */
key.value = exp;
@@ -3450,6 +3462,8 @@ output_constant_def (tree exp, int defer
constant_descriptor_tree **loc
= const_desc_htab->find_slot_with_hash (&key, key.hash, INSERT);
+ const_desc_htab_inserting = false;
+
desc = *loc;
if (desc == 0)
{
@@ -3594,6 +3608,14 @@ tree_output_constant_def (tree exp)
struct constant_descriptor_tree *desc, key;
tree decl;
+ /* The hash table methods may call output_constant_def recursively
+ for addressed constants, which may modify the hash table, so
+ handle them first. */
+ output_addressed_constants (exp);
+
+ gcc_assert (!const_desc_htab_inserting);
+ const_desc_htab_inserting = true;
+
/* Look up EXP in the table of constant descriptors. If we didn't find
it, create a new one. */
key.value = exp;
@@ -3601,6 +3623,8 @@ tree_output_constant_def (tree exp)
constant_descriptor_tree **loc
= const_desc_htab->find_slot_with_hash (&key, key.hash, INSERT);
+ const_desc_htab_inserting = false;
+
desc = *loc;
if (desc == 0)
{
