[ELF] - Linkerscript: merge section attributes when create output section.

[grimar]

Summary

If there are input sections with the same name but with different attributes
and linker script layout is used then we create single output section.

What is wrong is that we create an output section based on the first input
section's attributes. This may make a wrong output section attributes,
patch fixes the issue.

[grimar]

Ping

[ruiu]

ELF/LinkerScript.cpp

11

V is a std::vector<InputSectionBase<ELFT> *>, but

11

S is a InputSectionBase<ELFT> *. How does it work?

[grimar]

Ping

ELF/LinkerScript.cpp

11

Do you mean call of updateFlags ? ArrayRef<InputSectionBase<ELFT> *> argument can be constructed from std::vector and as well as from single pointer:

/// Construct an ArrayRef from a single element.
/*implicit*/ ArrayRef(const T &OneElt)
  : Data(&OneElt), Length(1) {}

[ruiu]

ELF/LinkerScript.cpp

11

Looks like it's too subtle to me.

[grimar]

• Addressed review comments.

ELF/LinkerScript.cpp

11

Reimplemented.

[ruiu]

ELF/OutputSections.cpp

1830

This looks pretty tricky. Do you think you can make a change to not use OutputSectionFactory in LinkerScript first?

[grimar]

ELF/OutputSections.cpp

1830

Do you mean LinkerScript should have its own factory like in D23315 ?

[grimar]

ELF/OutputSections.cpp

1830

Change to avoid using OutputSectionsFactory is D23866

ELF/LinkerScript.cpp

 //===- LinkerScript.cpp ---------------------------------------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file contains the parser/evaluator of the linker script.
 // It parses a linker script and write the result to Config or ScriptConfig
 // objects.
 //
 // If SECTIONS command is used, a ScriptConfig contains an AST
 // of the command which will later be consumed by createSections() and
 // assignAddresses().
 //
 //===----------------------------------------------------------------------===//
 
for (const std::unique_ptr<BaseCommand> &Base : OutCmd.Commands) {
     if (Cmd->SortOuter)
       std::stable_sort(V.begin(), V.end(), getComparator<ELFT>(Cmd->SortOuter));
     Ret.insert(Ret.end(), V.begin(), V.end());
   }
   return Ret;
 }
 
 template <class ELFT>
+static void updateFlags(OutputSectionBase<ELFT> *OutSec,
+                       InputSectionBase<ELFT> *InSec) {
+  typename ELFT::uint Flags = OutSec->getFlags();
+  Flags |= InSec->getSectionHdr()->sh_flags & ~SHF_GROUP & ~SHF_COMPRESSED;
+  OutSec->setFlags(Flags);
+}
+
+template <class ELFT>
 void LinkerScript<ELFT>::createSections(OutputSectionFactory<ELFT> &Factory) {
   for (const std::unique_ptr<BaseCommand> &Base1 : Opt.Commands) {
     if (auto *Cmd = dyn_cast<SymbolAssignment>(Base1.get())) {
       if (shouldDefine<ELFT>(Cmd))
         addRegular<ELFT>(Cmd);
       continue;
     }
 
if (auto *Cmd = dyn_cast<OutputSectionCommand>(Base1.get())) {
         OutputSections->push_back(OutSec);
 
       uint32_t Subalign = Cmd->SubalignExpr ? Cmd->SubalignExpr(0) : 0;
       for (InputSectionBase<ELFT> *Sec : V) {
         if (Subalign)
           Sec->Alignment = Subalign;
         if (!Sec->OutSec)
           OutSec->addSection(Sec);
+        updateFlags<ELFT>(OutSec, Sec);
       }
     }
   }
 
   // Add orphan sections.
   for (const std::unique_ptr<ObjectFile<ELFT>> &F :
        Symtab<ELFT>::X->getObjectFiles()) {
     for (InputSectionBase<ELFT> *S : F->getSections()) {
       if (isDiscarded(S) || S->OutSec)
         continue;
       OutputSectionBase<ELFT> *OutSec;
       bool IsNew;
       std::tie(OutSec, IsNew) = Factory.create(S, getOutputSectionName(S));
       if (IsNew)
         OutputSections->push_back(OutSec);
       OutSec->addSection(S);
+      updateFlags<ELFT>(OutSec, S);
     }
   }
 }
 
 template <class ELFT> void assignOffsets(OutputSectionBase<ELFT> *Sec) {
   auto *OutSec = dyn_cast<OutputSection<ELFT>>(Sec);
   if (!OutSec) {
     Sec->assignOffsets();

ELF/OutputSections.h

public:
   }
 
   unsigned SectionIndex;
 
   // Returns the size of the section in the output file.
   uintX_t getSize() const { return Header.sh_size; }
   void setSize(uintX_t Val) { Header.sh_size = Val; }
   uintX_t getFlags() const { return Header.sh_flags; }
+  void setFlags(uintX_t Val) { Header.sh_flags = Val; }
   uint32_t getPhdrFlags() const;
   uintX_t getFileOff() const { return Header.sh_offset; }
   uintX_t getAlignment() const { return Header.sh_addralign; }
   uint32_t getType() const { return Header.sh_type; }
 
   void updateAlignment(uintX_t Alignment) {
     if (Alignment > Header.sh_addralign)
       Header.sh_addralign = Alignment;

ELF/OutputSections.cpp

OutputSectionFactory<ELFT>::create(InputSectionBase<ELFT> *C,
   return {Sec, true};
 }
 
 template <class ELFT>
 SectionKey<ELFT::Is64Bits>
 OutputSectionFactory<ELFT>::createKey(InputSectionBase<ELFT> *C,
                                       StringRef OutsecName) {
   const Elf_Shdr *H = C->getSectionHdr();
-  uintX_t Flags = H->sh_flags & ~SHF_GROUP & ~SHF_COMPRESSED;
+
+  // Ignore flags part of the key if linkerscript has SECTIONS command.
+  // We combine input sections with different attributes into single output
+  // section in this case. Flags are updated later.
+  uintX_t Flags = ScriptConfig->HasContents ? 0 : H->sh_flags & ~SHF_GROUP &
+                                                      ~SHF_COMPRESSED;
 
   // 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.
   uintX_t Alignment = 0;
   if (isa<MergeInputSection<ELFT>>(C))
     Alignment = std::max(H->sh_addralign, H->sh_entsize);
 

test/ELF/linkerscript/linkerscript-phdrs.s

 # CHECK:     ProgramHeaders [
 # CHECK-NEXT:  ProgramHeader {
 # CHECK-NEXT:    Type: PT_LOAD (0x1)
 # CHECK-NEXT:    Offset: 0x0
 # CHECK-NEXT:    VirtualAddress: 0x10000000
 # CHECK-NEXT:    PhysicalAddress: 0x10000000
 # CHECK-NEXT:    FileSize: 521
 # CHECK-NEXT:    MemSize: 521
-# CHECK-NEXT:    Flags [ (0x5)
+# CHECK-NEXT:    Flags [ (0x7)
 # CHECK-NEXT:      PF_R (0x4)
+# CHECK-NEXT:      PF_W (0x2)
 # CHECK-NEXT:      PF_X (0x1)
 # CHECK-NEXT:    ]
 
 .global _start
 _start:
  nop
 
 .section .foo.1,"a"
 foo1:
  .long 0
 
 .section .foo.2,"aw"
 foo2:
  .long 0

test/ELF/linkerscript/linkerscript-repsection-va.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
 
 # RUN: echo "SECTIONS {.foo : {*(.foo.*)} }" > %t.script
 # RUN: ld.lld -o %t1 --script %t.script %t
 # RUN: llvm-objdump -section-headers %t1 | FileCheck %s
 # CHECK:      Sections:
 # CHECK-NEXT: Idx Name          Size      Address          Type
 # CHECK-NEXT:   0               00000000 0000000000000000
-# CHECK-NEXT:   1 .foo          00000008 0000000000000120 DATA
-# CHECK-NEXT:   2 .text         00000001 0000000000000128 TEXT DATA
+# CHECK-NEXT:   1 .foo          0000000c 0000000000000158 TEXT DATA
+# CHECK-NEXT:   2 .text         00000001 0000000000000164 TEXT DATA
+
+# RUN: llvm-readobj -sections %t1 | FileCheck %s --check-prefix=ATTR
+# ATTR:      Section {
+# ATTR:        Index:
+# ATTR:        Name: .foo
+# ATTR-NEXT:   Type:
+# ATTR-NEXT:   Flags [
+# ATTR-NEXT:     SHF_ALLOC
+# ATTR-NEXT:     SHF_EXECINSTR
+# ATTR-NEXT:     SHF_WRITE
+# ATTR-NEXT:   ]
 
 .global _start
 _start:
  nop
 
 .section .foo.1,"a"
 foo1:
  .long 0
 
 .section .foo.2,"aw"
 foo2:
  .long 0
+
+.section .foo.3,"ax"
+foo3:
+ .long 0