Bug 31230 - debug information depends on gc parameters
Summary: debug information depends on gc parameters
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: debug (show other bugs)
Version: 4.3.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: GC, wrong-debug
Depends on:
Blocks:
 
Reported: 2007-03-16 20:25 UTC by Joseph S. Myers
Modified: 2022-12-01 01:42 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2007-03-30 22:44:06


Attachments
Testcase cut down from dbxout.i in original bootstrap failure. (84.85 KB, text/plain)
2007-03-16 20:26 UTC, Joseph S. Myers
Details
Add equate_type_number_to_die call to prevent garbage collection. (407 bytes, patch)
2007-03-30 22:49 UTC, Jim Wilson
Details | Diff
minimal test case of 14 lines, cut down from varasm.i (75 bytes, text/plain)
2010-06-21 10:20 UTC, Tom de Vries
Details
naive patch. run callbacks on hashtable entries exhaustively before deleting (684 bytes, patch)
2010-06-21 10:32 UTC, Tom de Vries
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph S. Myers 2007-03-16 20:25:28 UTC
The following problem can cause bootstrap comparison failures in some circumstances (and was originally observed when bootstrapping with CFLAGS=-g BOOT_CFLAGS=-g).

Consider the code from tree.c:

/* See if the data pointed to by the type hash table is marked.  We consider
   it marked if the type is marked or if a debug type number or symbol
   table entry has been made for the type.  This reduces the amount of
   debugging output and eliminates that dependency of the debug output on
   the number of garbage collections.  */

static int
type_hash_marked_p (const void *p)
{
  tree type = ((struct type_hash *) p)->type;

  return ggc_marked_p (type) || TYPE_SYMTAB_POINTER (type);
}

(TYPE_SYMTAB_POINTER, TYPE_SYMTAB_ADDRESS and TYPE_SYMTAB_DIE all name 
fields in the same union; the one applicable depends on the debug format 
in use.)

The array type char[100] is generated, and handled for debug info 
purposes.  Some time later, char[100] arises again.

If there was no garbage collection inbetween, the same type is used and 
the debug info is shared.  If however there was a garbage collection, the 
index type (the special integer type with range 0 to 99 used for 
TYPE_DOMAIN of the array type) gets garbage collected - while the array 
type is no doubt marked for debug info, the index type (which doesn't 
appear directly in debug info) isn't.  Thus for the new char[100] type a 
new index type is generated, and so a new array type, and so extra debug 
info.

I think there may also be other conditions required for the types to be garbage collected and to trigger this bug.  I'll attach a .i file that shows it on i686-pc-linux-gnu if you compare the results of compiling with -g --param ggc-min-expand=0 --param ggc-min-heapsize=0 and -g --param ggc-min-expand=100 --param ggc-min-heapsize=131072, but it seems quite fragile.
Comment 1 Joseph S. Myers 2007-03-16 20:26:40 UTC
Created attachment 13215 [details]
Testcase cut down from dbxout.i in original bootstrap failure.
Comment 2 Jim Wilson 2007-03-30 22:44:06 UTC
I can reproduce the problem using the provided testcase.

It looks like all we have to do is mark the array type TYPE_DOMAIN as used, to prevent it from being garbage collected.  This just requires adding an equate_type_number_to_die call, which should be harmless by itself, I think.  This  solves the problem for the provided testcase.  I haven't done a bootstrap or gdb testsuite run to test the patch yet.

It might be nice to try to reuse this info, via lookup_type_die, now that we have saved it.  Unfortunately, the dwarf3 standard doesn't have any provision for this.  
Comment 3 Jim Wilson 2007-03-30 22:49:20 UTC
Created attachment 13304 [details]
Add equate_type_number_to_die call to prevent garbage collection.

This patch is untested, but works for the testcase in the bug report.
Comment 4 Tom de Vries 2010-06-21 10:20:00 UTC
Created attachment 20953 [details]
minimal test case of 14 lines, cut down from varasm.i

I also ran into this bug, while building gcc 4.3.5 for x86_64-unknown-linux-gnu with make {CFLAGS,BOOT_CFLAGS,STAGE1_CFLAGS}=\"-g3\ -O0\ -dH\".

I managed to minimized the test case down to 14 lines.

The difference in debug info can be reproduced using:
...
$ cc1 varasm.i -O0 -g -quiet -o varasm.s 
$ cc1 varasm.i -O0 -g -quiet -o varasm.s.0.0 --param ggc-min-expand=0 --param ggc-min-heapsize=0 
...
Comment 5 Tom de Vries 2010-06-21 10:32:04 UTC
Created attachment 20954 [details]
naive patch. run callbacks on hashtable entries exhaustively before deleting

Furthermore, I investigated why this problem does not occur with 4.4.0 onwards, and I found that this is due to the fact that -funit-at-a-time is hard coded to on for 4.4.0, which causes f1 to be live at the same time as f3 (no cgraph_release_function_body() in between). An easy workaround for this problem in 4.3.5 is therefore -funit-at-a-time.

I also managed to reproduce the problem for -gstabs. The patch from comment 3 works indeed, but not for -gstabs, which makes a lot of sense since the patch is dwarf specific. Of course we might attempt to fix the stab format (and possible others) in a similar way, but the fact that the fix needs to be repeated made me wonder whether the problem had to be dealt with at another level than specific debug formats.

Let's take a look at what happens exactly during garbage collection in between f1 and f3 in mark_roots():
- gt_ggc_rtab is traversed, and neither array type nor index type is marked live
- gt_ggc_cache_rtab is traversed, in particular type_hash_table, and the hash entry with the index type is hit (before the entry with the array type, but this is non-deterministic) and processed by ggc_htab_delete(). The entry is not considered live, and consequently the entry is cleared.
- next the entry with the array type is hit and processed by ggc_htab_delete(). The entry is considered live due to TYPE_SYMTAB_POINTER (type). Consequently the callback is called, marking the entry and everything reachable from it live, including the index type. Unfortunately, the hash entry for the index type is already gone.

During parsing of f3, a new index type equivalent to the old one is created, but type_hash_canon cannot find the old index type in the hash table (since that entry has been deleted), so the new index type is now a canonical type, and gets an entry in the type_hash_table. Next, a new array type equivalent to the old one is created, but type_hash_canon cannot find the old array type, even though the entry has not been deleted. The new array type has a different index type than the old array type, and consequently the hashcode for the new array type is different than the hascode for the old array type, so the old array type is not found. The new array type is now also a canonical type, and gets an entry in the type_hash_table. The old index type, the old array type and the hash table entry associated with the old array type are now unused but not freed.

The question is whether to blame this on 
- invalid use of the garbage collection infrastructure. Using the if_marked construction to mark an object live, is only allowed if everything reachable from that object is also live. 
- the garbage collection infrastructure itself. If the if_marked construction is used to mark an object live, the garbage collection infrastructure should mark everything that is reachable from that object also as live.

The patch in comment 3 seems to take the first choice. I decided to explore the second choice, and created a naive patch of ggc_mark_roots(). It solves the inconsistent debug info problem, both for dwarf2 and for stabs. I did a debug bootstrap build (-g3 -O0 -dH) with the patch and ran the testsuites (gcc, objc, gfortran, g++, libgomp, libstdc++, libjava, libmudflap, libffi), with the same results as a normal bootstrap build without the patch, so the patch looks sane at least.

This is my first time looking into the gcc garbage collector, so I'd appreciate some comments on my findings.
Comment 6 Andrew Pinski 2022-12-01 01:42:53 UTC
r0-101483-g5c9fae0d6eee58 removed the TYPE_SYMTAB_POINTER part of type_hash_marked_p .

I have no idea if the rest still applies. Plus stabs debugging has been removed in GCC 13.