[Bug default/24249] Section offsets not monotonically increasing

[Bug default/24249] Section offsets not monotonically increasing

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=24249

--- Comment #7 from Tom de Vries <vries at gcc dot gnu.org> ---
Created attachment 11635
  --> https://sourceware.org/bugzilla/attachment.cgi?id=11635&action=edit
Tentative patch

(In reply to Jakub Jelinek from comment #5)
> Pedantically, ELF doesn't require that.  But I view it as a QoI thing, there
> is no reason for a sane ELF producer to just put sections randomly in
> .shstrtab, sorting them by increasing .sh_offset is most reasonable.  And,
> both prelink and dwz and other tools just choose not to optimize if the
> producer didn't do a reasonable job.

I think it's reasonable to draw the line somewhere, so I'd understand it if
this gets marked resolved-wontfix.

OTOH, I'm not happy with not having a workaround (the ability to run say an elf
sanitizer or some such to fix this trivial problem to allow dwz to run on the
sanitized elf).

This tentative patch fixes up the bad-quality input in fdopen_dso in a fairly
orthogonal way, by sorting dso->shdr and dso->scn according to sh_offset, and
updating dso->ehdr.e_shstrndx.
...
$ gcc hello.c -g -fuse-ld=gold -Wl,--gdb-index
$ dwz a.out
dwz: Section offsets in a.out not monotonically increasing, fixing up
$ echo $?
0
$ readelf -S a.out
Section Headers:
  [Nr] Name              Type             Address           Offset
       Size              EntSize          Flags  Link  Info  Align
  ...
  [37] .shstrtab         STRTAB           0000000000000000  0000284f
       000000000000017f  0000000000000000           0     0     1
  [38] .gdb_index        PROGBITS         0000000000000000  00003390
       00000000000022f3  0000000000000000           0     0     4
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24249] Section offsets not monotonically increasing

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=24249

Mark Wielaard <mark at klomp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mark at klomp dot org

--- Comment #11 from Mark Wielaard <mark at klomp dot org> ---
(In reply to Tom de Vries from comment #10)
> (In reply to Jakub Jelinek from comment #9)
> > e.g. if we process *.debug files stripped into file
> > by objcopy or eu-strip, won't this shdr reordering then break the matching
> > between original binary and *.debug file?
> 
> Indeed the ordering of entries in the section table in the original binary
> and the .debug file will not be the same.
> 
> I'm not aware though of a tool that relies on this ordering to be the same.

I think eu-unstrip does deal with differences in ordering, but you might want
to test whether it still works correctly.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24249] Section offsets not monotonically increasing

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=24249

--- Comment #8 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Tom de Vries from comment #7)
> This tentative patch fixes up the bad-quality input in fdopen_dso in a
> fairly orthogonal way, by sorting dso->shdr and dso->scn according to
> sh_offset, and updating dso->ehdr.e_shstrndx.

Review remark from Micha: If this patch changes order of the section header
table, then it should also update the section index member (st_shndx) of symbol
table entries.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24249] Section offsets not monotonically increasing

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=24249

--- Comment #12 from Tom de Vries <vries at gcc dot gnu.org> ---
patch posted: https://sourceware.org/ml/dwz/2019-q1/msg00108.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24249] Section offsets not monotonically increasing

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=24249

--- Comment #6 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #2)
> That would be a gold bug then.

Refiled as PR24250 - "[gold] Section offsets not monotonically increasing with
--gdb-index"

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24249] New: Section offsets not monotonically increasing

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=24249

            Bug ID: 24249
           Summary: Section offsets not monotonically increasing
           Product: dwz
           Version: unspecified
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: default
          Assignee: nobody at sourceware dot org
          Reporter: vries at gcc dot gnu.org
                CC: dwz at sourceware dot org
  Target Milestone: ---

For a hello world source file hello.c, using the gold linker does not cause
trouble for dwz:
...
$ gcc hello.c -g -fuse-ld=gold
$ dwz a.out 
...

And adding the .gdb_index section using gdb-add-index also works fine with dwz:
...
$ gcc hello.c -g -fuse-ld=gold
$ gdb-add-index a.out
$ dwz a.out 
...

But, I get this error when using the gold linker to generate the .gdb_index
section:
...
$ gcc hello.c -g -fuse-ld=gold -Wl,--gdb-index
$ dwz a.out 
dwz: Section offsets in a.out not monotonically increasing
...

Inspecting the section offsets around .gdb_index:
...
$ readelf -S a.out | grep '[[]' | egrep -C1  'Offset|\.gdb_index'
  [Nr] Name              Type             Address           Offset
  [ 0]                   NULL             0000000000000000  00000000
--
  [31] .debug_str        PROGBITS         0000000000000000  00001b2f
  [32] .gdb_index        PROGBITS         0000000000000000  000034b0
  [33] .debug_ranges     PROGBITS         0000000000000000  000021f0
...
indeed shows that the offsets are not monotonically increasing.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24249] Section offsets not monotonically increasing

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=24249

--- Comment #3 from Tom de Vries <vries at gcc dot gnu.org> ---
Created attachment 11629
  --> https://sourceware.org/bugzilla/attachment.cgi?id=11629&action=edit
dwz-0.12-ignore-nobits.patch

Using this (
https://build.opensuse.org/package/view_file/openSUSE:Factory/dwz/dwz-0.12-ignore-nobits.patch
) patch, we get:
...
$ gcc hello.c -g -fuse-ld=gold -Wl,--gdb-index
$ dwz a.out 
$ readelf -S a.out | grep '[[]' | egrep -C1  'Offset|\.gdb_index'
  [Nr] Name              Type             Address           Offset
  [ 0]                   NULL             0000000000000000  00000000
--
  [31] .debug_str        PROGBITS         0000000000000000  00001a11
  [32] .gdb_index        PROGBITS         0000000000000000  00003390
  [33] .debug_ranges     PROGBITS         0000000000000000  000020d0
...

So, the patch allows dwz to handle the input. Note that the section offsets in
the dwz output are still not monotonically increasing.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24249] Section offsets not monotonically increasing

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=24249

--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
In related PR21191 - "objcopy --only-keep-debug creates non-monotonically
increasing section offsets", it's also dwz that complains.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24249] Section offsets not monotonically increasing

[jakub at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=24249

--- Comment #5 from Jakub Jelinek <jakub at redhat dot com> ---
Pedantically, ELF doesn't require that.  But I view it as a QoI thing, there is
no reason for a sane ELF producer to just put sections randomly in .shstrtab,
sorting them by increasing .sh_offset is most reasonable.  And, both prelink
and dwz and other tools just choose not to optimize if the producer didn't do a
reasonable job.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24249] Section offsets not monotonically increasing

[jakub at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=24249

--- Comment #2 from Jakub Jelinek <jakub at redhat dot com> ---
That would be a gold bug then.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24249] Section offsets not monotonically increasing

[jakub at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=24249

--- Comment #9 from Jakub Jelinek <jakub at redhat dot com> ---
I still have concerns, e.g. if we process *.debug files stripped into file by
objcopy or eu-strip, won't this shdr reordering then break the matching between
original binary and *.debug file?
On the other side, this code has been written primarily for prelink, when dwz
only modifies debug sections perhaps it could away with relying on this or only
care (and reshuffle) the non-allocated sections that way and ignore others.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24249] Section offsets not monotonically increasing

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=24249

--- Comment #10 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #9)
> I still have concerns,

FWIW, dwz-0.12-ignore-nobits.patch handles this PR without reordering of the
section table, so from that point of view it's the easier solution.

> e.g. if we process *.debug files stripped into file
> by objcopy or eu-strip, won't this shdr reordering then break the matching
> between original binary and *.debug file?

Indeed the ordering of entries in the section table in the original binary and
the .debug file will not be the same.

I'm not aware though of a tool that relies on this ordering to be the same.

> On the other side, this code has been written primarily for prelink, when
> dwz only modifies debug sections perhaps it could away with relying on this

Sorry, not sure what you mean here.

> or only care (and reshuffle) the non-allocated sections that way and ignore
> others.

Agreed, that would work for this example.


I start to wonder now whether a mix of the two posted approaches
(dwz-0.12-ignore-nobits.patch and the tentative patch) would be the best
solution:
1. resort the section header based on offset (ignoring nobits offsets, see
PR24251)
2. recalculate the offsets base on the new sizes (in write_dso)
3. unsort the sections back into the original order.
This approach would have the same result as dwz-0.12-ignore-nobits.patch, and
avoid the problem of having to rewrite symbol tables, but would be less
intrusive to write_dso than dwz-0.12-ignore-nobits.patch. WDYT?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24249] Section offsets not monotonically increasing

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=24249

--- Comment #4 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #2)
> That would be a gold bug then.

I read here ( rbc#1366145 - "dwz applied to a dts-compiled binary complains
about section offsets not monotonically increasing", 
https://bugzilla.redhat.com/show_bug.cgi?id=1366145 ):
...
 The .shstrtab section would appear in the middle of the section header table,
but at the end of the actual file. This meant that the file offsets of the
entries in the section header table were no longer in a monotonically
increasing order. Whilst this property is not a requirement of the ELF
standard, it is expected by a variety of other tools which process ELF
binaries.
...

I suppose this is the rationale behind classifying this as gold bug.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24249] Section offsets not monotonically increasing

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=24249

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|enhancement                 |normal

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/24249] Section offsets not monotonically increasing

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=24249

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #13 from Tom de Vries <vries at gcc dot gnu.org> ---
https://sourceware.org/git/?p=dwz.git;a=commit;h=dff012ab3c1f4f6b70d06dc1a224c887ce6339eb

-- 
You are receiving this mail because:
You are on the CC list for the bug.