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

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

[Jordan Rupprecht via gdb-patches]

When loading dwp files, we create an array of elf sections indexed by the section index in the dwp file. The size of this array is calculated by section_count + 1 (the +1 handling the null section). However, when loading the bfd file, strtab/symtab sections are not added to the list, nor do they increment section_count, so section_count is actually smaller than the number of sections.

This happens to work when using GNU dwp, which lays out .debug section first, with sections like .shstrtab coming at the end. Other tools, like llvm-dwp, put .strtab first, and gdb crashes when loading those dwp files.

For instance, with the current state of gdb, loading a file like this:
$ readelf -SW <file.dwp>
[ 0] <empty>
[ 1] .debug_foo PROGBITS ...
[ 2] .strtab    STRTAB ...

... results in section_count = 2 (.debug is the only thing placed into bfd->sections, so section_count + 1 == 2), and sectp->this_idx = 1 when mapping over .debug_foo in dwarf2_locate_common_dwp_sections, which passes the assertion that 1 < 2.

However, using a dwp file produced by llvm-dwp:
$ readelf -SW <file.dwp>
[ 0] <empty>
[ 1] .strtab    STRTAB ...
[ 2] .debug_foo PROGBITS ...
... results in section_count = 2 (.debug is the only thing placed into bfd->sections, so section_count + 1 == 2), and sectp->this_idx = 2 when mapping over .debug_foo in dwarf2_locate_common_dwp_sections, which fails the assertion that 2 < 2.

This patch changes the calculation of section_count to look at the actual highest this_idx value we see instead of assuming that all strtab/symtab sections come at the end.
---
 gdb/ChangeLog    |  6 ++++++
 gdb/dwarf2read.c | 17 +++++++++++++++--
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6a551dfa64..c789b34890 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2019-02-22  Jordan Rupprecht  <rupprecht@google.com>
+
+	* dwarf2read.c (get_largest_section_index): new function
+	(open_and_init_dwp_file): Call get_largest_section_index to
+	calculate dwp_file->num_sections.
+
 2019-02-22  Simon Marchi  <simon.marchi@polymtl.ca>
 
 	* MAINTAINERS: Update my email address.
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 98f46e0416..07a9d4ea5d 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -13182,6 +13182,18 @@ open_dwp_file (struct dwarf2_per_objfile *dwarf2_per_objfile,
   return NULL;
 }
 
+/* Return the largest section index */
+static int
+get_largest_section_index(bfd *bfd)
+{
+  int max_sec_idx = 0;
+  asection *sectp;
+  for (sectp = bfd->sections; sectp != NULL; sectp = sectp->next)
+    max_sec_idx = std::max (max_sec_idx, elf_section_data (sectp)->this_idx);
+
+  return max_sec_idx;
+}
+
 /* Initialize the use of the DWP file for the current objfile.
    By convention the name of the DWP file is ${objfile}.dwp.
    The result is NULL if it can't be found.  */
@@ -13230,8 +13242,9 @@ 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;
+  /* +1: so that dwp_file->elf_sections[max_idx] is valid */
+  dwp_file->num_sections =
+    get_largest_section_index (dwp_file->dbfd.get()) + 1;
   dwp_file->elf_sections =
     OBSTACK_CALLOC (&objfile->objfile_obstack,
 		    dwp_file->num_sections, asection *);
-- 
2.21.0.rc0.258.g878e2cd30e-goog

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

[Simon Marchi]

On 2019-02-22 19:53, Jordan Rupprecht via gdb-patches wrote:
> When loading dwp files, we create an array of elf sections indexed by
> the section index in the dwp file. The size of this array is
> calculated by section_count + 1 (the +1 handling the null section).
> However, when loading the bfd file, strtab/symtab sections are not
> added to the list, nor do they increment section_count, so
> section_count is actually smaller than the number of sections.

Just wondering, is this the expected behavior of BFD, to not make the 
strtab section count as a section (as far as bfd_count_sections is 
concerned)?  If so, why?

Otherwise can we just elf_numsections instead of bfd_count_sections?  
Since we index the array by ELF section index, using the number of ELF 
sections seems appropriate, it should always match.  We wouldn't need 
the +1 then.

> This happens to work when using GNU dwp, which lays out .debug section
> first, with sections like .shstrtab coming at the end. Other tools,
> like llvm-dwp, put .strtab first, and gdb crashes when loading those
> dwp files.
> 
> For instance, with the current state of gdb, loading a file like this:
> $ readelf -SW <file.dwp>
> [ 0] <empty>
> [ 1] .debug_foo PROGBITS ...
> [ 2] .strtab    STRTAB ...
> 
> ... results in section_count = 2 (.debug is the only thing placed into
> bfd->sections, so section_count + 1 == 2), and sectp->this_idx = 1
> when mapping over .debug_foo in dwarf2_locate_common_dwp_sections,
> which passes the assertion that 1 < 2.
> 
> However, using a dwp file produced by llvm-dwp:
> $ readelf -SW <file.dwp>
> [ 0] <empty>
> [ 1] .strtab    STRTAB ...
> [ 2] .debug_foo PROGBITS ...
> ... results in section_count = 2 (.debug is the only thing placed into
> bfd->sections, so section_count + 1 == 2), and sectp->this_idx = 2
> when mapping over .debug_foo in dwarf2_locate_common_dwp_sections,
> which fails the assertion that 2 < 2.
> 
> This patch changes the calculation of section_count to look at the
> actual highest this_idx value we see instead of assuming that all
> strtab/symtab sections come at the end.

Thanks for the detailed explanation.

Here are just some formatting comments.

> ---
>  gdb/ChangeLog    |  6 ++++++
>  gdb/dwarf2read.c | 17 +++++++++++++++--
>  2 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 6a551dfa64..c789b34890 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,9 @@
> +2019-02-22  Jordan Rupprecht  <rupprecht@google.com>
> +
> +	* dwarf2read.c (get_largest_section_index): new function

Capital letter and period at the end ("New function.").

> +	(open_and_init_dwp_file): Call get_largest_section_index to
> +	calculate dwp_file->num_sections.
> +
>  2019-02-22  Simon Marchi  <simon.marchi@polymtl.ca>
> 
>  	* MAINTAINERS: Update my email address.
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 98f46e0416..07a9d4ea5d 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -13182,6 +13182,18 @@ open_dwp_file (struct dwarf2_per_objfile
> *dwarf2_per_objfile,
>    return NULL;
>  }
> 
> +/* Return the largest section index */

Period at the end, followed by two spaces:

/* Return the largest section index.  */

> +static int
> +get_largest_section_index(bfd *bfd)

Space before the parentheses:

get_largest_section_index (bfd *bfd)

> +{
> +  int max_sec_idx = 0;
> +  asection *sectp;

You can declare sectp in the for loop header.

> +  for (sectp = bfd->sections; sectp != NULL; sectp = sectp->next)
> +    max_sec_idx = std::max (max_sec_idx, elf_section_data 
> (sectp)->this_idx);
> +
> +  return max_sec_idx;
> +}
> +
>  /* Initialize the use of the DWP file for the current objfile.
>     By convention the name of the DWP file is ${objfile}.dwp.
>     The result is NULL if it can't be found.  */
> @@ -13230,8 +13242,9 @@ 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;
> +  /* +1: so that dwp_file->elf_sections[max_idx] is valid */

Period and two spaces at the end.

> +  dwp_file->num_sections =
> +    get_largest_section_index (dwp_file->dbfd.get()) + 1;

Normally, the = should be on the second line when breaking assignments 
(despite the counter example just below).  But in this case, it can fit 
on a single line I believe.

>    dwp_file->elf_sections =
>      OBSTACK_CALLOC (&objfile->objfile_obstack,
>  		    dwp_file->num_sections, asection *);

Thanks,

Simon

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

[Jordan Rupprecht via gdb-patches]

On Sat, Feb 23, 2019 at 8:01 PM Simon Marchi <simon.marchi@polymtl.ca> wrote:
>
> On 2019-02-22 19:53, Jordan Rupprecht via gdb-patches wrote:
> > When loading dwp files, we create an array of elf sections indexed by
> > the section index in the dwp file. The size of this array is
> > calculated by section_count + 1 (the +1 handling the null section).
> > However, when loading the bfd file, strtab/symtab sections are not
> > added to the list, nor do they increment section_count, so
> > section_count is actually smaller than the number of sections.
>
> Just wondering, is this the expected behavior of BFD, to not make the
> strtab section count as a section (as far as bfd_count_sections is
> concerned)?  If so, why?
I'm not very familiar with bfd, so I don't know if it's expected. It
seems that bfd->sections contains "interesting" sections (e.g.
progbits), and treats sections like strtab/symtab as a kind of
metadata section that get stored differently. The method I'm stepping
through is bfd_section_from_shdr here:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=bfd/elf.c;h=f16acaa08d8e24af9af069efcdcb244b2c19c734;hb=HEAD#l1994.
Running that method over a normal .dwp file, all the .debug_* methods
get added to sections, but .strtab/.symtab/.shstrtab don't.

So it *seems* like that's expected behavior, but I can't say for sure
that it is.

>
> Otherwise can we just elf_numsections instead of bfd_count_sections?
> Since we index the array by ELF section index, using the number of ELF
> sections seems appropriate, it should always match.  We wouldn't need
> the +1 then.

Don't know how I didn't see that. Now I don't need
get_largest_section_index() anymore. Thanks!
All your comments are basically not relevant any more :)

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

[Jordan Rupprecht via gdb-patches]

[-- Attachment #1: Type: text/plain, Size: 1318 bytes --]

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

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4849 bytes --]

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

[Simon Marchi]

On 2019-02-25 15:17, Jordan Rupprecht via gdb-patches wrote:
> Don't know how I didn't see that. Now I don't need
> get_largest_section_index() anymore. Thanks!
> All your comments are basically not relevant any more :)

Well, it's useful to learn for next time :).  Are you going to send an 
updated version?

Thanks,

Simon

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

[Simon Marchi]

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

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

[Jordan Rupprecht via gdb-patches]

[-- Attachment #1: Type: text/plain, Size: 1720 bytes --]

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

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4849 bytes --]

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

[Tom Tromey]

>>>>> "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

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

[Simon Marchi]

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

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

[Simon Marchi]

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

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

[Tom Tromey]

>>>>> "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

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

[Simon Marchi]

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

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

[Eric Christopher]

(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

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

[Adrian Prantl]

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

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

[Simon Marchi]

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

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

[David Blaikie]

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
>
>