[llvm] r312756 - [llvm-objcopy] Add support for special section indexes in symbol table greater than SHN_LORESERVE
Sean Silva via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 11 16:09:36 PDT 2017
For now we can restrict it to SHN_ABS and SHN_COMMON, no?
On Sep 11, 2017 3:13 PM, "Jake Ehrlich via llvm-commits" <
llvm-commits at lists.llvm.org> wrote:
This is discussed a bit in review here: https://reviews.llvm.org/D37393
Basically Roland was uncomfortable with assuming we can handle everything
above SHN_LORESERVE uniformly. To quote Roland
"""
It's probably OK to just pass all other >= SHN_LORESERVE values through
blindly, but I'm not sure it's the best idea.
Their meaning and appropriate handling depends on each specific value
defined by a particular machine or OS.
For all the ones I'm familiar with, just passing it through is all you need
to do. But I'm not entirely sanguine about assuming that uniformly.
I think it's probably better to handle each known case individually, and
error out for values we don't expect.
"""
Right now we just handle everything that LLVM directly supports (or at
least has an enum for). This ensures we fail on any index that might not be
handled correctly. We don't have a counterexample to show that this is the
case however. We also don't have proof that no such case exists. This is
airing on the side of caution at the cost of having architecture specific
enum usage outside of architecture-specific code. No one, my self included,
seems to really know what's best here.
Maybe this is a discussion for llvm-dev?
On Mon, Sep 11, 2017 at 2:44 PM Rafael Avila de Espindola via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Petr Hosek via llvm-commits <llvm-commits at lists.llvm.org> writes:
>
> > Author: phosek
> > Date: Thu Sep 7 16:02:50 2017
> > New Revision: 312756
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=312756&view=rev
> > Log:
> > [llvm-objcopy] Add support for special section indexes in symbol table
> greater than SHN_LORESERVE
> >
> > As is indexes above SHN_LORESERVE will not be handled correctly because
> > they'll be treated as indexes of sections rather than special values
> > that should just be copied. This change adds support to copy them
> > though.
> >
> > Patch by Jake Ehrlich
> >
> > Differential Revision: https://reviews.llvm.org/D37393
> >
> > Added:
> > llvm/trunk/test/tools/llvm-objcopy/abs-symbol.test
> > llvm/trunk/test/tools/llvm-objcopy/common-symbol.test
> > llvm/trunk/test/tools/llvm-objcopy/section-index-unsupported.test
> > Modified:
> > llvm/trunk/tools/llvm-objcopy/Object.cpp
> > llvm/trunk/tools/llvm-objcopy/Object.h
> >
> > Added: llvm/trunk/test/tools/llvm-objcopy/abs-symbol.test
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/
> llvm-objcopy/abs-symbol.test?rev=312756&view=auto
> > ============================================================
> ==================
> > --- llvm/trunk/test/tools/llvm-objcopy/abs-symbol.test (added)
> > +++ llvm/trunk/test/tools/llvm-objcopy/abs-symbol.test Thu Sep 7
> 16:02:50 2017
> > @@ -0,0 +1,36 @@
> > +# RUN: yaml2obj %s > %t
> > +# RUN: llvm-objcopy %t %t2
> > +# RUN: llvm-readobj -symbols %t2 | FileCheck %s
> > +
> > +!ELF
> > +FileHeader:
> > + Class: ELFCLASS64
> > + Data: ELFDATA2LSB
> > + Type: ET_EXEC
> > + Machine: EM_X86_64
> > +Symbols:
> > + Global:
> > + - Name: test
> > + Index: SHN_ABS
> > + Value: 0x1234
> > +
> > +#CHECK: Symbols [
> > +#CHECK-NEXT: Symbol {
> > +#CHECK-NEXT: Name:
> > +#CHECK-NEXT: Value: 0x0
> > +#CHECK-NEXT: Size: 0
> > +#CHECK-NEXT: Binding: Local (0x0)
> > +#CHECK-NEXT: Type: None (0x0)
> > +#CHECK-NEXT: Other: 0
> > +#CHECK-NEXT: Section: Undefined (0x0)
> > +#CHECK-NEXT: }
> > +#CHECK-NEXT: Symbol {
> > +#CHECK-NEXT: Name: test
> > +#CHECK-NEXT: Value: 0x1234
> > +#CHECK-NEXT: Size: 0
> > +#CHECK-NEXT: Binding: Global (0x1)
> > +#CHECK-NEXT: Type: None (0x0)
> > +#CHECK-NEXT: Other: 0
> > +#CHECK-NEXT: Section: Absolute (0xFFF1)
> > +#CHECK-NEXT: }
> > +#CHECK-NEXT:]
> >
> > Added: llvm/trunk/test/tools/llvm-objcopy/common-symbol.test
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/
> llvm-objcopy/common-symbol.test?rev=312756&view=auto
> > ============================================================
> ==================
> > --- llvm/trunk/test/tools/llvm-objcopy/common-symbol.test (added)
> > +++ llvm/trunk/test/tools/llvm-objcopy/common-symbol.test Thu Sep 7
> 16:02:50 2017
> > @@ -0,0 +1,84 @@
> > +# RUN: yaml2obj %s > %t
> > +# RUN: llvm-objcopy %t %t2
> > +# RUN: llvm-readobj -symbols %t2 | FileCheck %s
> > +
> > +!ELF
> > +FileHeader:
> > + Class: ELFCLASS64
> > + Data: ELFDATA2LSB
> > + Type: ET_EXEC
> > + Machine: EM_X86_64
> > +Symbols:
> > + Global:
> > + - Name: test
> > + Index: SHN_COMMON
> > + Value: 0x1234
> > + - Name: test2
> > + Index: SHN_HEXAGON_SCOMMON
> > + Value: 0x1235
> > + - Name: test3
> > + Index: SHN_HEXAGON_SCOMMON_2
> > + Value: 0x1236
> > + - Name: test4
> > + Index: SHN_HEXAGON_SCOMMON_4
> > + Value: 0x1237
> > + - Name: test5
> > + Index: SHN_HEXAGON_SCOMMON_8
> > + Value: 0x1238
> > +
> > +#CHECK: Symbols [
> > +#CHECK-NEXT: Symbol {
> > +#CHECK-NEXT: Name:
> > +#CHECK-NEXT: Value: 0x0
> > +#CHECK-NEXT: Size: 0
> > +#CHECK-NEXT: Binding: Local (0x0)
> > +#CHECK-NEXT: Type: None (0x0)
> > +#CHECK-NEXT: Other: 0
> > +#CHECK-NEXT: Section: Undefined (0x0)
> > +#CHECK-NEXT: }
> > +#CHECK-NEXT: Symbol {
> > +#CHECK-NEXT: Name: test
> > +#CHECK-NEXT: Value: 0x1234
> > +#CHECK-NEXT: Size: 0
> > +#CHECK-NEXT: Binding: Global (0x1)
> > +#CHECK-NEXT: Type: None (0x0)
> > +#CHECK-NEXT: Other: 0
> > +#CHECK-NEXT: Section: Common (0xFFF2)
> > +#CHECK-NEXT: }
> > +#CHECK-NEXT: Symbol {
> > +#CHECK-NEXT: Name: test2
> > +#CHECK-NEXT: Value: 0x1235
> > +#CHECK-NEXT: Size: 0
> > +#CHECK-NEXT: Binding: Global (0x1)
> > +#CHECK-NEXT: Type: None (0x0)
> > +#CHECK-NEXT: Other: 0
> > +#CHECK-NEXT: Section: Processor Specific (0xFF00)
> > +#CHECK-NEXT: }
> > +#CHECK-NEXT: Symbol {
> > +#CHECK-NEXT: Name: test3
> > +#CHECK-NEXT: Value: 0x1236
> > +#CHECK-NEXT: Size: 0
> > +#CHECK-NEXT: Binding: Global (0x1)
> > +#CHECK-NEXT: Type: None (0x0)
> > +#CHECK-NEXT: Other: 0
> > +#CHECK-NEXT: Section: Processor Specific (0xFF02)
> > +#CHECK-NEXT: }
> > +#CHECK-NEXT: Symbol {
> > +#CHECK-NEXT: Name: test4
> > +#CHECK-NEXT: Value: 0x1237
> > +#CHECK-NEXT: Size: 0
> > +#CHECK-NEXT: Binding: Global (0x1)
> > +#CHECK-NEXT: Type: None (0x0)
> > +#CHECK-NEXT: Other: 0
> > +#CHECK-NEXT: Section: Processor Specific (0xFF03)
> > +#CHECK-NEXT: }
> > +#CHECK-NEXT: Symbol {
> > +#CHECK-NEXT: Name: test5
> > +#CHECK-NEXT: Value: 0x1238
> > +#CHECK-NEXT: Size: 0
> > +#CHECK-NEXT: Binding: Global (0x1)
> > +#CHECK-NEXT: Type: None (0x0)
> > +#CHECK-NEXT: Other: 0
> > +#CHECK-NEXT: Section: Processor Specific (0xFF04)
> > +#CHECK-NEXT: }
> > +#CHECK-NEXT:]
> >
> > Added: llvm/trunk/test/tools/llvm-objcopy/section-index-unsupported.test
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/
> llvm-objcopy/section-index-unsupported.test?rev=312756&view=auto
> > ============================================================
> ==================
> > --- llvm/trunk/test/tools/llvm-objcopy/section-index-unsupported.test
> (added)
> > +++ llvm/trunk/test/tools/llvm-objcopy/section-index-unsupported.test
> Thu Sep 7 16:02:50 2017
> > @@ -0,0 +1,15 @@
> > +# RUN: yaml2obj %s > %t
> > +# RUN: not llvm-objcopy %t %t2 2>&1 >/dev/null | FileCheck %s
> > +
> > +!ELF
> > +FileHeader:
> > + Class: ELFCLASS64
> > + Data: ELFDATA2LSB
> > + Type: ET_EXEC
> > + Machine: EM_X86_64
> > +Symbols:
> > + Global:
> > + - Name: test
> > + Index: 0xff05
> > +
> > +# CHECK: [[_:.*]] Symbol 'test' has unsupported value greater than or
> equal to SHN_LORESERVE: 65285
> >
> > Modified: llvm/trunk/tools/llvm-objcopy/Object.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-
> objcopy/Object.cpp?rev=312756&r1=312755&r2=312756&view=diff
> > ============================================================
> ==================
> > --- llvm/trunk/tools/llvm-objcopy/Object.cpp (original)
> > +++ llvm/trunk/tools/llvm-objcopy/Object.cpp Thu Sep 7 16:02:50 2017
> > @@ -90,14 +90,54 @@ void StringTableSection::writeSection(Fi
> > StrTabBuilder.write(Out.getBufferStart() + Offset);
> > }
> >
> > +static bool isValidReservedSectionIndex(uint16_t Index) {
> > + switch (Index) {
> > + case SHN_ABS:
> > + case SHN_COMMON:
> > + case SHN_HEXAGON_SCOMMON:
> > + case SHN_HEXAGON_SCOMMON_2:
> > + case SHN_HEXAGON_SCOMMON_4:
> > + case SHN_HEXAGON_SCOMMON_8:
> > + return true;
> > + default:
> > + return false;
> > + }
> > +}
>
> Should this function just be "Index >= SHN_LORESERVE"? It is strange to
> check for Hexagon values in non architecture-specific code. It also
> looks like you just need to check for "is this really an section
> index", which is what SHN_LORESERVE provides.
>
> Cheers,
> Rafael
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170911/383b2763/attachment.html>
More information about the llvm-commits
mailing list