On Wed, Jul 22, 2020 at 6:04 AM Simon Marchi <simark@simark.ca> wrote:

> On 2020-07-21 12:06 a.m., Caroline Tice wrote:
> > The DWARF v5 Spec describes a (slightly) new format for V5 .dwp files.
> > This patch updates GDB to allow it to read/process .dwp files in the
> > new DWARF v5 format, while continuing to be able to read/process .dwp
> > files in the older V1 & V2 formats.
>
> Can you please describe in the commit message what those differences are?
>
>
Done.


> > The one thing I felt a little odd about in this patch:  I couldn't
> > re-use the enum dwarf_sect
> > definitions, because  in version 5 several of the sections have the
> > same name as in the previous versions, but have a different ordering,
> > with different numbers attached.  So I had to create a new enum,
> > dwarf_sect_v5 for this purpose.
>
> That part would need to be cross-posted to the binutils mailing list.
> binutils
> does use the DW_SECT_* enumerators, presumably to read dwp files too, so
> they
> would likely use those new DWARF 5 enumerators eventually.
>
>
I will create/submit a patch to the binutils mailing list.


> > Is this patch ok to commit?
>
> It would be useful to precise somewhere, perhaps in the comment on `struct
> dwp_sections`.
> that versions 1 and 2 are pre-standard versions, and that version 5 was
> introduced in
> DWARF5.  And that versions 3 and 4 don't exist.
>

Done.


>
> I don't have time to do an in-depth review right now, but one question
> that came to mind
> is: is an advantage of having virtual_v2_or_v5_dwo_sections over having
> separate
> virtual_v2_dwo_sections and virtual_v5_dwo_sections?


Not particularly; I was just trying to avoid code duplication.


> Now when using v2 or v5, there are
> fields you don't use (because they are either v2-specific or v5-specific),
> so I imagine
> it's just more error prone.  Does it avoid a lot of code duplication?
>

A small amount, but not a lot.


>
> Simon
>

Below is my updated patch (mostly just updated comments & commit message)