[llvm] r312756 - [llvm-objcopy] Add support for special section indexes in symbol table greater than SHN_LORESERVE
Jake Ehrlich via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 11 16:31:36 PDT 2017
I would be fine with that personally. I added this to fix an issue we had
with SHN_ABS. I CC'd Roland to get his opinion as well.
On Mon, Sep 11, 2017 at 4:09 PM Sean Silva <chisophugis at gmail.com> wrote:
> 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/7c7ec5fe/attachment.html>
More information about the llvm-commits
mailing list