[PATCH] D29929: [ELF] - Do not segfault when using -r and section groups.

Rafael Avila de Espindola via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 14 05:44:31 PST 2017


My preference would be to error on this broken input if at all possible.

Cheers,
Rafael

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar created this revision.
>
> If we had SHT_GROUP sections, then when -r was used we might crash.
> This is PR31952.
>
> Issue happened because we emited relocation section though its target was discared
> because was a member of duplicated group.
> More details about how issue was revealed available in commens at https://reviews.llvm.org/D29920 page.
>
> Fix was not to create SHT_REL[A] sections for -r when target was discarded.
>
> Testcase based on one from @phosek (https://reviews.llvm.org/D29920).
> Interesting that I was not able to create the test without yaml2obj.
> But I think I could do that using GNU as.
>
> If we have code below:
>
>   .section .text,"axG", at progbits,foo,comdat
>   .quad bar
>
> Then output using GNU as is.
>
>   as test.s -o test.as
>   readelf -a test.as
>   COMDAT group section [    1] `.group' [foo] contains 1 sections:
>      [Index]    Name
>      [    5]   .text
>
> But llvm-as puts .rela section to the group either:
>
>   llvm-mc -filetype=obj -triple=x86_64-pc-linux test.s -o test.o
>   readelf -a test.o
>   COMDAT group section [    3] `.group' [foo] contains 2 sections:
>      [Index]    Name
>      [    4]   .text
>      [    5]   .rela.text
>
> So GNU as does not put rela section to the group. That why issue happens I believe.
> So looks objects created by llvm-as are not affected by issue. Because .rela.text from output above
> is discarded as member of duplicated group earlier, when we hande SHT_GROUP in initializeSections().
>
>
> https://reviews.llvm.org/D29929
>
> Files:
>   ELF/InputFiles.cpp
>   test/ELF/relocation-group.test
>
>
> Index: test/ELF/relocation-group.test
> ===================================================================
> --- test/ELF/relocation-group.test
> +++ test/ELF/relocation-group.test
> @@ -0,0 +1,36 @@
> +# RUN: yaml2obj %s -o %t.o
> +# RUN: ld.lld %t.o %t.o -o %t -r
> +# RUN: llvm-readobj -s %t.o | FileCheck %s
> +
> +# CHECK:     Name: .text.foo
> +# CHECK:     Name: .rela.text.foo
> +
> +--- !ELF
> +FileHeader:
> +  Class:           ELFCLASS64
> +  Data:            ELFDATA2LSB
> +  Type:            ET_REL
> +  Machine:         EM_X86_64
> +Sections:
> +  - Name:            .group
> +    Type:            SHT_GROUP
> +    Link:            .symtab
> +    Info:            foo
> +    Members:
> +      - SectionOrType:    GRP_COMDAT
> +      - SectionOrType:    .text.foo
> +  - Name:            .text.foo
> +    Type:            SHT_PROGBITS
> +    Flags:           [ SHF_ALLOC, SHF_EXECINSTR, SHF_GROUP ]
> +  - Name:            .rela.text.foo
> +    Type:            SHT_RELA
> +    Flags:           [ SHF_INFO_LINK ]
> +    Link:            .symtab
> +    Info:            .text.foo
> +    Relocations:
> +      - Offset:          0x0000000000000000
> +        Symbol:          foo
> +        Type:            R_X86_64_64
> +Symbols:
> +  Global:
> +    - Name:            foo
> Index: ELF/InputFiles.cpp
> ===================================================================
> --- ELF/InputFiles.cpp
> +++ ELF/InputFiles.cpp
> @@ -367,17 +367,20 @@
>      return &InputSection<ELFT>::Discarded;
>    case SHT_RELA:
>    case SHT_REL: {
> +    // Find the relocation target section and associate this
> +    // section with it. Target can be discarded, for example
> +    // if it is a duplicated member of SHT_GROUP section, we
> +    // do not create or proccess relocatable sections then.
> +    InputSectionBase<ELFT> *Target = getRelocTarget(Sec);
> +    if (!Target)
> +      return nullptr;
> +
>      // This section contains relocation information.
>      // If -r is given, we do not interpret or apply relocation
>      // but just copy relocation sections to output.
>      if (Config->Relocatable)
>        return make<InputSection<ELFT>>(this, &Sec, Name);
>  
> -    // Find the relocation target section and associate this
> -    // section with it.
> -    InputSectionBase<ELFT> *Target = getRelocTarget(Sec);
> -    if (!Target)
> -      return nullptr;
>      if (Target->FirstRelocation)
>        fatal(toString(this) +
>              ": multiple relocation sections to one section are not supported");
>
>
> Index: test/ELF/relocation-group.test
> ===================================================================
> --- test/ELF/relocation-group.test
> +++ test/ELF/relocation-group.test
> @@ -0,0 +1,36 @@
> +# RUN: yaml2obj %s -o %t.o
> +# RUN: ld.lld %t.o %t.o -o %t -r
> +# RUN: llvm-readobj -s %t.o | FileCheck %s
> +
> +# CHECK:     Name: .text.foo
> +# CHECK:     Name: .rela.text.foo
> +
> +--- !ELF
> +FileHeader:
> +  Class:           ELFCLASS64
> +  Data:            ELFDATA2LSB
> +  Type:            ET_REL
> +  Machine:         EM_X86_64
> +Sections:
> +  - Name:            .group
> +    Type:            SHT_GROUP
> +    Link:            .symtab
> +    Info:            foo
> +    Members:
> +      - SectionOrType:    GRP_COMDAT
> +      - SectionOrType:    .text.foo
> +  - Name:            .text.foo
> +    Type:            SHT_PROGBITS
> +    Flags:           [ SHF_ALLOC, SHF_EXECINSTR, SHF_GROUP ]
> +  - Name:            .rela.text.foo
> +    Type:            SHT_RELA
> +    Flags:           [ SHF_INFO_LINK ]
> +    Link:            .symtab
> +    Info:            .text.foo
> +    Relocations:
> +      - Offset:          0x0000000000000000
> +        Symbol:          foo
> +        Type:            R_X86_64_64
> +Symbols:
> +  Global:
> +    - Name:            foo
> Index: ELF/InputFiles.cpp
> ===================================================================
> --- ELF/InputFiles.cpp
> +++ ELF/InputFiles.cpp
> @@ -367,17 +367,20 @@
>      return &InputSection<ELFT>::Discarded;
>    case SHT_RELA:
>    case SHT_REL: {
> +    // Find the relocation target section and associate this
> +    // section with it. Target can be discarded, for example
> +    // if it is a duplicated member of SHT_GROUP section, we
> +    // do not create or proccess relocatable sections then.
> +    InputSectionBase<ELFT> *Target = getRelocTarget(Sec);
> +    if (!Target)
> +      return nullptr;
> +
>      // This section contains relocation information.
>      // If -r is given, we do not interpret or apply relocation
>      // but just copy relocation sections to output.
>      if (Config->Relocatable)
>        return make<InputSection<ELFT>>(this, &Sec, Name);
>  
> -    // Find the relocation target section and associate this
> -    // section with it.
> -    InputSectionBase<ELFT> *Target = getRelocTarget(Sec);
> -    if (!Target)
> -      return nullptr;
>      if (Target->FirstRelocation)
>        fatal(toString(this) +
>              ": multiple relocation sections to one section are not supported");


More information about the llvm-commits mailing list