This is the mail archive of the
mailing list for the GDB project.
Re: [patch] Fix duplicate types for single DIE
- From: Tom Tromey <tromey at redhat dot com>
- To: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Cc: gdb-patches at sourceware dot org, Keith Seitz <keiths at redhat dot com>
- Date: Fri, 04 Jun 2010 16:47:35 -0600
- Subject: Re: [patch] Fix duplicate types for single DIE
- References: <20100513115029.GA27341@host0.dyn.jankratochvil.net>
- Reply-to: tromey at redhat dot com
>>>>> "Jan" == Jan Kratochvil <firstname.lastname@example.org> 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> 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
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
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.)
Jan> 2010-05-13 Jan Kratochvil <email@example.com>
Jan> * dwarf2read.c (read_structure_type): Move set_descriptive_type after
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.