This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] comdat types
- From: Tom Tromey <tromey at redhat dot com>
- To: dje at google dot com (Doug Evans)
- Cc: gdb-patches at sourceware dot org, ccoutant at google dot com
- Date: Thu, 25 Jun 2009 13:46:45 -0600
- Subject: Re: [RFA] comdat types
- References: <20090620000402.C37E5843F5@localhost>
- Reply-to: tromey at redhat dot com
>>>>> "Doug" == Doug Evans <dje@google.com> writes:
Doug> This patch adds support for "comdat types" from dwarf4.
Doug> http://wiki.dwarfstd.org/index.php?title=COMDAT_Type_Sections
I am a fan of this.
I'm curious about one thing on this page, and I thought I'd take the
opportunity to ask. DW_TAG_type_unit may have a DW_AT_language child.
But, the language is not mentioned in the suggested method for
computing a type's signature. This seems strange to me: either the
language matters (in which case, it seems like it ought to be in the
signature); or the language does not matter, in which case, why
mention it?
Doug> I need to revisit them with the latest gcc (the dwarf4 branch)
Doug> as they seem like gcc issues.
Doug> [specifically gdb.cp/classes.exp, and "info type foo" doesn't print the
Doug> source location though it should]
Yeah; my view is that since this patch can't hurt anything, if the gcc
changes require gdb changes in turn, it won't matter.
Doug> Ok to check in?
I read through it. Unfortunately, I don't think I am enough of a
dwarf2read.c expert to approve this. So, I would appreciate it if
someone more expert could claim it. Alternatively, if nobody has time
to do this, and they say so, I will learn more and take it on.
One concern I have is whether there is any chance that the
specification will change between now and when DWARF-4 is published.
I do have a few nits.
Doug> + types_htab = htab_create_alloc_ex (41,
Doug> + hash_type_signature,
Doug> + eq_type_signature,
Doug> + NULL,
Doug> + &objfile->objfile_obstack,
Doug> + hashtab_obstack_allocate,
Doug> + dummy_obstack_deallocate);
This is just a side note -- I've seen a few hash tables allocated on
obstacks. Doesn't resizing the table waste memory?
Too bad we don't have allocation pools instead of obstacks. I suppose
in this specific case we could use the objfile data machinery to
deallocate hash tables.
Doug> + if (dwarf2_die_debug)
Doug> + {
Doug> + fprintf_unfiltered (gdb_stdlog, "Signatured types:\n");
Doug> + }
Over-bracing. There's a fair amount of this in the patch.
Doug> + if (this_cu->from_debug_types)
Doug> + {
Doug> + /* ??? How come this is for .debug_types only? */
Doug> + this_cu->offset = cu.header.offset;
Doug> + this_cu->length = cu.header.length + cu.header.initial_length_size;
Daniel has asked before for "no new FIXMEs". You can't escape this by
spelling it "???" :-)
Seriously, I think this is a valid concern and I would like some
definitive resolution to the various "???" comments.
Doug> + NotFound:
Should be "not_found". There's a couple of these.
Tom