Re: PATCH fix heisenbug in cp/spew.c

[Geoff Keating <geoffk at geoffk dot org>]

> Date: Mon, 29 Jul 2002 11:58:57 +0100
> From: Richard Earnshaw <rearnsha@arm.com>

> 1) When we are recursing through the tree structures we can get a *very* 
> deep stack (in the case I was debugging there were more than 3000 frames 
> on the stack) -- is there any way we could tune the autogenerated 
> tree-walking code to tail-call itself -- ideally with a marker in the 
> structure to indicate the most beneficial tail-call pointer?

This is being worked on in the branch.  (I want to first add code to
generate statistics about how deep the stack gets in the first place,
so I know how much the change actually helps.)

> 2) We have a lot of cases in the autogenerated memory-walking code where 
> we have sequences like:
> 
>     unsigned int tag1 = (((*x)).yychar);
>     if (tag1 == (NSNAME)) {
>       gt_ggc_m_tree_node ((*x).yylval.ttype);
>     }
>     if (tag1 == (PTYPENAME_DEFN)) {
>       gt_ggc_m_tree_node ((*x).yylval.ttype);
>     }
>     if (tag1 == (TYPENAME_DEFN)) {
>       gt_ggc_m_tree_node ((*x).yylval.ttype);
>     }
>     if (tag1 == (IDENTIFIER_DEFN)) {
>       gt_ggc_m_tree_node ((*x).yylval.ttype);
>     }
>     if (tag1 == (PRE_PARSED_FUNCTION_DECL)) {
>       gt_ggc_m_unparsed_text ((*x).yylval.pi);
>     }
>     if (tag1 == (DEFARG_MARKER)) {
>       gt_ggc_m_tree_node ((*x).yylval.ttype);
>     }
>     if (tag1 == (DEFARG)) {
> 	...
> 
> This should be done as else-if tests, since it is clear that not more than 
> one can be true.  Better still might be a switch -- gcc could then 
> optimize small tables into if-else chains.  GCC with -O will probably 
> simplify the above, but the first stage of the bootstrap will certainly 
> not do this optimization, which means that we have to test every case.

This is changed already in the branch.

> 3) When checking is enabled can we make the tree marking code check 
> (cheaply) that memory it is marking is not marked as unallocated -- I 
> don't know if it would have found this bug, but it might have caused a 
> diagnostic fault to have triggered more often than the above conditions 
> would have occurred.

That seems to be tricky, because we don't save the old use counts of
the page.

This is supposed to be dealt with by having the freeing code set all
free memory to a bit pattern that should cause a crash.  I wonder why
that didn't find this bug earlier?

> Bootstrapped on i686-pc-linux-gnu with a regtest (C++ only) and also 
> verified to have fixed the original testsuite failure on 
> arm-unknown-linux-gnu.
> 
> Fixed with:
> 
> 2002-07-29  Richard Earnshaw  <rearnsha@arm.com>
> 
> 	* spew.c (space_for_token): Allocate zeroed memory for a new token
> 	chunk.

This is OK.

-- 
- Geoffrey Keating <geoffk@geoffk.org> <geoffk@redhat.com>