[llvm] r312756 - [llvm-objcopy] Add support for special section indexes in symbol table greater than SHN_LORESERVE
Rafael Avila de Espindola via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 11 16:42:09 PDT 2017
I think the two self consistent options are
* Passing everything >= SHN_LORESERVE and not SHN_XINDEX unchanged.
* Check the OS and Arch specific values we know of, but for the correct
OS and Arch.
Currently we try check for hexagon only values on x86 for example.
Cheers,
Rafael
Jake Ehrlich <jakehehrlich at google.com> writes:
> 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
>>
More information about the llvm-commits
mailing list