help debug hash_map garbage collection issue

help debug hash_map garbage collection issue

[Martin Sebor]

I have a static hash_map object that needs to persist across passes:

   static GTY(()) hash_map<tree, bitmap> *map;

I initialize the map like so:

   map = hash_map<tree, bitmap>::create_ggc (4);

But I see crashes when accessing the map after adding and removing
some number of entries in a few passes.  The crashes are due to
the map having been garbage-collected and the memory allocated to
it poisoned (it has the 0xa5a5a5a5a5a5a5a5 bit pattern that I see
in GDD being written into the map by poison_pages()).

I assumed marking the map pointer GTY(()) would keep the garbage
collector from touching the map.  Is there something else I need
to do to keep it from doing that?

Thanks
Martin

PS Just in case it matters, the bitmap in the table is allocated
via BITMAP_ALLOC() with a bitmap_obstack variable defined alongside
the map.

Re: help debug hash_map garbage collection issue

[Marek Polacek]

On Tue, Apr 20, 2021 at 02:03:00PM -0600, Martin Sebor via Gcc wrote:
> I have a static hash_map object that needs to persist across passes:
> 
>   static GTY(()) hash_map<tree, bitmap> *map;
> 
> I initialize the map like so:
> 
>   map = hash_map<tree, bitmap>::create_ggc (4);
> 
> But I see crashes when accessing the map after adding and removing
> some number of entries in a few passes.  The crashes are due to
> the map having been garbage-collected and the memory allocated to
> it poisoned (it has the 0xa5a5a5a5a5a5a5a5 bit pattern that I see
> in GDD being written into the map by poison_pages()).
> 
> I assumed marking the map pointer GTY(()) would keep the garbage
> collector from touching the map.  Is there something else I need
> to do to keep it from doing that?
> 
> Thanks
> Martin
> 
> PS Just in case it matters, the bitmap in the table is allocated
> via BITMAP_ALLOC() with a bitmap_obstack variable defined alongside
> the map.

My sense is that this is the problem.  What happens if you use
BITMAP_GGC_ALLOC?

Marek

Re: help debug hash_map garbage collection issue

[Martin Sebor]

On 4/20/21 2:13 PM, Marek Polacek wrote:
> On Tue, Apr 20, 2021 at 02:03:00PM -0600, Martin Sebor via Gcc wrote:
>> I have a static hash_map object that needs to persist across passes:
>>
>>    static GTY(()) hash_map<tree, bitmap> *map;
>>
>> I initialize the map like so:
>>
>>    map = hash_map<tree, bitmap>::create_ggc (4);
>>
>> But I see crashes when accessing the map after adding and removing
>> some number of entries in a few passes.  The crashes are due to
>> the map having been garbage-collected and the memory allocated to
>> it poisoned (it has the 0xa5a5a5a5a5a5a5a5 bit pattern that I see
>> in GDD being written into the map by poison_pages()).
>>
>> I assumed marking the map pointer GTY(()) would keep the garbage
>> collector from touching the map.  Is there something else I need
>> to do to keep it from doing that?
>>
>> Thanks
>> Martin
>>
>> PS Just in case it matters, the bitmap in the table is allocated
>> via BITMAP_ALLOC() with a bitmap_obstack variable defined alongside
>> the map.
> 
> My sense is that this is the problem.  What happens if you use
> BITMAP_GGC_ALLOC?

Same thing :(

Martin

> 
> Marek
> 

Re: help debug hash_map garbage collection issue

[Martin Sebor]

On 4/20/21 2:36 PM, Martin Sebor wrote:
> On 4/20/21 2:13 PM, Marek Polacek wrote:
>> On Tue, Apr 20, 2021 at 02:03:00PM -0600, Martin Sebor via Gcc wrote:
>>> I have a static hash_map object that needs to persist across passes:
>>>
>>>    static GTY(()) hash_map<tree, bitmap> *map;
>>>
>>> I initialize the map like so:
>>>
>>>    map = hash_map<tree, bitmap>::create_ggc (4);
>>>
>>> But I see crashes when accessing the map after adding and removing
>>> some number of entries in a few passes.  The crashes are due to
>>> the map having been garbage-collected and the memory allocated to
>>> it poisoned (it has the 0xa5a5a5a5a5a5a5a5 bit pattern that I see
>>> in GDD being written into the map by poison_pages()).
>>>
>>> I assumed marking the map pointer GTY(()) would keep the garbage
>>> collector from touching the map.  Is there something else I need
>>> to do to keep it from doing that?
>>>
>>> Thanks
>>> Martin
>>>
>>> PS Just in case it matters, the bitmap in the table is allocated
>>> via BITMAP_ALLOC() with a bitmap_obstack variable defined alongside
>>> the map.
>>
>> My sense is that this is the problem.  What happens if you use
>> BITMAP_GGC_ALLOC?
> 
> Same thing :(

I reduced the problem to the following patch/test case no involving
a bitmap or other pointers.  With it applied, GCC reliably crashes.
An example of a stack trace is below the patch.  What am I missing?

diff --git a/gcc/gimple.c b/gcc/gimple.c
index 87864f3d0dd..0e6cded166e 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -2749,11 +2749,21 @@ gimple_call_operator_delete_p (const gcall *stmt)
    return false;
  }

+typedef int_hash <int, 0, INT_MAX> i_hash;
+
+static GTY(()) hash_map<i_hash, int> *ii_map;
+
  /* Return true when STMT is builtins call.  */

  bool
  gimple_call_builtin_p (const gimple *stmt)
  {
+  if (!ii_map)
+    ii_map = ii_map->create_ggc (4);
+
+  uintptr_t key = gimple_code (stmt);
+  ii_map->put (key, key);
+
    tree fndecl;
    if (is_gimple_call (stmt)
        && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE


during GIMPLE pass: cplxlower0
/src/gcc/master/gcc/testsuite/gcc.dg/builtins-1.c:97:55: internal 
compiler error: Segmentation fault
0x1308b30 crash_signal
	/src/gcc/master/gcc/toplev.c:327
0xe7eaa0 bool simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 
2147483647> >, int>::is_empty<hash_map<int_hash<int, 0, 2147483647>, 
int, simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 
2147483647> >, int> >::hash_entry>(hash_map<int_hash<int, 0, 
2147483647>, int, 
simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
 >, int> >::hash_entry const&)
	/src/gcc/master/gcc/hash-map-traits.h:75
0xe7e36a hash_map<int_hash<int, 0, 2147483647>, int, 
simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
 >, int> >::hash_entry::is_empty(hash_map<int_hash<int, 0, 2147483647>, 
int, simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 
2147483647> >, int> >::hash_entry const&)
	/src/gcc/master/gcc/hash-map.h:71
0xe7ea32 hash_table<hash_map<int_hash<int, 0, 2147483647>, int, 
simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
 >, int> >::hash_entry, false, 
xcallocator>::is_empty(hash_map<int_hash<int, 0, 2147483647>, int, 
simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
 >, int> >::hash_entry&)
	/src/gcc/master/gcc/hash-table.h:541
0xe7e89d hash_table<hash_map<int_hash<int, 0, 2147483647>, int, 
simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
 >, int> >::hash_entry, false, xcallocator>::expand()
	/src/gcc/master/gcc/hash-table.h:819
0xe7e155 hash_table<hash_map<int_hash<int, 0, 2147483647>, int, 
simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
 >, int> >::hash_entry, false, xcallocator>::find_slot_with_hash(int 
const&, unsigned int, insert_option)
	/src/gcc/master/gcc/hash-table.h:964
0xe7d9b8 hash_map<int_hash<int, 0, 2147483647>, int, 
simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
 >, int> >::put(int const&, int const&)
	/src/gcc/master/gcc/hash-map.h:166
0xe7932f gimple_call_builtin_p(gimple const*)
	/src/gcc/master/gcc/gimple.c:2766
0xece684 maybe_fold_stmt
	/src/gcc/master/gcc/gimplify.c:3303
0xed888e gimplify_modify_expr
	/src/gcc/master/gcc/gimplify.c:5994
0xf03d14 gimplify_expr(tree_node**, gimple**, gimple**, bool 
(*)(tree_node*), int)
	/src/gcc/master/gcc/gimplify.c:14083
0xedbf17 gimplify_stmt(tree_node**, gimple**)
	/src/gcc/master/gcc/gimplify.c:6877
0xec55f0 gimplify_and_add(tree_node*, gimple**)
	/src/gcc/master/gcc/gimplify.c:489
0xec5d8f internal_get_tmp_var
	/src/gcc/master/gcc/gimplify.c:642
0xec5dd6 get_formal_tmp_var(tree_node*, gimple**)
	/src/gcc/master/gcc/gimplify.c:663
0xf07a6b gimplify_expr(tree_node**, gimple**, gimple**, bool 
(*)(tree_node*), int)
	/src/gcc/master/gcc/gimplify.c:15072
0xf0d30a force_gimple_operand_1(tree_node*, gimple**, bool 
(*)(tree_node*), tree_node*)
	/src/gcc/master/gcc/gimplify-me.c:78
0xf0d3d7 force_gimple_operand_gsi_1(gimple_stmt_iterator*, tree_node*, 
bool (*)(tree_node*), tree_node*, bool, gsi_iterator_update)
	/src/gcc/master/gcc/gimplify-me.c:115
0xf0d490 force_gimple_operand_gsi(gimple_stmt_iterator*, tree_node*, 
bool, tree_node*, bool, gsi_iterator_update)
	/src/gcc/master/gcc/gimplify-me.c:141
0x13791c5 gimplify_build1(gimple_stmt_iterator*, tree_code, tree_node*, 
tree_node*)
	/src/gcc/master/gcc/tree-cfg.c:9249

Re: help debug hash_map garbage collection issue

[Martin Sebor]

On 4/20/21 4:15 PM, Martin Sebor wrote:
> On 4/20/21 2:36 PM, Martin Sebor wrote:
>> On 4/20/21 2:13 PM, Marek Polacek wrote:
>>> On Tue, Apr 20, 2021 at 02:03:00PM -0600, Martin Sebor via Gcc wrote:
>>>> I have a static hash_map object that needs to persist across passes:
>>>>
>>>>    static GTY(()) hash_map<tree, bitmap> *map;
>>>>
>>>> I initialize the map like so:
>>>>
>>>>    map = hash_map<tree, bitmap>::create_ggc (4);
>>>>
>>>> But I see crashes when accessing the map after adding and removing
>>>> some number of entries in a few passes.  The crashes are due to
>>>> the map having been garbage-collected and the memory allocated to
>>>> it poisoned (it has the 0xa5a5a5a5a5a5a5a5 bit pattern that I see
>>>> in GDD being written into the map by poison_pages()).
>>>>
>>>> I assumed marking the map pointer GTY(()) would keep the garbage
>>>> collector from touching the map.  Is there something else I need
>>>> to do to keep it from doing that?
>>>>
>>>> Thanks
>>>> Martin
>>>>
>>>> PS Just in case it matters, the bitmap in the table is allocated
>>>> via BITMAP_ALLOC() with a bitmap_obstack variable defined alongside
>>>> the map.
>>>
>>> My sense is that this is the problem.  What happens if you use
>>> BITMAP_GGC_ALLOC?
>>
>> Same thing :(
> 
> I reduced the problem to the following patch/test case no involving
> a bitmap or other pointers.  With it applied, GCC reliably crashes.
> An example of a stack trace is below the patch.  What am I missing?

It seems that assigning the address of an object allocated by
ggc_alloc() (which presumably includes BITMAP_GGC_ALLOC()) to
a GTY-marked pointer "defeats" the purpose of the marker and
allows it to be garbage collected.  I suppose it's the spirit
of "garbage in, garbage out" (pun intended).  Since (if?)
the combination makes no sense I shouldn't be too surprised
that it results in a crash.

If it really doesn't make sense, it would be really nice if
the compiler warned about it.  I suppose expanding the GTY marker
to some GCC-internal attribute would make it possible to detect.

Alternatively, it would be helpful if this "gotcha" was at
least documented, both in the code and in the manual.  That
would have saved me a day's worth of frustrating debugging.
Let me put something together.

> 
> diff --git a/gcc/gimple.c b/gcc/gimple.c
> index 87864f3d0dd..0e6cded166e 100644
> --- a/gcc/gimple.c
> +++ b/gcc/gimple.c
> @@ -2749,11 +2749,21 @@ gimple_call_operator_delete_p (const gcall *stmt)
>     return false;
>   }
> 
> +typedef int_hash <int, 0, INT_MAX> i_hash;
> +
> +static GTY(()) hash_map<i_hash, int> *ii_map;
> +
>   /* Return true when STMT is builtins call.  */
> 
>   bool
>   gimple_call_builtin_p (const gimple *stmt)
>   {
> +  if (!ii_map)
> +    ii_map = ii_map->create_ggc (4);
> +
> +  uintptr_t key = gimple_code (stmt);
> +  ii_map->put (key, key);
> +
>     tree fndecl;
>     if (is_gimple_call (stmt)
>         && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
> 
> 
> during GIMPLE pass: cplxlower0
> /src/gcc/master/gcc/testsuite/gcc.dg/builtins-1.c:97:55: internal 
> compiler error: Segmentation fault
> 0x1308b30 crash_signal
>      /src/gcc/master/gcc/toplev.c:327
> 0xe7eaa0 bool simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 
> 2147483647> >, int>::is_empty<hash_map<int_hash<int, 0, 2147483647>, 
> int, simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 
> 2147483647> >, int> >::hash_entry>(hash_map<int_hash<int, 0, 
> 2147483647>, int, 
> simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
>  >, int> >::hash_entry const&)
>      /src/gcc/master/gcc/hash-map-traits.h:75
> 0xe7e36a hash_map<int_hash<int, 0, 2147483647>, int, 
> simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
>  >, int> >::hash_entry::is_empty(hash_map<int_hash<int, 0, 2147483647>, 
> int, simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 
> 2147483647> >, int> >::hash_entry const&)
>      /src/gcc/master/gcc/hash-map.h:71
> 0xe7ea32 hash_table<hash_map<int_hash<int, 0, 2147483647>, int, 
> simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
>  >, int> >::hash_entry, false, 
> xcallocator>::is_empty(hash_map<int_hash<int, 0, 2147483647>, int, 
> simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
>  >, int> >::hash_entry&)
>      /src/gcc/master/gcc/hash-table.h:541
> 0xe7e89d hash_table<hash_map<int_hash<int, 0, 2147483647>, int, 
> simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
>  >, int> >::hash_entry, false, xcallocator>::expand()
>      /src/gcc/master/gcc/hash-table.h:819
> 0xe7e155 hash_table<hash_map<int_hash<int, 0, 2147483647>, int, 
> simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
>  >, int> >::hash_entry, false, xcallocator>::find_slot_with_hash(int 
> const&, unsigned int, insert_option)
>      /src/gcc/master/gcc/hash-table.h:964
> 0xe7d9b8 hash_map<int_hash<int, 0, 2147483647>, int, 
> simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
>  >, int> >::put(int const&, int const&)
>      /src/gcc/master/gcc/hash-map.h:166
> 0xe7932f gimple_call_builtin_p(gimple const*)
>      /src/gcc/master/gcc/gimple.c:2766
> 0xece684 maybe_fold_stmt
>      /src/gcc/master/gcc/gimplify.c:3303
> 0xed888e gimplify_modify_expr
>      /src/gcc/master/gcc/gimplify.c:5994
> 0xf03d14 gimplify_expr(tree_node**, gimple**, gimple**, bool 
> (*)(tree_node*), int)
>      /src/gcc/master/gcc/gimplify.c:14083
> 0xedbf17 gimplify_stmt(tree_node**, gimple**)
>      /src/gcc/master/gcc/gimplify.c:6877
> 0xec55f0 gimplify_and_add(tree_node*, gimple**)
>      /src/gcc/master/gcc/gimplify.c:489
> 0xec5d8f internal_get_tmp_var
>      /src/gcc/master/gcc/gimplify.c:642
> 0xec5dd6 get_formal_tmp_var(tree_node*, gimple**)
>      /src/gcc/master/gcc/gimplify.c:663
> 0xf07a6b gimplify_expr(tree_node**, gimple**, gimple**, bool 
> (*)(tree_node*), int)
>      /src/gcc/master/gcc/gimplify.c:15072
> 0xf0d30a force_gimple_operand_1(tree_node*, gimple**, bool 
> (*)(tree_node*), tree_node*)
>      /src/gcc/master/gcc/gimplify-me.c:78
> 0xf0d3d7 force_gimple_operand_gsi_1(gimple_stmt_iterator*, tree_node*, 
> bool (*)(tree_node*), tree_node*, bool, gsi_iterator_update)
>      /src/gcc/master/gcc/gimplify-me.c:115
> 0xf0d490 force_gimple_operand_gsi(gimple_stmt_iterator*, tree_node*, 
> bool, tree_node*, bool, gsi_iterator_update)
>      /src/gcc/master/gcc/gimplify-me.c:141
> 0x13791c5 gimplify_build1(gimple_stmt_iterator*, tree_code, tree_node*, 
> tree_node*)
>      /src/gcc/master/gcc/tree-cfg.c:9249
> 

Re: help debug hash_map garbage collection issue

[Martin Sebor]

On 4/20/21 5:03 PM, Martin Sebor wrote:
> On 4/20/21 4:15 PM, Martin Sebor wrote:
>> On 4/20/21 2:36 PM, Martin Sebor wrote:
>>> On 4/20/21 2:13 PM, Marek Polacek wrote:
>>>> On Tue, Apr 20, 2021 at 02:03:00PM -0600, Martin Sebor via Gcc wrote:
>>>>> I have a static hash_map object that needs to persist across passes:
>>>>>
>>>>>    static GTY(()) hash_map<tree, bitmap> *map;
>>>>>
>>>>> I initialize the map like so:
>>>>>
>>>>>    map = hash_map<tree, bitmap>::create_ggc (4);
>>>>>
>>>>> But I see crashes when accessing the map after adding and removing
>>>>> some number of entries in a few passes.  The crashes are due to
>>>>> the map having been garbage-collected and the memory allocated to
>>>>> it poisoned (it has the 0xa5a5a5a5a5a5a5a5 bit pattern that I see
>>>>> in GDD being written into the map by poison_pages()).
>>>>>
>>>>> I assumed marking the map pointer GTY(()) would keep the garbage
>>>>> collector from touching the map.  Is there something else I need
>>>>> to do to keep it from doing that?
>>>>>
>>>>> Thanks
>>>>> Martin
>>>>>
>>>>> PS Just in case it matters, the bitmap in the table is allocated
>>>>> via BITMAP_ALLOC() with a bitmap_obstack variable defined alongside
>>>>> the map.
>>>>
>>>> My sense is that this is the problem.  What happens if you use
>>>> BITMAP_GGC_ALLOC?
>>>
>>> Same thing :(
>>
>> I reduced the problem to the following patch/test case no involving
>> a bitmap or other pointers.  With it applied, GCC reliably crashes.
>> An example of a stack trace is below the patch.  What am I missing?
> 
> It seems that assigning the address of an object allocated by
> ggc_alloc() (which presumably includes BITMAP_GGC_ALLOC()) to
> a GTY-marked pointer "defeats" the purpose of the marker and
> allows it to be garbage collected.

But this is not true.  There are plenty of counterexamples, including
in tree.c which defines several GTY hash_tables (but no maps).  I tried
moving the test case below from gimple.c to tree.c but there it doesn't
even compile.  I get this error (and a couple of others):

In file included from /src/gcc/master/gcc/tree.c:16179:
./gt-tree.h: In function ‘void gt_ggc_mx(int_hash<int, 0, 2147483647>&)’:
./gt-tree.h:156:21: error: no matching function for call to 
‘gt_ggc_mx(int_hash<int, 0, 2147483647>*)’
    gt_ggc_mx (&((*x)));
                      ^
So as another experiment I moved it builtins.c where it does compile
(go figure) but then it crashes when I call it the same way from
c_strlen().

(Aren't you glad you tried to help? ;)

> I suppose it's the spirit
> of "garbage in, garbage out" (pun intended).  Since (if?)
> the combination makes no sense I shouldn't be too surprised
> that it results in a crash.
> 
> If it really doesn't make sense, it would be really nice if
> the compiler warned about it.  I suppose expanding the GTY marker
> to some GCC-internal attribute would make it possible to detect.
> 
> Alternatively, it would be helpful if this "gotcha" was at
> least documented, both in the code and in the manual.  That
> would have saved me a day's worth of frustrating debugging.
> Let me put something together.
> 
>>
>> diff --git a/gcc/gimple.c b/gcc/gimple.c
>> index 87864f3d0dd..0e6cded166e 100644
>> --- a/gcc/gimple.c
>> +++ b/gcc/gimple.c
>> @@ -2749,11 +2749,21 @@ gimple_call_operator_delete_p (const gcall *stmt)
>>     return false;
>>   }
>>
>> +typedef int_hash <int, 0, INT_MAX> i_hash;
>> +
>> +static GTY(()) hash_map<i_hash, int> *ii_map;
>> +
>>   /* Return true when STMT is builtins call.  */
>>
>>   bool
>>   gimple_call_builtin_p (const gimple *stmt)
>>   {
>> +  if (!ii_map)
>> +    ii_map = ii_map->create_ggc (4);
>> +
>> +  uintptr_t key = gimple_code (stmt);
>> +  ii_map->put (key, key);
>> +
>>     tree fndecl;
>>     if (is_gimple_call (stmt)
>>         && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
>>
>>
>> during GIMPLE pass: cplxlower0
>> /src/gcc/master/gcc/testsuite/gcc.dg/builtins-1.c:97:55: internal 
>> compiler error: Segmentation fault
>> 0x1308b30 crash_signal
>>      /src/gcc/master/gcc/toplev.c:327
>> 0xe7eaa0 bool simple_hashmap_traits<default_hash_traits<int_hash<int, 
>> 0, 2147483647> >, int>::is_empty<hash_map<int_hash<int, 0, 
>> 2147483647>, int, 
>> simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
>> >, int> >::hash_entry>(hash_map<int_hash<int, 0, 2147483647>, int, 
>> simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
>>  >, int> >::hash_entry const&)
>>      /src/gcc/master/gcc/hash-map-traits.h:75
>> 0xe7e36a hash_map<int_hash<int, 0, 2147483647>, int, 
>> simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
>>  >, int> >::hash_entry::is_empty(hash_map<int_hash<int, 0, 
>> 2147483647>, int, 
>> simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
>> >, int> >::hash_entry const&)
>>      /src/gcc/master/gcc/hash-map.h:71
>> 0xe7ea32 hash_table<hash_map<int_hash<int, 0, 2147483647>, int, 
>> simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
>>  >, int> >::hash_entry, false, 
>> xcallocator>::is_empty(hash_map<int_hash<int, 0, 2147483647>, int, 
>> simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
>>  >, int> >::hash_entry&)
>>      /src/gcc/master/gcc/hash-table.h:541
>> 0xe7e89d hash_table<hash_map<int_hash<int, 0, 2147483647>, int, 
>> simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
>>  >, int> >::hash_entry, false, xcallocator>::expand()
>>      /src/gcc/master/gcc/hash-table.h:819
>> 0xe7e155 hash_table<hash_map<int_hash<int, 0, 2147483647>, int, 
>> simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
>>  >, int> >::hash_entry, false, xcallocator>::find_slot_with_hash(int 
>> const&, unsigned int, insert_option)
>>      /src/gcc/master/gcc/hash-table.h:964
>> 0xe7d9b8 hash_map<int_hash<int, 0, 2147483647>, int, 
>> simple_hashmap_traits<default_hash_traits<int_hash<int, 0, 2147483647> 
>>  >, int> >::put(int const&, int const&)
>>      /src/gcc/master/gcc/hash-map.h:166
>> 0xe7932f gimple_call_builtin_p(gimple const*)
>>      /src/gcc/master/gcc/gimple.c:2766
>> 0xece684 maybe_fold_stmt
>>      /src/gcc/master/gcc/gimplify.c:3303
>> 0xed888e gimplify_modify_expr
>>      /src/gcc/master/gcc/gimplify.c:5994
>> 0xf03d14 gimplify_expr(tree_node**, gimple**, gimple**, bool 
>> (*)(tree_node*), int)
>>      /src/gcc/master/gcc/gimplify.c:14083
>> 0xedbf17 gimplify_stmt(tree_node**, gimple**)
>>      /src/gcc/master/gcc/gimplify.c:6877
>> 0xec55f0 gimplify_and_add(tree_node*, gimple**)
>>      /src/gcc/master/gcc/gimplify.c:489
>> 0xec5d8f internal_get_tmp_var
>>      /src/gcc/master/gcc/gimplify.c:642
>> 0xec5dd6 get_formal_tmp_var(tree_node*, gimple**)
>>      /src/gcc/master/gcc/gimplify.c:663
>> 0xf07a6b gimplify_expr(tree_node**, gimple**, gimple**, bool 
>> (*)(tree_node*), int)
>>      /src/gcc/master/gcc/gimplify.c:15072
>> 0xf0d30a force_gimple_operand_1(tree_node*, gimple**, bool 
>> (*)(tree_node*), tree_node*)
>>      /src/gcc/master/gcc/gimplify-me.c:78
>> 0xf0d3d7 force_gimple_operand_gsi_1(gimple_stmt_iterator*, tree_node*, 
>> bool (*)(tree_node*), tree_node*, bool, gsi_iterator_update)
>>      /src/gcc/master/gcc/gimplify-me.c:115
>> 0xf0d490 force_gimple_operand_gsi(gimple_stmt_iterator*, tree_node*, 
>> bool, tree_node*, bool, gsi_iterator_update)
>>      /src/gcc/master/gcc/gimplify-me.c:141
>> 0x13791c5 gimplify_build1(gimple_stmt_iterator*, tree_code, 
>> tree_node*, tree_node*)
>>      /src/gcc/master/gcc/tree-cfg.c:9249
>>
> 

Re: help debug hash_map garbage collection issue

[Jason Merrill]

On Tue, Apr 20, 2021 at 8:31 PM Martin Sebor via Gcc <gcc@gcc.gnu.org> wrote:
>
> On 4/20/21 5:03 PM, Martin Sebor wrote:
> > On 4/20/21 4:15 PM, Martin Sebor wrote:
> >> On 4/20/21 2:36 PM, Martin Sebor wrote:
> >>> On 4/20/21 2:13 PM, Marek Polacek wrote:
> >>>> On Tue, Apr 20, 2021 at 02:03:00PM -0600, Martin Sebor via Gcc wrote:
> >>>>> I have a static hash_map object that needs to persist across passes:
> >>>>>
> >>>>>    static GTY(()) hash_map<tree, bitmap> *map;
> >>>>>
> >>>>> I initialize the map like so:
> >>>>>
> >>>>>    map = hash_map<tree, bitmap>::create_ggc (4);
> >>>>>
> >>>>> But I see crashes when accessing the map after adding and removing
> >>>>> some number of entries in a few passes.  The crashes are due to
> >>>>> the map having been garbage-collected and the memory allocated to
> >>>>> it poisoned (it has the 0xa5a5a5a5a5a5a5a5 bit pattern that I see
> >>>>> in GDD being written into the map by poison_pages()).
> >>>>>
> >>>>> I assumed marking the map pointer GTY(()) would keep the garbage
> >>>>> collector from touching the map.  Is there something else I need
> >>>>> to do to keep it from doing that?
> >>>>>
> >>>>> Thanks
> >>>>> Martin
> >>>>>
> >>>>> PS Just in case it matters, the bitmap in the table is allocated
> >>>>> via BITMAP_ALLOC() with a bitmap_obstack variable defined alongside
> >>>>> the map.
> >>>>
> >>>> My sense is that this is the problem.  What happens if you use
> >>>> BITMAP_GGC_ALLOC?
> >>>
> >>> Same thing :(
> >>
> >> I reduced the problem to the following patch/test case no involving
> >> a bitmap or other pointers.  With it applied, GCC reliably crashes.
> >> An example of a stack trace is below the patch.  What am I missing?
> >
> > It seems that assigning the address of an object allocated by
> > ggc_alloc() (which presumably includes BITMAP_GGC_ALLOC()) to
> > a GTY-marked pointer "defeats" the purpose of the marker and
> > allows it to be garbage collected.
>
> But this is not true.  There are plenty of counterexamples, including
> in tree.c which defines several GTY hash_tables (but no maps).  I tried
> moving the test case below from gimple.c to tree.c but there it doesn't
> even compile.  I get this error (and a couple of others):
>
> In file included from /src/gcc/master/gcc/tree.c:16179:
> ./gt-tree.h: In function ‘void gt_ggc_mx(int_hash<int, 0, 2147483647>&)’:
> ./gt-tree.h:156:21: error: no matching function for call to
> ‘gt_ggc_mx(int_hash<int, 0, 2147483647>*)’
>     gt_ggc_mx (&((*x)));
>                       ^
> So as another experiment I moved it builtins.c where it does compile
> (go figure) but then it crashes when I call it the same way from
> c_strlen().

This is because tree.c is in GTFILES, while gimple.c and tree.c are
not, so gengtype never sees the GTY decoration in the latter files.

I don't know why when it does see the GTY, it ends up trying to mark
an int_hash*.

Jason

Re: help debug hash_map garbage collection issue

[Martin Sebor]

[-- Attachment #1: Type: text/plain, Size: 5258 bytes --]

On 4/20/21 8:26 PM, Jason Merrill wrote:
> On Tue, Apr 20, 2021 at 8:31 PM Martin Sebor via Gcc <gcc@gcc.gnu.org> wrote:
>>
>> On 4/20/21 5:03 PM, Martin Sebor wrote:
>>> On 4/20/21 4:15 PM, Martin Sebor wrote:
>>>> On 4/20/21 2:36 PM, Martin Sebor wrote:
>>>>> On 4/20/21 2:13 PM, Marek Polacek wrote:
>>>>>> On Tue, Apr 20, 2021 at 02:03:00PM -0600, Martin Sebor via Gcc wrote:
>>>>>>> I have a static hash_map object that needs to persist across passes:
>>>>>>>
>>>>>>>     static GTY(()) hash_map<tree, bitmap> *map;
>>>>>>>
>>>>>>> I initialize the map like so:
>>>>>>>
>>>>>>>     map = hash_map<tree, bitmap>::create_ggc (4);
>>>>>>>
>>>>>>> But I see crashes when accessing the map after adding and removing
>>>>>>> some number of entries in a few passes.  The crashes are due to
>>>>>>> the map having been garbage-collected and the memory allocated to
>>>>>>> it poisoned (it has the 0xa5a5a5a5a5a5a5a5 bit pattern that I see
>>>>>>> in GDD being written into the map by poison_pages()).
>>>>>>>
>>>>>>> I assumed marking the map pointer GTY(()) would keep the garbage
>>>>>>> collector from touching the map.  Is there something else I need
>>>>>>> to do to keep it from doing that?
>>>>>>>
>>>>>>> Thanks
>>>>>>> Martin
>>>>>>>
>>>>>>> PS Just in case it matters, the bitmap in the table is allocated
>>>>>>> via BITMAP_ALLOC() with a bitmap_obstack variable defined alongside
>>>>>>> the map.
>>>>>>
>>>>>> My sense is that this is the problem.  What happens if you use
>>>>>> BITMAP_GGC_ALLOC?
>>>>>
>>>>> Same thing :(
>>>>
>>>> I reduced the problem to the following patch/test case no involving
>>>> a bitmap or other pointers.  With it applied, GCC reliably crashes.
>>>> An example of a stack trace is below the patch.  What am I missing?
>>>
>>> It seems that assigning the address of an object allocated by
>>> ggc_alloc() (which presumably includes BITMAP_GGC_ALLOC()) to
>>> a GTY-marked pointer "defeats" the purpose of the marker and
>>> allows it to be garbage collected.
>>
>> But this is not true.  There are plenty of counterexamples, including
>> in tree.c which defines several GTY hash_tables (but no maps).  I tried
>> moving the test case below from gimple.c to tree.c but there it doesn't
>> even compile.  I get this error (and a couple of others):
>>
>> In file included from /src/gcc/master/gcc/tree.c:16179:
>> ./gt-tree.h: In function ‘void gt_ggc_mx(int_hash<int, 0, 2147483647>&)’:
>> ./gt-tree.h:156:21: error: no matching function for call to
>> ‘gt_ggc_mx(int_hash<int, 0, 2147483647>*)’
>>      gt_ggc_mx (&((*x)));
>>                        ^
>> So as another experiment I moved it builtins.c where it does compile
>> (go figure) but then it crashes when I call it the same way from
>> c_strlen().
> 
> This is because tree.c is in GTFILES, while gimple.c and tree.c are
> not, so gengtype never sees the GTY decoration in the latter files.
> 
> I don't know why when it does see the GTY, it ends up trying to mark
> an int_hash*.

Thanks for the hint.  I didn't notice this in the manual before
so here are the steps to use GTY in file.c for reference:

1) add file.c to GTFILES in Makefile.in
2) add #include "gt-file.h" to the end of file.c
3) reconfigure + rebuild

I think I also finally figured out the compilation errors and ICEs,
after over two days of wrestling with it.  It's seems like hash_map
support for GTY is incomplete, as is ggc.h.  I suspect the same
problems as in hash_map are going to be in other hash-based containers.

(A gotcha that leads to cryptic errors that can add to the confusion
here is removing the GTY type from the source file as the gt-file.c
still refers to it.  I dealt with it by reconfiguring again.  Maybe
there's a better way.)

The reason for the error above (and others like it) is simply that
there's no gt_ggc_mx() defined for int_hash.  The only gt_ function
documented in the manual that is defined (in the generated gt-foo.h)
is the three-argument overload of gt_pcx_nx().

But that's not the only missing piece or problem.

The gt_*() function templates in hash-map.h are defined static so
(IIRC from my early C++ days) they're not considered by ADL.  That
leads to more compilation errors.

Also, ggc.h defines a few gt_*() overloads for a subset of integer
types but not nearly for all, so that leads to even more errors for
instantiations of int_hash on different integer types (e.g.,
uintptr_t).

Adding the full set of overloads (including, for instance, short)
leads to still more problems for int_hash specializations on types
like short.

/src/gcc/master/gcc/hash-map.h:107:14: error: no matching function for 
call to ‘gt_pch_nx(short int*, void (*&)(void*, void*), void*&)’
     gt_pch_nx (&x, op, cookie);
     ~~~~~~~~~~^~~~~~~~~~~~~~~~

Similar to ggc.h, there is a small subset of no-op overloads like
this in hash_map but not for types like short.

The attached POC patch fixes this (foo can be called from anywhere).
I haven't done a full bootstrap with it yet or looked at hash_set or
but please let me know if you see any issues with it.  I'll submit
a complete patch once I'm done with it.

Martin

PS For the overloads I resisted introducing SFINAE to keep things
simple and avoid pulling in <type_traits> (among other things).

[-- Attachment #2: foo.diff --]
[-- Type: text/x-patch, Size: 4525 bytes --]

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 8a5fb3fd99c..9ff8d1596f2 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1373,6 +1373,7 @@ OBJS = \
 	fixed-value.o \
 	fold-const.o \
 	fold-const-call.o \
+	foo.o \
 	function.o \
 	function-abi.o \
 	function-tests.o \
@@ -2662,6 +2663,7 @@ GTFILES = $(CPPLIB_H) $(srcdir)/input.h $(srcdir)/coretypes.h \
   $(srcdir)/dojump.c $(srcdir)/emit-rtl.h \
   $(srcdir)/emit-rtl.c $(srcdir)/except.h $(srcdir)/explow.c $(srcdir)/expr.c \
   $(srcdir)/expr.h \
+  $(srcdir)/foo.c \
   $(srcdir)/function.c $(srcdir)/except.c \
   $(srcdir)/ggc-tests.c \
   $(srcdir)/gcse.c $(srcdir)/godump.c \
diff --git a/gcc/foo.c b/gcc/foo.c
new file mode 100644
index 00000000000..ea9cebfd9b2
--- /dev/null
+++ b/gcc/foo.c
@@ -0,0 +1,35 @@
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "backend.h"
+#include "tree.h"
+#include "hash-map.h"
+
+// typedef int_hash<char, 0, CHAR_MAX> UintPtrHash;
+// typedef int_hash<short, 0, SHRT_MAX> UintPtrHash;
+// typedef int_hash<int, 0, INT_MAX> UintPtrHash;
+// typedef int_hash<long, 0, LONG_MAX> UintPtrHash;
+// typedef int_hash<intptr_t, 0, INTPTR_MAX> UintPtrHash;
+typedef int_hash<uintptr_t, 0, UINTPTR_MAX> UintPtrHash;
+
+inline void
+gt_ggc_mx (UintPtrHash *) { }
+
+inline void
+gt_pch_nx (UintPtrHash *) { }
+
+typedef hash_map<UintPtrHash, tree> UintPtrHashMap;
+static GTY(()) UintPtrHashMap *tree_map;
+
+void
+foo (tree x)
+{
+  if (!tree_map)
+    tree_map = tree_map->create_ggc (1);
+
+  tree_map->put ((uintptr_t)x, x);
+  if (tree_map->elements () > 1024)
+    tree_map->empty ();
+}
+
+#include "gt-foo.h"
diff --git a/gcc/ggc.h b/gcc/ggc.h
index 65f6cb4d19d..ad0e7488011 100644
--- a/gcc/ggc.h
+++ b/gcc/ggc.h
@@ -332,19 +332,24 @@ gt_pch_nx (const char *)
 {
 }
 
-inline void
-gt_ggc_mx (int)
-{
-}
-
-inline void
-gt_pch_nx (int)
-{
-}
-
-inline void
-gt_pch_nx (unsigned int)
-{
-}
+#define DEFINE_GT_HELPERS(T)				\
+  inline void gt_ggc_mx (T) { }				\
+  inline void gt_pch_nx (T) { }				\
+  typedef void unused_ggc_type /* expect semicolon */
+
+DEFINE_GT_HELPERS (bool);
+DEFINE_GT_HELPERS (char);
+DEFINE_GT_HELPERS (signed char);
+DEFINE_GT_HELPERS (unsigned char);
+DEFINE_GT_HELPERS (short);
+DEFINE_GT_HELPERS (unsigned short);
+DEFINE_GT_HELPERS (int);
+DEFINE_GT_HELPERS (unsigned int);
+DEFINE_GT_HELPERS (long);
+DEFINE_GT_HELPERS (unsigned long);
+DEFINE_GT_HELPERS (long long);
+DEFINE_GT_HELPERS (unsigned long long);
+
+#undef DEFINE_GT_HELPERS
 
 #endif
diff --git a/gcc/hash-map.h b/gcc/hash-map.h
index 0779c930f0a..60f6f50296a 100644
--- a/gcc/hash-map.h
+++ b/gcc/hash-map.h
@@ -107,27 +107,31 @@ class GTY((user)) hash_map
 	  gt_pch_nx (&x, op, cookie);
 	}
 
-    static void
-      pch_nx_helper (int, gt_pointer_operator, void *)
-	{
-	}
-
-    static void
-      pch_nx_helper (unsigned int, gt_pointer_operator, void *)
-	{
-	}
-
-    static void
-      pch_nx_helper (bool, gt_pointer_operator, void *)
-	{
-	}
-
     template<typename T>
       static void
       pch_nx_helper (T *&x, gt_pointer_operator op, void *cookie)
 	{
 	  op (&x, cookie);
 	}
+
+    /* The overloads below should match those in ggc.h.  */
+#define DEFINE_PCH_HELPER(T)			\
+    static void pch_nx_helper (T, gt_pointer_operator, void *) { }
+
+    DEFINE_PCH_HELPER (bool);
+    DEFINE_PCH_HELPER (char);
+    DEFINE_PCH_HELPER (signed char);
+    DEFINE_PCH_HELPER (unsigned char);
+    DEFINE_PCH_HELPER (short);
+    DEFINE_PCH_HELPER (unsigned short);
+    DEFINE_PCH_HELPER (int);
+    DEFINE_PCH_HELPER (unsigned int);
+    DEFINE_PCH_HELPER (long);
+    DEFINE_PCH_HELPER (unsigned long);
+    DEFINE_PCH_HELPER (long long);
+    DEFINE_PCH_HELPER (unsigned long long);
+
+#undef DEFINE_PCH_HELPER
   };
 
 public:
@@ -301,21 +305,21 @@ private:
 /* ggc marking routines.  */
 
 template<typename K, typename V, typename H>
-static inline void
+inline void
 gt_ggc_mx (hash_map<K, V, H> *h)
 {
   gt_ggc_mx (&h->m_table);
 }
 
 template<typename K, typename V, typename H>
-static inline void
+inline void
 gt_pch_nx (hash_map<K, V, H> *h)
 {
   gt_pch_nx (&h->m_table);
 }
 
 template<typename K, typename V, typename H>
-static inline void
+inline void
 gt_cleare_cache (hash_map<K, V, H> *h)
 {
   if (h)
@@ -323,7 +327,7 @@ gt_cleare_cache (hash_map<K, V, H> *h)
 }
 
 template<typename K, typename V, typename H>
-static inline void
+inline void
 gt_pch_nx (hash_map<K, V, H> *h, gt_pointer_operator op, void *cookie)
 {
   op (&h->m_table.m_entries, cookie);