public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
@ 2012-05-18 23:34 saugustine
  2012-05-19 15:21 ` Jason Merrill
  0 siblings, 1 reply; 14+ messages in thread
From: saugustine @ 2012-05-18 23:34 UTC (permalink / raw)
  To: jason.merrill; +Cc: gcc-patches, reply

Hi Jasaon,

Thanks so much for reviewing this patch. I realize it is a lot to see.

The motivation and new dwarf attributes and tags all stem from the debug
fission project as described at http://gcc.gnu.org/wiki/DebugFission. I
have several more patches dealing with fission coming.

Fission has been discussed in the Dwarf standardization meetings.

Thanks again, I am happy to make the changes you deem necessary.


http://codereview.appspot.com/6197069/diff/1/gcc/dwarf2out.c
File gcc/dwarf2out.c (left):

http://codereview.appspot.com/6197069/diff/1/gcc/dwarf2out.c#oldcode22231
gcc/dwarf2out.c:22231: FOR_EACH_VEC_ELT (pubname_entry, pubtype_table,
i, p)
On 2012/05/18 22:39:18, Jason Merrill wrote:
> You don't seem to have added anything to output_pubnames to avoid
emitting
> entries for pruned types.

The code I removed doesn't deal with omitting individual entries.
Instead, it decides whether to emit the pubtypes section at all--if it
is empty, then don't emit it. Pruned entries are handled as they always
were in output_pubnames.

What this code did was to check if the pubtypes section was emptied via
pruning. If it was, then don't emit it.

However, now that there is the DW_AT_GNU_pubtypes attribute that points
to the section, we need to emit the label no matter what. The presence
of this attribute helps the consumer know the difference between "No
pubtypes were present in the source" and "The compiler didn't generate
pubtypes."

http://codereview.appspot.com/6197069/diff/1/gcc/dwarf2out.c
File gcc/dwarf2out.c (right):

http://codereview.appspot.com/6197069/diff/1/gcc/dwarf2out.c#newcode8070
gcc/dwarf2out.c:8070: add_AT_lineptr (die, DW_AT_GNU_pubnames,
debug_pubnames_section_label);
On 2012/05/18 22:39:18, Jason Merrill wrote:
> What are these attributes for?  Is there a proposal to add them?
pubnames/types
> are used for lookup from a name to a DIE, so there seems to be no
reason to have
> a pointer the other way.

The entire motivation for this patch, including the proposed new
attributes is at:

http://gcc.gnu.org/wiki/DebugFission

In particular, the section titled, "Building a GDB Index".
(Unfortunately, there isn't an anchor to that section easily linkable.)
This was discussed at the past couple of dwarf standardization meetings.

http://codereview.appspot.com/6197069/diff/1/gcc/dwarf2out.c#newcode8162
gcc/dwarf2out.c:8162: || is_cu_die (die->die_parent) || is_namespace_die
(die->die_parent))
On 2012/05/18 22:39:18, Jason Merrill wrote:
> The DWARF standard says that pubnames is for names with global scope;
this
> change would add namespace-scope statics as well.

I am matching what gold and gdb do when building the gdb index. I
suppose Cary will need to add this subtle change to the proposal, and I
can also guard it with "dwarf_split_debug_info".

http://codereview.appspot.com/6197069/diff/1/gcc/dwarf2out.c#newcode9177
gcc/dwarf2out.c:9177: add_pubtype (type, base_type_result);
On 2012/05/18 22:39:18, Jason Merrill wrote:
> Why do we need pubtype entries for base types?

Same as above--to make these addable to the index, which in turn makes
gdb faster. I'll add this to the note to Cary.

http://codereview.appspot.com/6197069/diff/1/gcc/dwarf2out.c#newcode19010
gcc/dwarf2out.c:19010: /* Bypass dwarf2_name's check for DECL_NAMELESS.
*/
On 2012/05/18 22:39:18, Jason Merrill wrote:
> Why bypass the DECL_NAMELESS check?

So objects within anonymous namespaces get pubnames:

namespace { void some_function...


> Can you point me at discussion about adding enumerators and namespaces
to
> pubnames?  Historically it has just been for things with addresses
(the standard
> says "objects and functions").

This is all about making every name gdb might need public. Yet another
note to add to the proposal, I suppose. I will see to it that Cary does.

http://codereview.appspot.com/6197069/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
  2012-05-18 23:34 [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069) saugustine
@ 2012-05-19 15:21 ` Jason Merrill
  2012-05-21 18:40   ` Cary Coutant
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Merrill @ 2012-05-19 15:21 UTC (permalink / raw)
  To: saugustine, gcc-patches, reply; +Cc: Cary Coutant, Tom Tromey

On 05/18/2012 07:34 PM, saugustine@google.com wrote:
> The motivation and new dwarf attributes and tags all stem from the debug
> fission project as described at http://gcc.gnu.org/wiki/DebugFission.

Right.  Not sure why I missed the pubtypes bits on that page before...

> The code I removed doesn't deal with omitting individual entries.
> Instead, it decides whether to emit the pubtypes section at all

Ah, yes, I see.

>> What are these attributes for? Is there a proposal to add them?
>
> The entire motivation for this patch, including the proposed new
> attributes is at:
>
> http://gcc.gnu.org/wiki/DebugFission
>
> In particular, the section titled, "Building a GDB Index".

OK, I've read that section and I still don't understand why the consumer 
would need a pointer from the CU to the pubnames section to build an 
index.  If we're looking for something by name, we go from pubnames to 
the CU.  If we're looking at the CU because we're in a function body, 
presumably we need to parse most of it anyway; is there really a 
significant benefit to having a separate "what names does this CU 
define" attribute?  The index is all name->die lookup, isn't it?

I'm also puzzled by what the proposal says about .debug_types.  Why 
would a pubtypes entry point to a CU that doesn't actually have a 
definition of the type?  Is it so that the consumer knows which .dwo to 
look in?  Perhaps we could do something else with the sig8 instead of 
pointing to the wrong unit.

>> The DWARF standard says that pubnames is for names with global scope; this
>> change would add namespace-scope statics as well.
>
> I am matching what gold and gdb do when building the gdb index.

Ah.  If that's what GDB wants I guess it makes sense, but I'd like to 
see reaction from the committee to this particular aspect.  I haven't 
noticed it come up in the committee discussion of Fission.

>> Why do we need pubtype entries for base types?
>
> Same as above--to make these addable to the index, which in turn makes
> gdb faster. I'll add this to the note to Cary.

Really?  GDB wants to look up built-in types in the index rather than 
just knowing what "int" means?

>> Why bypass the DECL_NAMELESS check?
>
> So objects within anonymous namespaces get pubnames:
>
> namespace { void some_function...

Hmm, it would give the anonymous namespace itself a pubname, but I don't 
see how it would make any difference to objects within the namespace.

Jason

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
  2012-05-19 15:21 ` Jason Merrill
@ 2012-05-21 18:40   ` Cary Coutant
  2012-05-22 17:20     ` Jason Merrill
  0 siblings, 1 reply; 14+ messages in thread
From: Cary Coutant @ 2012-05-21 18:40 UTC (permalink / raw)
  To: Jason Merrill; +Cc: saugustine, gcc-patches, reply, Tom Tromey

>> The entire motivation for this patch, including the proposed new
>> attributes is at:
>>
>> http://gcc.gnu.org/wiki/DebugFission
>>
>> In particular, the section titled, "Building a GDB Index".
>
> OK, I've read that section and I still don't understand why the consumer
> would need a pointer from the CU to the pubnames section to build an index.
>  If we're looking for something by name, we go from pubnames to the CU.  If
> we're looking at the CU because we're in a function body, presumably we need
> to parse most of it anyway; is there really a significant benefit to having
> a separate "what names does this CU define" attribute?  The index is all
> name->die lookup, isn't it?

The DW_AT_GNU_pubnames/pubtypes attributes serve two purposes: (1)
they let the linker know which CUs have pubnames & pubtypes sections,
and (2) they let us know that the pubnames and pubtypes are "reliable"
(relative to the incomplete sections that GCC has generated in the
past). When building the .gdb_index, the linker starts with the top
compile_unit DIE of the CU. If there are pubnames/pubtypes attributes,
it can go read those sections; otherwise, it needs to parse the whole
DIE tree (slow) to extract the information needed for .gdb_index. It
would be possible to use the pointer from the pubnames section to the
CU, but that's a reverse pointer, and it would take time to set up the
links to figure out which pubnames/pubtypes sections go with which
CUs.


> I'm also puzzled by what the proposal says about .debug_types.  Why would a
> pubtypes entry point to a CU that doesn't actually have a definition of the
> type?  Is it so that the consumer knows which .dwo to look in?  Perhaps we
> could do something else with the sig8 instead of pointing to the wrong unit.

Yes, it's to find the .dwo. I never did anything to accomodate
.debug_types sections with .debug_pubtypes -- probably a more
substantial change to the format is in order, but I wanted to save
that for what I expect to be a complete revamping of the accelerated
lookup tables in DWARF.


>>> The DWARF standard says that pubnames is for names with global scope;
>>> this
>>> change would add namespace-scope statics as well.
>>
>> I am matching what gold and gdb do when building the gdb index.
>
> Ah.  If that's what GDB wants I guess it makes sense, but I'd like to see
> reaction from the committee to this particular aspect.  I haven't noticed it
> come up in the committee discussion of Fission.

We had some discussion in the DWARF workgroup during the DWARF 4
development about what should go into pubnames and pubtypes, but
didn't really settle on any specific wording change for the standard.
I think it was generally agreed that it's ultimately a
quality-of-implementation issue where the producer and consumer need
to agree on what's useful there. For the purposes of a debugger, the
only thing that really make sense is to have a name-to-CU index that
indexes all the names that a user might type at the user interface.
Without the fixes we're putting in here, the pubnames section has been
essentially useless, and has been ignored entirely by gdb -- to the
point that a change went in a year or so ago to disable the section by
default except for Darwin.

I anticipate further discussion of accelerated lookup in the DWARF 5
discussions, and I expect these tables to get a major overhaul. Until
then, I think it's reasonable to make use of them as best we can
without making any major structural changes.


>>> Why do we need pubtype entries for base types?
>>
>> Same as above--to make these addable to the index, which in turn makes
>> gdb faster. I'll add this to the note to Cary.
>
> Really?  GDB wants to look up built-in types in the index rather than just
> knowing what "int" means?

It doesn't actually always mean the same thing, though. It might be
less surprising if you ask that about some other base type like
"double" (and consider that GCC has options like -fshort-double).
Also, GDB can narrow the set of CUs that it has to read sometimes by
knowing which CUs actually contain a declaration of a given base type
(again, it's easier to see the advantage for "double" than for "int").


>>> Why bypass the DECL_NAMELESS check?
>>
>> So objects within anonymous namespaces get pubnames:
>>
>> namespace { void some_function...
>
> Hmm, it would give the anonymous namespace itself a pubname, but I don't see
> how it would make any difference to objects within the namespace.

The items within an anonymous namespace need to be indexed so that a
GDB user can just type "some_function" instead of "(anonymous
namespace)::some_function".

-cary

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
  2012-05-21 18:40   ` Cary Coutant
@ 2012-05-22 17:20     ` Jason Merrill
  2012-05-22 17:27       ` Sterling Augustine
  2012-05-22 17:49       ` Cary Coutant
  0 siblings, 2 replies; 14+ messages in thread
From: Jason Merrill @ 2012-05-22 17:20 UTC (permalink / raw)
  To: Cary Coutant; +Cc: saugustine, gcc-patches, reply, Tom Tromey

On 05/21/2012 02:39 PM, Cary Coutant wrote:
> The DW_AT_GNU_pubnames/pubtypes attributes serve two purposes: (1)
> they let the linker know which CUs have pubnames & pubtypes sections,
> and (2) they let us know that the pubnames and pubtypes are "reliable"
> (relative to the incomplete sections that GCC has generated in the
> past). When building the .gdb_index, the linker starts with the top
> compile_unit DIE of the CU. If there are pubnames/pubtypes attributes,
> it can go read those sections; otherwise, it needs to parse the whole
> DIE tree (slow) to extract the information needed for .gdb_index. It
> would be possible to use the pointer from the pubnames section to the
> CU, but that's a reverse pointer, and it would take time to set up the
> links to figure out which pubnames/pubtypes sections go with which
> CUs.

I would expect the linker to start by processing the pubnames/pubtypes 
sections, and then if it wants to look through the CUs as well it 
already knows which ones it can skip.

The presence or absence of an attribute seems like a fragile way to 
determine whether or not particular debug info is sufficiently reliable. 
  If a consumer wants to make decisions based on changing behavior in 
different versions of a producer, it could look at the DW_AT_producer 
string.

>> I'm also puzzled by what the proposal says about .debug_types.  Why would a
>> pubtypes entry point to a CU that doesn't actually have a definition of the
>> type?  Is it so that the consumer knows which .dwo to look in?  Perhaps we
>> could do something else with the sig8 instead of pointing to the wrong unit.
>
> Yes, it's to find the .dwo. I never did anything to accomodate
> .debug_types sections with .debug_pubtypes -- probably a more
> substantial change to the format is in order, but I wanted to save
> that for what I expect to be a complete revamping of the accelerated
> lookup tables in DWARF.

Hmm.  Looking at the code, it seems that the pubtypes entries for these 
types will point to the main CU, but with the offset in the type_unit, 
so a consumer trying to follow the pointer will find garbage.  If you 
want to have a pubtypes entry, it needs to point to a type skeleton to 
avoid crashing standard-conforming consumers.

>>>> The DWARF standard says that pubnames is for names with global scope;
>>>> this change would add namespace-scope statics as well.
>
> I think it was generally agreed that it's ultimately a
> quality-of-implementation issue where the producer and consumer need
> to agree on what's useful there.

OK.

>> Really?  GDB wants to look up built-in types in the index rather than just
>> knowing what "int" means?
>
> It doesn't actually always mean the same thing, though. It might be
> less surprising if you ask that about some other base type like
> "double" (and consider that GCC has options like -fshort-double).
> Also, GDB can narrow the set of CUs that it has to read sometimes by
> knowing which CUs actually contain a declaration of a given base type
> (again, it's easier to see the advantage for "double" than for "int").

OK.

>>>> Why bypass the DECL_NAMELESS check?
>>>
>>> So objects within anonymous namespaces get pubnames:
>>>
>>> namespace { void some_function...
>>
>> Hmm, it would give the anonymous namespace itself a pubname, but I don't see
>> how it would make any difference to objects within the namespace.
>
> The items within an anonymous namespace need to be indexed so that a
> GDB user can just type "some_function" instead of "(anonymous
> namespace)::some_function".

Yes, I agree that we want to put "some_function" in pubnames.  I still 
don't see how putting the anonymous namespace itself in pubnames helps 
at all.  As far as pubnames is concerned, the anonymous namespace should 
be transparent, and "some_function" should appear unqualified.

Jason

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
  2012-05-22 17:20     ` Jason Merrill
@ 2012-05-22 17:27       ` Sterling Augustine
  2012-05-22 17:49       ` Cary Coutant
  1 sibling, 0 replies; 14+ messages in thread
From: Sterling Augustine @ 2012-05-22 17:27 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Cary Coutant, gcc-patches, reply, Tom Tromey

On Tue, May 22, 2012 at 10:19 AM, Jason Merrill <jason@redhat.com> wrote:

I'll let Cary handle the other questions.

> Yes, I agree that we want to put "some_function" in pubnames.  I still don't
> see how putting the anonymous namespace itself in pubnames helps at all.  As
> far as pubnames is concerned, the anonymous namespace should be transparent,
> and "some_function" should appear unqualified.

Anonymous namespaces are funky in general, but gdb would like to be
able to do things like tab completion on:

(gdb) b '(anonymous namespace)::
(gdb) b 'foo::(anonymous namespace)::

Without being able to lookup anonymous namespaces, gdb has no idea
where to continue from.

This works today without pubnames because gdb goes and constructs a
big list that includes anonymous namespaces by hand. Without anonymous
namespaces in the index, gdb would either lose that functionality, or
would have to parse the dwarf to find them. If it does that, then the
index isn't very helpful.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
  2012-05-22 17:20     ` Jason Merrill
  2012-05-22 17:27       ` Sterling Augustine
@ 2012-05-22 17:49       ` Cary Coutant
  2012-05-22 18:46         ` Jason Merrill
  1 sibling, 1 reply; 14+ messages in thread
From: Cary Coutant @ 2012-05-22 17:49 UTC (permalink / raw)
  To: Jason Merrill; +Cc: saugustine, gcc-patches, reply, Tom Tromey

> I would expect the linker to start by processing the pubnames/pubtypes
> sections, and then if it wants to look through the CUs as well it already
> knows which ones it can skip.
>
> The presence or absence of an attribute seems like a fragile way to
> determine whether or not particular debug info is sufficiently reliable.  If
> a consumer wants to make decisions based on changing behavior in different
> versions of a producer, it could look at the DW_AT_producer string.

In the context of Tom's original suggestion to add .gdb_index, I
suggested fixing the problems with .debug_pubnames and using the
producer string to distinguish. Tom had several objections to the use
of the producer string for that:

   http://sourceware.org/ml/archer/2009-q4/msg00065.html

I'm simply trying to do something that's fairly simple and that works.
These attributes are GNU extensions, and I do not intend to propose
these as part of the DWARF 5 standard -- they're stopgap measures
until we have a better plan for accelerated lookup tables.

Starting with the pubnames sections then going back and reading the
debug_info sections would require an extra table and a lookup for each
CU to see if it's already been covered by a pubnames section. Building
the .gdb_index in gold is still too slow, but these attributes help
make it a bit faster than it was without them.

Are these two attributes a showstopper issue for you?


> Hmm.  Looking at the code, it seems that the pubtypes entries for these
> types will point to the main CU, but with the offset in the type_unit, so a
> consumer trying to follow the pointer will find garbage.  If you want to
> have a pubtypes entry, it needs to point to a type skeleton to avoid
> crashing standard-conforming consumers.

Yes, I understand that's broken, but there are no consumers at this
point that make any use of that offset. Would it be acceptable if we
just put 0 there? (Given that I expect .debug_pub* to go away soon, I
don't think it's worth the trouble of filling in the offset with
anything more meaningful.)

-cary

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
  2012-05-22 17:49       ` Cary Coutant
@ 2012-05-22 18:46         ` Jason Merrill
  2012-05-22 20:05           ` Cary Coutant
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Merrill @ 2012-05-22 18:46 UTC (permalink / raw)
  To: Cary Coutant; +Cc: saugustine, gcc-patches, reply, Tom Tromey

On 05/22/2012 01:27 PM, Sterling Augustine wrote:
> Anonymous namespaces are funky in general, but gdb would like to be
> able to do things like tab completion on:
>
> (gdb) b '(anonymous namespace)::
> (gdb) b 'foo::(anonymous namespace)::
>
> Without being able to lookup anonymous namespaces, gdb has no idea
> where to continue from.

Ah.  I guess that makes sense.

On 05/22/2012 01:49 PM, Cary Coutant wrote:
> In the context of Tom's original suggestion to add .gdb_index, I
> suggested fixing the problems with .debug_pubnames and using the
> producer string to distinguish. Tom had several objections to the use
> of the producer string for that:
>
>     http://sourceware.org/ml/archer/2009-q4/msg00065.html

Sure, his point about backporting patches is a strong argument.

> I'm simply trying to do something that's fairly simple and that works.
> These attributes are GNU extensions, and I do not intend to propose
> these as part of the DWARF 5 standard -- they're stopgap measures
> until we have a better plan for accelerated lookup tables.

> Starting with the pubnames sections then going back and reading the
> debug_info sections would require an extra table and a lookup for each
> CU to see if it's already been covered by a pubnames section.

Yes, but I would expect this table lookup to be faster than going to the 
disk to read the CU DIE and abbrev in order to check for the attribute. 
  OTOH, I suppose you need to read it anyway if you want to check 
somehow whether you should trust the pub* information.

> Building
> the .gdb_index in gold is still too slow, but these attributes help
> make it a bit faster than it was without them.
>
> Are these two attributes a showstopper issue for you?

No, I'm just reluctant to add more relocations.  Could we make them just 
flags?

>> Hmm.  Looking at the code, it seems that the pubtypes entries for these
>> types will point to the main CU, but with the offset in the type_unit, so a
>> consumer trying to follow the pointer will find garbage.  If you want to
>> have a pubtypes entry, it needs to point to a type skeleton to avoid
>> crashing standard-conforming consumers.
>
> Yes, I understand that's broken, but there are no consumers at this
> point that make any use of that offset. Would it be acceptable if we
> just put 0 there? (Given that I expect .debug_pub* to go away soon, I
> don't think it's worth the trouble of filling in the offset with
> anything more meaningful.)

I wouldn't expect it to be much harder to put the skeleton there than 
plain 0.

Jason

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
  2012-05-22 18:46         ` Jason Merrill
@ 2012-05-22 20:05           ` Cary Coutant
  2012-05-22 21:56             ` Jakub Jelinek
  2012-05-29 22:55             ` Cary Coutant
  0 siblings, 2 replies; 14+ messages in thread
From: Cary Coutant @ 2012-05-22 20:05 UTC (permalink / raw)
  To: Jason Merrill; +Cc: saugustine, gcc-patches, reply, Tom Tromey

> Yes, but I would expect this table lookup to be faster than going to the
> disk to read the CU DIE and abbrev in order to check for the attribute.
>  OTOH, I suppose you need to read it anyway if you want to check somehow
> whether you should trust the pub* information.

Right. I also need to read the top-level DIE to get the range list for the CU.

>> Building
>> the .gdb_index in gold is still too slow, but these attributes help
>> make it a bit faster than it was without them.
>>
>> Are these two attributes a showstopper issue for you?
>
> No, I'm just reluctant to add more relocations.  Could we make them just
> flags?

That might be workable. Let me take a look at the gold changes I'd
need to make for that.

They don't have to be relocations, though -- since they're only used
by the linker, a raw section-relative offset would be sufficient.

>> Yes, I understand that's broken, but there are no consumers at this
>> point that make any use of that offset. Would it be acceptable if we
>> just put 0 there? (Given that I expect .debug_pub* to go away soon, I
>> don't think it's worth the trouble of filling in the offset with
>> anything more meaningful.)
>
> I wouldn't expect it to be much harder to put the skeleton there than plain
> 0.

I was concerned that we might not always have a skeleton type to point
to, but I confess I haven't looked closely enough to know whether
that's a valid concern.

-cary

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
  2012-05-22 20:05           ` Cary Coutant
@ 2012-05-22 21:56             ` Jakub Jelinek
  2012-05-29 22:55             ` Cary Coutant
  1 sibling, 0 replies; 14+ messages in thread
From: Jakub Jelinek @ 2012-05-22 21:56 UTC (permalink / raw)
  To: Cary Coutant; +Cc: Jason Merrill, saugustine, gcc-patches, reply, Tom Tromey

On Tue, May 22, 2012 at 01:04:15PM -0700, Cary Coutant wrote:
> That might be workable. Let me take a look at the gold changes I'd
> need to make for that.
> 
> They don't have to be relocations, though -- since they're only used
> by the linker, a raw section-relative offset would be sufficient.

But if they are just flags, they can be DW_FORM_flag_present and waste
space just in .debug_abbrev, not in .debug_info.

	Jakub

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
  2012-05-22 20:05           ` Cary Coutant
  2012-05-22 21:56             ` Jakub Jelinek
@ 2012-05-29 22:55             ` Cary Coutant
  2012-05-30 17:52               ` Cary Coutant
  1 sibling, 1 reply; 14+ messages in thread
From: Cary Coutant @ 2012-05-29 22:55 UTC (permalink / raw)
  To: Jason Merrill; +Cc: saugustine, gcc-patches, reply, Tom Tromey

>>> Yes, I understand that's broken, but there are no consumers at this
>>> point that make any use of that offset. Would it be acceptable if we
>>> just put 0 there? (Given that I expect .debug_pub* to go away soon, I
>>> don't think it's worth the trouble of filling in the offset with
>>> anything more meaningful.)
>>
>> I wouldn't expect it to be much harder to put the skeleton there than plain
>> 0.
>
> I was concerned that we might not always have a skeleton type to point
> to, but I confess I haven't looked closely enough to know whether
> that's a valid concern.

At the time we emit the pubtypes table, we have a pointer to the DIE
that has been moved to the type unit, and there's no mapping from that
back to the skeleton DIE. As it stands, we don't even emit a skeleton
DIE unless one of its descendants is a declaration, so we can't count
on always having a skeleton DIE to point to. In the case of
enumeration constants, if we did have a skeleton DIE, it would only be
for the parent enumeration type.

How about we modify the patch to just emit a 0 for the DIE offset in a
pubtype entry?

-cary

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
  2012-05-29 22:55             ` Cary Coutant
@ 2012-05-30 17:52               ` Cary Coutant
  2012-05-30 18:16                 ` Jason Merrill
  0 siblings, 1 reply; 14+ messages in thread
From: Cary Coutant @ 2012-05-30 17:52 UTC (permalink / raw)
  To: Jason Merrill; +Cc: saugustine, gcc-patches, reply, Tom Tromey

> At the time we emit the pubtypes table, we have a pointer to the DIE
> that has been moved to the type unit, and there's no mapping from that
> back to the skeleton DIE. As it stands, we don't even emit a skeleton
> DIE unless one of its descendants is a declaration, so we can't count
> on always having a skeleton DIE to point to. In the case of
> enumeration constants, if we did have a skeleton DIE, it would only be
> for the parent enumeration type.
>
> How about we modify the patch to just emit a 0 for the DIE offset in a
> pubtype entry?

I can add a field to the comdat_type_node structure to keep track of
the skeleton DIE for a given type unit, so that I can easily get the
right DIE offset for cases where there is a skeleton DIE. When there
is no skeleton DIE, I'll change it to emit 0 for the DIE offset. Sound
OK?

-cary

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
  2012-05-30 17:52               ` Cary Coutant
@ 2012-05-30 18:16                 ` Jason Merrill
  0 siblings, 0 replies; 14+ messages in thread
From: Jason Merrill @ 2012-05-30 18:16 UTC (permalink / raw)
  To: Cary Coutant; +Cc: saugustine, gcc-patches, reply, Tom Tromey

On 05/30/2012 01:52 PM, Cary Coutant wrote:
>> At the time we emit the pubtypes table, we have a pointer to the DIE
> I can add a field to the comdat_type_node structure to keep track of
> the skeleton DIE for a given type unit, so that I can easily get the
> right DIE offset for cases where there is a skeleton DIE. When there
> is no skeleton DIE, I'll change it to emit 0 for the DIE offset. Sound
> OK?

OK.

Jason

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
@ 2012-06-07 21:47 saugustine
  0 siblings, 0 replies; 14+ messages in thread
From: saugustine @ 2012-06-07 21:47 UTC (permalink / raw)
  To: jason.merrill, ccoutant, jason; +Cc: gcc-patches, reply

On 2012/06/01 17:58:41, saugustine wrote:
> The enclosed patch updates the earlier patch to address all of the
feedback I
> have gotten regarding generating pubnames. It fixes the offset problem
in
> the pubtypes table; switches DW_AT_pubtypes to a flag and so on.

> It also adds and documents a new option "-g[no-]pubtypes" which allows
users
> to generate pubtypes even if the target disables them by default.

> Tested with bootstrap and running the test_pubnames_and_indices.py
script
> recently contributed to the GDB project.


Ping? I have another patch that depends on this one coming.

http://codereview.appspot.com/6197069/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
@ 2012-05-18 22:39 jason.merrill
  0 siblings, 0 replies; 14+ messages in thread
From: jason.merrill @ 2012-05-18 22:39 UTC (permalink / raw)
  To: saugustine; +Cc: gcc-patches, reply

This patch makes a lot of changes to the behavior of .debug_pubnames
that I haven't seen any discussion of, and that don't seem obvious to
me.  Can you point me at discussion threads?


http://codereview.appspot.com/6197069/diff/1/gcc/dwarf2out.c
File gcc/dwarf2out.c (left):

http://codereview.appspot.com/6197069/diff/1/gcc/dwarf2out.c#oldcode22231
gcc/dwarf2out.c:22231: FOR_EACH_VEC_ELT (pubname_entry, pubtype_table,
i, p)
You don't seem to have added anything to output_pubnames to avoid
emitting entries for pruned types.

http://codereview.appspot.com/6197069/diff/1/gcc/dwarf2out.c
File gcc/dwarf2out.c (right):

http://codereview.appspot.com/6197069/diff/1/gcc/dwarf2out.c#newcode8070
gcc/dwarf2out.c:8070: add_AT_lineptr (die, DW_AT_GNU_pubnames,
debug_pubnames_section_label);
What are these attributes for?  Is there a proposal to add them?
pubnames/types are used for lookup from a name to a DIE, so there seems
to be no reason to have a pointer the other way.

http://codereview.appspot.com/6197069/diff/1/gcc/dwarf2out.c#newcode8162
gcc/dwarf2out.c:8162: || is_cu_die (die->die_parent) || is_namespace_die
(die->die_parent))
The DWARF standard says that pubnames is for names with global scope;
this change would add namespace-scope statics as well.

http://codereview.appspot.com/6197069/diff/1/gcc/dwarf2out.c#newcode9177
gcc/dwarf2out.c:9177: add_pubtype (type, base_type_result);
Why do we need pubtype entries for base types?

http://codereview.appspot.com/6197069/diff/1/gcc/dwarf2out.c#newcode19010
gcc/dwarf2out.c:19010: /* Bypass dwarf2_name's check for DECL_NAMELESS.
*/
Why bypass the DECL_NAMELESS check?

Can you point me at discussion about adding enumerators and namespaces
to pubnames?  Historically it has just been for things with addresses
(the standard says "objects and functions").

http://codereview.appspot.com/6197069/

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2012-06-07 21:26 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-18 23:34 [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069) saugustine
2012-05-19 15:21 ` Jason Merrill
2012-05-21 18:40   ` Cary Coutant
2012-05-22 17:20     ` Jason Merrill
2012-05-22 17:27       ` Sterling Augustine
2012-05-22 17:49       ` Cary Coutant
2012-05-22 18:46         ` Jason Merrill
2012-05-22 20:05           ` Cary Coutant
2012-05-22 21:56             ` Jakub Jelinek
2012-05-29 22:55             ` Cary Coutant
2012-05-30 17:52               ` Cary Coutant
2012-05-30 18:16                 ` Jason Merrill
  -- strict thread matches above, loose matches on Subject: below --
2012-06-07 21:47 saugustine
2012-05-18 22:39 jason.merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).