[dwarf2read] Fix crash when loading dwp files: calculate num_sections based on actual section indices, not just the number of sections.

  Sorry, patch somehow didn't make it in the previous message. Including
it manually:

---
 gdb/ChangeLog    | 6 ++++++
 gdb/dwarf2read.c | 3 +--
 2 files changed, 7 insertions(+), 2 deletions(-)

                    dwp_file->num_sections, asection *);
--
2.21.0.rc0.258.g878e2cd30e-goog
  

On 2019-02-25 15:21, Jordan Rupprecht wrote:
> Sorry, patch somehow didn't make it in the previous message. Including
> it manually:
> 
> ---
>  gdb/ChangeLog    | 6 ++++++
>  gdb/dwarf2read.c | 3 +--
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 118f17588a..c2340e694c 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,9 @@
> +2019-02-25  Jordan Rupprecht  <rupprecht@google.com>
> +
> +       * dwarf2read.c (open_and_init_dwp_file): Call
> +       elf_numsections instead of bfd_count_sections to initialize
> +       dwp_file->num_sections.
> +
>  2019-02-25  Tom Tromey  <tromey@adacore.com>
> 
>         * solib-darwin.c (darwin_get_dyld_bfd): Don't release dyld_bfd.
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 98f46e0416..2908a233fe 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -13230,8 +13230,7 @@ open_and_init_dwp_file (struct
> dwarf2_per_objfile *dwarf2_per_objfile)
>    std::unique_ptr<struct dwp_file> dwp_file
>      (new struct dwp_file (name, std::move (dbfd)));
> 
> -  /* +1: section 0 is unused */
> -  dwp_file->num_sections = bfd_count_sections (dwp_file->dbfd) + 1;
> +  dwp_file->num_sections = elf_numsections (dwp_file->dbfd);
>    dwp_file->elf_sections =
>      OBSTACK_CALLOC (&objfile->objfile_obstack,
>                     dwp_file->num_sections, asection *);
> --
> 2.21.0.rc0.258.g878e2cd30e-goog

I have tweaked the original commit message a bit and pushed.

Thanks again for the patch.

Simon
  

Thanks Simon!

On Mon, Feb 25, 2019 at 12:55 PM Simon Marchi <simon.marchi@polymtl.ca> wrote:
>
> On 2019-02-25 15:21, Jordan Rupprecht wrote:
> > Sorry, patch somehow didn't make it in the previous message. Including
> > it manually:
> >
> > ---
> >  gdb/ChangeLog    | 6 ++++++
> >  gdb/dwarf2read.c | 3 +--
> >  2 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> > index 118f17588a..c2340e694c 100644
> > --- a/gdb/ChangeLog
> > +++ b/gdb/ChangeLog
> > @@ -1,3 +1,9 @@
> > +2019-02-25  Jordan Rupprecht  <rupprecht@google.com>
> > +
> > +       * dwarf2read.c (open_and_init_dwp_file): Call
> > +       elf_numsections instead of bfd_count_sections to initialize
> > +       dwp_file->num_sections.
> > +
> >  2019-02-25  Tom Tromey  <tromey@adacore.com>
> >
> >         * solib-darwin.c (darwin_get_dyld_bfd): Don't release dyld_bfd.
> > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> > index 98f46e0416..2908a233fe 100644
> > --- a/gdb/dwarf2read.c
> > +++ b/gdb/dwarf2read.c
> > @@ -13230,8 +13230,7 @@ open_and_init_dwp_file (struct
> > dwarf2_per_objfile *dwarf2_per_objfile)
> >    std::unique_ptr<struct dwp_file> dwp_file
> >      (new struct dwp_file (name, std::move (dbfd)));
> >
> > -  /* +1: section 0 is unused */
> > -  dwp_file->num_sections = bfd_count_sections (dwp_file->dbfd) + 1;
> > +  dwp_file->num_sections = elf_numsections (dwp_file->dbfd);
> >    dwp_file->elf_sections =
> >      OBSTACK_CALLOC (&objfile->objfile_obstack,
> >                     dwp_file->num_sections, asection *);
> > --
> > 2.21.0.rc0.258.g878e2cd30e-goog
>
> I have tweaked the original commit message a bit and pushed.
>
> Thanks again for the patch.
>
> Simon
  

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

>> -  dwp_file->num_sections = bfd_count_sections (dwp_file->dbfd) + 1;
>> +  dwp_file->num_sections = elf_numsections (dwp_file->dbfd);

Simon> I have tweaked the original commit message a bit and pushed.
Simon> Thanks again for the patch.

I wonder if there's any way to reach this with a non-ELF objfile --
Mach-O seems probable.  I think we also handle DWARF on AIX (ECOFF IIRC)
but I don't know if that does dwp.

Tom
  

On 2019-02-26 10:08, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
>>> -  dwp_file->num_sections = bfd_count_sections (dwp_file->dbfd) + 1;
>>> +  dwp_file->num_sections = elf_numsections (dwp_file->dbfd);
> 
> Simon> I have tweaked the original commit message a bit and pushed.
> Simon> Thanks again for the patch.
> 
> I wonder if there's any way to reach this with a non-ELF objfile --
> Mach-O seems probable.  I think we also handle DWARF on AIX (ECOFF 
> IIRC)
> but I don't know if that does dwp.

Oh, maybe.  Unfortunately, I don't have access to a Mac to test this at 
the moment.

I'll give it a try on gcc119.fsffrance.org for AIX.

Simon
  

On 2019-02-26 11:24, Simon Marchi wrote:
> On 2019-02-26 10:08, Tom Tromey wrote:
>>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
>> 
>>>> -  dwp_file->num_sections = bfd_count_sections (dwp_file->dbfd) + 1;
>>>> +  dwp_file->num_sections = elf_numsections (dwp_file->dbfd);
>> 
>> Simon> I have tweaked the original commit message a bit and pushed.
>> Simon> Thanks again for the patch.
>> 
>> I wonder if there's any way to reach this with a non-ELF objfile --
>> Mach-O seems probable.  I think we also handle DWARF on AIX (ECOFF 
>> IIRC)
>> but I don't know if that does dwp.
> 
> Oh, maybe.  Unfortunately, I don't have access to a Mac to test this
> at the moment.
> 
> I'll give it a try on gcc119.fsffrance.org for AIX.
> 
> Simon

So my impression now is that dwp doesn't apply to non-ELF projects.  It 
is built as part of gold, which itself deals only with ELF, AFAIK.  I 
tried to build gold on AIX, without success.

Simon
  

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> So my impression now is that dwp doesn't apply to non-ELF projects.
Simon> It is built as part of gold, which itself deals only with ELF, AFAIK.
Simon> I tried to build gold on AIX, without success.

I think there's also llvm-dwp.  Does it work on Mach-O?

Tom
  

On 2019-02-27 12:22, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
> Simon> So my impression now is that dwp doesn't apply to non-ELF 
> projects.
> Simon> It is built as part of gold, which itself deals only with ELF, 
> AFAIK.
> Simon> I tried to build gold on AIX, without success.
> 
> I think there's also llvm-dwp.  Does it work on Mach-O?
> 
> Tom

Of course, there's llvm-dwp, it's the reason this patch was written :).

Maybe David (in CC) can help answer this?

Simon
  

(Adding in Adrian)

While llvm-dwp will run just fine on osx as a program, it's not
intended for the platform. I don't know if there are any plans for
dwarf5-esque split dwarf on apple platforms. Adrian might be able to
comment more.

-eric

On Wed, Feb 27, 2019 at 9:40 AM Simon Marchi <simon.marchi@polymtl.ca> wrote:
>
> On 2019-02-27 12:22, Tom Tromey wrote:
> >>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> >
> > Simon> So my impression now is that dwp doesn't apply to non-ELF
> > projects.
> > Simon> It is built as part of gold, which itself deals only with ELF,
> > AFAIK.
> > Simon> I tried to build gold on AIX, without success.
> >
> > I think there's also llvm-dwp.  Does it work on Mach-O?
> >
> > Tom
>
> Of course, there's llvm-dwp, it's the reason this patch was written :).
>
> Maybe David (in CC) can help answer this?
>
> Simon
  

Split DWARF solves a problem that doesn't exist on Darwin (macOS/iOS, ...). The motivation behind split DWARF is to reduce the number of relocations in debug info that have to be processed by the linker. But on Darwin, debug info is not processed by the linker at all, instead a tool called dsymutil (cf. llvm/tools/dsymutil) serves a conceptually similar task to dwp and archives the debug info from the .o files into a .dSYM bundle, separate from the executable.

-- adrian

> On Feb 27, 2019, at 9:56 AM, Eric Christopher <echristo@gmail.com> wrote:
> 
> (Adding in Adrian)
> 
> While llvm-dwp will run just fine on osx as a program, it's not
> intended for the platform. I don't know if there are any plans for
> dwarf5-esque split dwarf on apple platforms. Adrian might be able to
> comment more.
> 
> -eric
> 
> On Wed, Feb 27, 2019 at 9:40 AM Simon Marchi <simon.marchi@polymtl.ca> wrote:
>> 
>> On 2019-02-27 12:22, Tom Tromey wrote:
>>>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
>>> 
>>> Simon> So my impression now is that dwp doesn't apply to non-ELF
>>> projects.
>>> Simon> It is built as part of gold, which itself deals only with ELF,
>>> AFAIK.
>>> Simon> I tried to build gold on AIX, without success.
>>> 
>>> I think there's also llvm-dwp.  Does it work on Mach-O?
>>> 
>>> Tom
>> 
>> Of course, there's llvm-dwp, it's the reason this patch was written :).
>> 
>> Maybe David (in CC) can help answer this?
>> 
>> Simon
  

On 2019-02-27 13:05, Adrian Prantl wrote:
> Split DWARF solves a problem that doesn't exist on Darwin (macOS/iOS,
> ...). The motivation behind split DWARF is to reduce the number of
> relocations in debug info that have to be processed by the linker. But
> on Darwin, debug info is not processed by the linker at all, instead a
> tool called dsymutil (cf. llvm/tools/dsymutil) serves a conceptually
> similar task to dwp and archives the debug info from the .o files into
> a .dSYM bundle, separate from the executable.
> 
> -- adrian

Ok, thanks to both of you for confirming.  I think we are fine using the 
elf-specific function.

Simon
  

On Wed, Feb 27, 2019 at 10:05 AM Adrian Prantl <aprantl@apple.com> wrote:

> Split DWARF solves a problem that doesn't exist on Darwin (macOS/iOS,
> ...). The motivation behind split DWARF is to reduce the number of
> relocations in debug info that have to be processed by the linker. But on
> Darwin, debug info is not processed by the linker at all, instead a tool
> called dsymutil (cf. llvm/tools/dsymutil) serves a conceptually similar
> task to dwp and archives the debug info from the .o files into a .dSYM
> bundle, separate from the executable.
>

Same conclusion (Darwin isn't interested in Split DWARF), but slightly
different reasons:

Split DWARF was intended to reduce the number of bytes needed to be sent to
the linker - that problem exists in the current Darwin distribution model,
but no one on Darwin seems to have found it to be a problem in practice
(basically it was a problem for Google - anyone using a distributed build
might eventually have this as a concern/bottleneck/problem).

(Split DWARF also reduces the size of the final binary, which is also
valuable - and Darwin doesn't have that problem. Basically Darwin's
distribution model is more or less like the "single file Split DWARF" mode
that the DWARFv5 spec talks about - where you don't actually split it into
a separate file, but you leave it in sections the linker will
discard/ignore)

But yeah, the LLVM implementations of Split DWARF and DWP haven't taken any
real concern to work on anything other than ELF, to the best of my
knowledge (of the parts I worked on, of the parts I've worked with, etc).

- Dave


>
> -- adrian
>
> > On Feb 27, 2019, at 9:56 AM, Eric Christopher <echristo@gmail.com>
> wrote:
> >
> > (Adding in Adrian)
> >
> > While llvm-dwp will run just fine on osx as a program, it's not
> > intended for the platform. I don't know if there are any plans for
> > dwarf5-esque split dwarf on apple platforms. Adrian might be able to
> > comment more.
> >
> > -eric
> >
> > On Wed, Feb 27, 2019 at 9:40 AM Simon Marchi <simon.marchi@polymtl.ca>
> wrote:
> >>
> >> On 2019-02-27 12:22, Tom Tromey wrote:
> >>>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> >>>
> >>> Simon> So my impression now is that dwp doesn't apply to non-ELF
> >>> projects.
> >>> Simon> It is built as part of gold, which itself deals only with ELF,
> >>> AFAIK.
> >>> Simon> I tried to build gold on AIX, without success.
> >>>
> >>> I think there's also llvm-dwp.  Does it work on Mach-O?
> >>>
> >>> Tom
> >>
> >> Of course, there's llvm-dwp, it's the reason this patch was written :).
> >>
> >> Maybe David (in CC) can help answer this?
> >>
> >> Simon
>
>