Re: [patch] Fix duplicate types for single DIE

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> Keith has provided "obstack-leak.patch" there as a prerequisite for
Jan> the physname patch there.  Attaching here a more complete solution
Jan> of the problem.

I think it makes sense.  Thanks for doing this.

Jan> On 250MB webkit debuginfo it reduced memory consumption by 3MB from
Jan> 4616MB (0.06%) (which is less than I hoped for; checked with
Jan> Keith's physname patch).  I have not found measureable any
Jan> performance difference.

FWIW I've been thinking for a while about how we could "intern" types to
reduce memory use.  However, it seems simpler expensive to just wait for
the .debug_types-enabled GCC to be deployed everywhere; also this
approach means no performance hit for GDB.

Jan> Tom Tromey may not agree with new internal_error as expressed in
Jan> 	Re: [patch] Fix dwarf2read to crash again
Jan> 	http://sourceware.org/ml/gdb-patches/2010-04/msg00812.html
Jan> I am not sure what are his specific plans; if some TRY_CATCH should be
Jan> involved the new internal_error call should OK there.

I am still going to get back to this eventually -- I took a long detour
through DW_OP_*piece.

I don't have a concrete plan other than to find appropriate places in
the DWARF reader to catch exceptions and arrange to do something
sensible.

I don't have a problem with an internal_error if it can only trigger
when there is a bug in GDB.  If strange-but-valid DWARF can trigger it,
then I think it should be a complaint or maybe an ordinary error.

Jan> With the new internal_error call I have verified at least the fixes
Jan> of read_tag_pointer_type and read_tag_const_type fixes (and not
Jan> just these two) to be required.  Other fixes are just written
Jan> artificially to cover all the cases of any indirect call of
Jan> tag_type_to_type, that is die_type, die_descriptive_type,
Jan> set_descriptive_type and die_containing_type.  (I believe at least
Jan> those *_descriptive_type are in fact never needed as their DWARF
Jan> usage should never lead to DWARF references loops.)

Thanks.

Jan> 2010-05-13  Jan Kratochvil  <jan.kratochvil@redhat.com>
Jan> 	* dwarf2read.c (read_structure_type): Move set_descriptive_type after
Jan> 	set_die_type.
Jan> 	(read_array_type): Remove type initialization.  Recheck get_die_type
Jan> 	after initial die_type.  Move set_die_type before set_descriptive_type.
Jan> 	(read_set_type): New variable domain_type.  Recheck get_die_type after
Jan> 	initial die_type.
Jan> 	(read_tag_pointer_type, read_tag_reference_type): New variable
Jan> 	target_type.  Recheck get_die_type after initial die_type.
Jan> 	(read_tag_ptr_to_member_type): Recheck get_die_type after initial
Jan> 	die_type and die_containing_type.
Jan> 	(read_tag_const_type, read_tag_volatile_type, read_subroutine_type):
Jan> 	Recheck get_die_type after initial die_type.
Jan> 	(read_subrange_type): Recheck get_die_type after initial die_type.
Jan> 	Move set_die_type before set_descriptive_type.
Jan> 	(set_die_type): Call internal_error if DIE has some type already set.

This patch is ok.  I'm sorry for the delay in the review.

I did not look closely to see if this conflicts with Keith's
obstack-leak.patch.  If so, it doesn't matter to me if that goes in and
then you back it out, or if Keith just doesn't commit it.

Tom