Re: [RFA] Delayed physname computation

[Keith Seitz <keiths at redhat dot com>]

I'm attaching the revised version of this patch, which implements your 
requests. I am also attaching a related patch (more below).

On 04/26/2010 11:38 AM, Tom Tromey wrote:
> > > > > > "Keith" == Keith Seitz<keiths@redhat.com>  writes:
>
> Keith>  Yeah, I wasn't 100% sure whether this needed a cleanup,
> Keith>  either. Nonetheless, we have two options, catch any errors (which I
> Keith>  think was suggested in another thread for a different problem) or add
> Keith>  a cleanup. A little paranoia can't hurt that much. If you have a
> Keith>  preference, I'll implement that.
>
> One of us can handle it in a followup patch.

I've added a cleanup for this.

> > > Can this call to dwarf2_physname exhibit the problem that this patch is
> > > trying to circumvent?
>
> Keith>  Unfortunately, it very well could. I guess it would be almost as
> Keith>  useful if this called dwarf2_fullname instead. What do you think?
>
> Yeah.  I think it is important for the error case not to crash.
> Printing something useful is good, maybe even the CU+DIE would be
> enough.

I've change this to call dwarf2_full_name, which should tell us 
everything except any overload information -- more than enough, IMO, for 
a developer to track down the underlying cause.

Now for the "new" part. Fedora testing has revealed a problem with the 
optimization that this delayed physname patch uses (allocating all the 
memory at once instead of piecemeal). The assert in add_to_method_list 
could be triggered.

This was happening because, for some code, like that below, gdb would 
call dwarf2_full_name (and determine_prefix) while reading in a specific 
DIE, which would then eventually try to read in the same DIE's type. 
This resulted in add_to_method_list being called twice for the same DIE 
and gdb would assert.

Here's an example that exhibits this "problem" (which I dub an obstack 
"leak" because we might end up attempting to allocate/assign two 
different struct type's for the same DIE):

class a
{
private:
  class b
  {
  public:
    b () { }
  };

  std::vector<b> list;
};

If you put a printf in set_die_type, you'll see that this function is 
called twice (with different type structs) for the same DIE.

Since there is no way to detect/trigger this in gdb without the delayed 
physname patch, there seems little point IMO in testing specifically for 
this. However it would be trivial for me to write up a testcase for 
this. Just say the word.

Keith

ChangeLog (for the obstack leak patch)
2010-05-11  Keith Seitz  <keiths@redhat.com>

        * dwarf2read.c (read_structure_type): Check if the current
        DIE's type was already completed after dwarf2_full_name
        was called.

Attachment: delayed_physnames-2.patch
Description: Text document

Attachment: obstack-leak.patch
Description: Text document