Page MenuHomePhabricator

[ELF] Fix DT_NEEDED value
ClosedPublic

Authored by davide on Sep 8 2016, 1:02 PM.

Details

Summary

The absolute path value might not exist at runtime.
To quote Eric Van Gyzen who reported the bug:

18:52 < vangyzen> That path is a build area on the build host.
18:53 < vangyzen> e.g. /build/product/X.Y.Z/lib/libfoo.so
18:54 < vangyzen> On the running product, it's simply /lib.

https://llvm.org/bugs/show_bug.cgi?id=30330

Diff Detail

Repository
rL LLVM

Event Timeline

davide updated this revision to Diff 70745.Sep 8 2016, 1:02 PM
davide retitled this revision from to [ELF] Fix DT_NEEDED value.
davide updated this object.
davide added a reviewer: ruiu.
davide added a subscriber: llvm-commits.

Also, the shared.s change is a side effect.
We can't do CHECK-NEXT

davide added a comment.Sep 8 2016, 1:03 PM

Also, the shared.s change is a side effect.
We can't do CHECK-NEXT

Section {
  Index: 7
  Name: .dynamic (46)
  Type: SHT_DYNAMIC (0x6)
  Flags [ (0x3)
    SHF_ALLOC (0x2)
    SHF_WRITE (0x1)
  ]
  Address: 0x12000
  Offset: 0x2000
  Size: 96
  Link: 4
  Info: 0
  AddressAlignment: 4
  EntrySize: 8
  SectionData (
    0000: 1D000000 10000000 01000000 18000000  |................|
    0010: 11000000 E4010100 12000000 10000000  |................|
    0020: 13000000 08000000 06000000 50010100  |............P...|
    0030: 0B000000 10000000 05000000 B8010100  |................|
    0040: 0A000000 29000000 04000000 90010100  |....)...........|
    0050: 15000000 00000000 00000000 00000000  |................|
  )
}

because the section data already contains a ).

ruiu edited edge metadata.Sep 8 2016, 1:08 PM

Did you check that this behavior matches GNU?

davide added a comment.Sep 8 2016, 1:13 PM
In D24363#537577, @ruiu wrote:

Did you check that this behavior matches GNU?

Yes, take a look at the bug report.

ruiu added inline comments.Sep 8 2016, 1:34 PM
ELF/OutputSections.cpp
689–690 ↗(On Diff #70745)

Currently, the default soname (used if DT_SONAME is absent) if an absolute path of a library, which I think wrong. Look at SharedFile<ELFT>::parseSoName. You want to fix SoName default value in that function instead of fixing it here.

davide updated this revision to Diff 70746.Sep 8 2016, 1:51 PM
davide edited edge metadata.
ruiu added inline comments.Sep 8 2016, 1:53 PM
ELF/InputFiles.cpp
481 ↗(On Diff #70746)

Stripping path parts from the default seems fine, but

494 ↗(On Diff #70746)

we should respect DT_SONAME value, no?

davide updated this revision to Diff 70749.Sep 8 2016, 2:18 PM
ruiu accepted this revision.Sep 8 2016, 2:25 PM
ruiu edited edge metadata.

LGTM

test/ELF/shared.s
122 ↗(On Diff #70749)

Probably you just want to remove this line.

This revision is now accepted and ready to land.Sep 8 2016, 2:25 PM
This revision was automatically updated to reflect the committed changes.