Index: ELF/LinkerScript.cpp =================================================================== --- ELF/LinkerScript.cpp +++ ELF/LinkerScript.cpp @@ -264,42 +264,12 @@ } template -static SectionKey createKey(InputSectionBase *C, - StringRef OutsecName) { - // When using linker script the merge rules are different. - // Unfortunately, linker scripts are name based. This means that expressions - // like *(.foo*) can refer to multiple input sections that would normally be - // placed in different output sections. We cannot put them in different - // output sections or we would produce wrong results for - // start = .; *(.foo.*) end = .; *(.bar) - // and a mapping of .foo1 and .bar1 to one section and .foo2 and .bar2 to - // another. The problem is that there is no way to layout those output - // sections such that the .foo sections are the only thing between the - // start and end symbols. - - // An extra annoyance is that we cannot simply disable merging of the contents - // of SHF_MERGE sections, but our implementation requires one output section - // per "kind" (string or not, which size/aligment). - // Fortunately, creating symbols in the middle of a merge section is not - // supported by bfd or gold, so we can just create multiple section in that - // case. - typedef typename ELFT::uint uintX_t; - uintX_t Flags = C->Flags & (SHF_MERGE | SHF_STRINGS); - - uintX_t Alignment = 0; - if (isa>(C)) - Alignment = std::max(C->Alignment, C->Entsize); - - return SectionKey{OutsecName, /*Type*/ 0, Flags, Alignment}; -} - -template void LinkerScript::addSection(OutputSectionFactory &Factory, InputSectionBase *Sec, StringRef Name) { OutputSectionBase *OutSec; bool IsNew; - std::tie(OutSec, IsNew) = Factory.create(createKey(Sec, Name), Sec); + std::tie(OutSec, IsNew) = Factory.create(Sec, Name); if (IsNew) OutputSections->push_back(OutSec); OutSec->addSection(Sec); Index: ELF/OutputSections.h =================================================================== --- ELF/OutputSections.h +++ ELF/OutputSections.h @@ -220,7 +220,6 @@ template struct SectionKey { typedef typename std::conditional::type uintX_t; StringRef Name; - uint32_t Type; uintX_t Flags; uintX_t Alignment; }; Index: ELF/OutputSections.cpp =================================================================== --- ELF/OutputSections.cpp +++ ELF/OutputSections.cpp @@ -537,21 +537,68 @@ template static SectionKey createKey(InputSectionBase *C, StringRef OutsecName) { + // The ELF spec just says + // ---------------------------------------------------------------- + // In the first phase, input sections that match in name, type and + // attribute flags should be concatenated into single sections. + // ---------------------------------------------------------------- + // + // However, It is clear that at least some flags have to be ignored for + // section merging. At the very least SHF_GROUP and SHF_COMPRESSED have to be + // ignored. We should not have two output .text sections just because one was + // in a group and another was not for example. + // + // It also seems that that wording was a late addition and didn't get the + // necessary scrutiny. + // + // Merging sections with different flags is expected by some users. One + // reason is that if one file has + // + // int *const bar __attribute__((section(".foo"))) = (int *)0; + // + // gcc with -fPIC will produce a read only .foo section. But if another + // file has + // + // int zed; + // int *const bar __attribute__((section(".foo"))) = (int *)&zed; + // + // gcc with -fPIC will produce a read write section. + // + // Last but not least, when using linker script the merge rules are forced by + // the script. Unfortunately, linker scripts are name based. This means that + // expressions like *(.foo*) can refer to multiple input sections with + // different flags. We cannot put them in different output sections or we + // would produce wrong results for + // + // start = .; *(.foo.*) end = .; *(.bar) + // + // and a mapping of .foo1 and .bar1 to one section and .foo2 and .bar2 to + // another. The problem is that there is no way to layout those output + // sections such that the .foo sections are the only thing between the start + // and end symbols. + // + // Given the above issues, we instead merge sections by name and error on + // incompatible types and flags. + // + // The exception being SHF_MERGE, where we create different output sections + // for each alignment. This makes each output section simple. In case of + // relocatable object generation we do not try to perform merging and treat + // SHF_MERGE sections as regular ones, but also create different output + // sections for them to allow merging at final linking stage. + // + // Fortunately, creating symbols in the middle of a merge section is not + // supported by bfd or gold, so the SHF_MERGE exception should not cause + // problems with most linker scripts. + typedef typename ELFT::uint uintX_t; - uintX_t Flags = getOutFlags(C); + uintX_t Flags = C->Flags & (SHF_MERGE | SHF_STRINGS); - // For SHF_MERGE we create different output sections for each alignment. - // This makes each output section simple and keeps a single level mapping from - // input to output. - // In case of relocatable object generation we do not try to perform merging - // and treat SHF_MERGE sections as regular ones, but also create different - // output sections for them to allow merging at final linking stage. uintX_t Alignment = 0; if (isa>(C) || (Config->Relocatable && (C->Flags & SHF_MERGE))) Alignment = std::max(C->Alignment, C->Entsize); - return SectionKey{OutsecName, C->Type, Flags, Alignment}; + return SectionKey{OutsecName, Flags, Alignment}; } template @@ -562,6 +609,10 @@ return create(Key, C); } +static uint64_t getIncompatibleFlags(uint64_t Flags) { + return Flags & (SHF_ALLOC | SHF_TLS); +} + template std::pair OutputSectionFactory::create(const SectionKey &Key, @@ -569,6 +620,12 @@ uintX_t Flags = getOutFlags(C); OutputSectionBase *&Sec = Map[Key]; if (Sec) { + if (getIncompatibleFlags(Sec->Flags) != getIncompatibleFlags(C->Flags)) + error("Section has flags incompatible with others with the same name " + + toString(C)); + if (Sec->Type != C->Type) + error("Section has different type from others with the same name " + + toString(C)); Sec->Flags |= Flags; return {Sec, false}; } @@ -591,28 +648,26 @@ template typename lld::elf::SectionKey DenseMapInfo>::getEmptyKey() { - return SectionKey{DenseMapInfo::getEmptyKey(), 0, 0, 0}; + return SectionKey{DenseMapInfo::getEmptyKey(), 0, 0}; } template typename lld::elf::SectionKey DenseMapInfo>::getTombstoneKey() { - return SectionKey{DenseMapInfo::getTombstoneKey(), 0, 0, - 0}; + return SectionKey{DenseMapInfo::getTombstoneKey(), 0, 0}; } template unsigned DenseMapInfo>::getHashValue(const Key &Val) { - return hash_combine(Val.Name, Val.Type, Val.Flags, Val.Alignment); + return hash_combine(Val.Name, Val.Flags, Val.Alignment); } template bool DenseMapInfo>::isEqual(const Key &LHS, const Key &RHS) { return DenseMapInfo::isEqual(LHS.Name, RHS.Name) && - LHS.Type == RHS.Type && LHS.Flags == RHS.Flags && - LHS.Alignment == RHS.Alignment; + LHS.Flags == RHS.Flags && LHS.Alignment == RHS.Alignment; } namespace llvm { Index: test/ELF/incompatible-section-flags.s =================================================================== --- /dev/null +++ test/ELF/incompatible-section-flags.s @@ -0,0 +1,18 @@ +// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o +// RUN: not ld.lld -shared %t.o -o %t 2>&1 | FileCheck %s + +// CHECK: error: Section has flags incompatible with others with the same name {{.*}}incompatible-section-flags.s.tmp.o:(.foo) +// CHECK: error: Section has flags incompatible with others with the same name {{.*}}incompatible-section-flags.s.tmp.o:(.bar) + +.section .foo, "awT", @progbits, unique, 1 +.quad 0 + +.section .foo, "aw", @progbits, unique, 2 +.quad 0 + + +.section .bar, "aw", @progbits, unique, 3 +.quad 0 + +.section .bar, "awT", @progbits, unique, 4 +.quad 0 Index: test/ELF/incompatible-section-types.s =================================================================== --- /dev/null +++ test/ELF/incompatible-section-types.s @@ -0,0 +1,10 @@ +// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o +// RUN: not ld.lld -shared %t.o -o %t 2>&1 | FileCheck %s + +// CHECK: error: Section has different type from others with the same name {{.*}}incompatible-section-types.s.tmp.o:(.foo) + +.section .foo, "aw", @progbits, unique, 1 +.quad 0 + +.section .foo, "aw", @nobits, unique, 2 +.quad 0 Index: test/ELF/linkerscript/phdrs.s =================================================================== --- test/ELF/linkerscript/phdrs.s +++ test/ELF/linkerscript/phdrs.s @@ -93,7 +93,7 @@ ## Check the numetic values for PHDRS. # RUN: echo "PHDRS {text PT_LOAD FILEHDR PHDRS; foo 0x11223344; } \ -# RUN: SECTIONS { . = SIZEOF_HEADERS; .foo : { *(.*) } : text : foo}" > %t1.script +# RUN: SECTIONS { . = SIZEOF_HEADERS; .foo : { *(.foo* .text*) } : text : foo}" > %t1.script # RUN: ld.lld -o %t2 --script %t1.script %t # RUN: llvm-readobj -program-headers %t2 | FileCheck --check-prefix=INT-PHDRS %s Index: test/ELF/linkerscript/sections.s =================================================================== --- test/ELF/linkerscript/sections.s +++ test/ELF/linkerscript/sections.s @@ -20,11 +20,10 @@ # SEC-DEFAULT: 2 .data 00000020 {{[0-9a-f]*}} DATA # SEC-DEFAULT: 3 other 00000003 {{[0-9a-f]*}} DATA # SEC-DEFAULT: 4 .bss 00000002 {{[0-9a-f]*}} BSS -# SEC-DEFAULT: 5 .shstrtab 00000002 {{[0-9a-f]*}} -# SEC-DEFAULT: 6 .comment 00000008 {{[0-9a-f]*}} -# SEC-DEFAULT: 7 .symtab 00000030 {{[0-9a-f]*}} -# SEC-DEFAULT: 8 .shstrtab 0000003b {{[0-9a-f]*}} -# SEC-DEFAULT: 9 .strtab 00000008 {{[0-9a-f]*}} +# SEC-DEFAULT: 5 .comment 00000008 {{[0-9a-f]*}} +# SEC-DEFAULT: 6 .symtab 00000030 {{[0-9a-f]*}} +# SEC-DEFAULT: 7 .shstrtab 0000003b {{[0-9a-f]*}} +# SEC-DEFAULT: 8 .strtab 00000008 {{[0-9a-f]*}} # Sections are put in order specified in linker script, other than alloc # sections going first. @@ -43,12 +42,11 @@ # Idx Name Size # SEC-ORDER: 1 .bss 00000002 {{[0-9a-f]*}} BSS # SEC-ORDER: 2 other 00000003 {{[0-9a-f]*}} DATA -# SEC-ORDER: 3 .shstrtab 00000002 {{[0-9a-f]*}} -# SEC-ORDER: 4 .shstrtab 0000003b {{[0-9a-f]*}} -# SEC-ORDER: 5 .symtab 00000030 {{[0-9a-f]*}} -# SEC-ORDER: 6 .strtab 00000008 {{[0-9a-f]*}} -# SEC-ORDER: 7 .data 00000020 {{[0-9a-f]*}} DATA -# SEC-ORDER: 8 .text 0000000e {{[0-9a-f]*}} TEXT DATA +# SEC-ORDER: 3 .shstrtab 0000003b {{[0-9a-f]*}} +# SEC-ORDER: 4 .symtab 00000030 {{[0-9a-f]*}} +# SEC-ORDER: 5 .strtab 00000008 {{[0-9a-f]*}} +# SEC-ORDER: 6 .data 00000020 {{[0-9a-f]*}} DATA +# SEC-ORDER: 7 .text 0000000e {{[0-9a-f]*}} TEXT DATA # .text and .data have swapped names but proper sizes and types. # RUN: echo "SECTIONS { \ @@ -63,11 +61,10 @@ # SEC-SWAP-NAMES: 2 .text 00000020 {{[0-9a-f]*}} DATA # SEC-SWAP-NAMES: 3 other 00000003 {{[0-9a-f]*}} DATA # SEC-SWAP-NAMES: 4 .bss 00000002 {{[0-9a-f]*}} BSS -# SEC-SWAP-NAMES: 5 .shstrtab 00000002 {{[0-9a-f]*}} -# SEC-SWAP-NAMES: 6 .comment 00000008 {{[0-9a-f]*}} -# SEC-SWAP-NAMES: 7 .symtab 00000030 {{[0-9a-f]*}} -# SEC-SWAP-NAMES: 8 .shstrtab 0000003b {{[0-9a-f]*}} -# SEC-SWAP-NAMES: 9 .strtab 00000008 {{[0-9a-f]*}} +# SEC-SWAP-NAMES: 5 .comment 00000008 {{[0-9a-f]*}} +# SEC-SWAP-NAMES: 6 .symtab 00000030 {{[0-9a-f]*}} +# SEC-SWAP-NAMES: 7 .shstrtab 0000003b {{[0-9a-f]*}} +# SEC-SWAP-NAMES: 8 .strtab 00000008 {{[0-9a-f]*}} # .shstrtab from the input object file is discarded. # RUN: echo "SECTIONS { \ @@ -102,11 +99,10 @@ # SEC-MULTI: 1 .text 0000000e {{[0-9a-f]*}} TEXT DATA # SEC-MULTI: 2 .data 00000023 {{[0-9a-f]*}} DATA # SEC-MULTI: 3 .bss 00000002 {{[0-9a-f]*}} BSS -# SEC-MULTI: 4 .shstrtab 00000002 {{[0-9a-f]*}} -# SEC-MULTI: 5 .comment 00000008 {{[0-9a-f]*}} -# SEC-MULTI: 6 .symtab 00000030 {{[0-9a-f]*}} -# SEC-MULTI: 7 .shstrtab 00000035 {{[0-9a-f]*}} -# SEC-MULTI: 8 .strtab 00000008 {{[0-9a-f]*}} +# SEC-MULTI: 4 .comment 00000008 {{[0-9a-f]*}} +# SEC-MULTI: 5 .symtab 00000030 {{[0-9a-f]*}} +# SEC-MULTI: 6 .shstrtab 00000035 {{[0-9a-f]*}} +# SEC-MULTI: 7 .strtab 00000008 {{[0-9a-f]*}} .globl _start _start: @@ -118,7 +114,5 @@ .section other,"aw" .short 10 .byte 20 -.section .shstrtab,"" -.short 20 .section .bss,"",@nobits .short 0 Index: test/ELF/section-name.s =================================================================== --- test/ELF/section-name.s +++ test/ELF/section-name.s @@ -19,9 +19,9 @@ .byte 0 .section .data,"aw" .byte 0 -.section .bss.a,"",@nobits +.section .bss.a,"aw",@nobits .byte 0 -.section .bss,"",@nobits +.section .bss,"aw",@nobits .byte 0 .section .foo.a,"aw" .byte 0 @@ -51,9 +51,8 @@ // CHECK: 7 .data 00000002 // CHECK: 8 .foo.a 00000001 // CHECK: 9 .foo 00000001 -// CHECK: 10 .bss 00000001 -// CHECK: 11 .bss 00000001 -// CHECK: 12 .comment 00000008 -// CHECK: 13 .symtab 00000060 -// CHECK: 14 .shstrtab 00000075 -// CHECK: 15 .strtab 0000001d +// CHECK: 10 .bss 00000002 +// CHECK: 11 .comment 00000008 +// CHECK: 12 .symtab 00000060 +// CHECK: 13 .shstrtab 00000075 +// CHECK: 14 .strtab 0000001d Index: test/ELF/string-table.s =================================================================== --- test/ELF/string-table.s +++ test/ELF/string-table.s @@ -6,10 +6,7 @@ .global _start _start: -.section foobar,"",@progbits,unique,1 -.section foobar,"T",@progbits,unique,2 -.section foobar,"",@nobits,unique,3 -.section foobar,"",@nobits,unique,4 +.section foobar,"",@progbits .section bar, "a" @@ -27,17 +24,4 @@ // CHECK-NEXT: ] // CHECK-NEXT: Address: 0x0 -// CHECK: Name: foobar -// CHECK-NEXT: Type: SHT_PROGBITS -// CHECK-NEXT: Flags [ -// CHECK-NEXT: SHF_TLS -// CHECK-NEXT: ] -// CHECK-NEXT: Address: 0x0 - -// CHECK: Name: foobar -// CHECK-NEXT: Type: SHT_NOBITS -// CHECK-NEXT: Flags [ -// CHECK-NEXT: ] -// CHECK-NEXT: Address: 0x0 - // CHECK-NOT: Name: foobar