[ELF] - Linkerscript: fix VA value assigned to sections when using constraints.

[grimar]

Summary

Previously we searched output section by name to assign VA. That did not
work in the case when multiple output sections with different constraints were defined in script.
Testcase shows the possible issue scenario, patch fixes the issue.

[ruiu]

ELF/LinkerScript.cpp

333 ↗

(On Diff #67841)

This function has duplicate code with matchConstraints. Can you reorganize this and that to reduce the amount of code?

[grimar]

• Addressed review comment.

ELF/LinkerScript.cpp

333 ↗

(On Diff #67841)

Done.

[grimar]

• Simplified.

[ruiu]

LGTM

ELF/LinkerScript.cpp

218–219 ↗

(On Diff #67901)

Instead of template this function, use uint64_t instead of ELF::uint.

lld/trunk/ELF/LinkerScript.cpp

if (auto *Cmd = dyn_cast<InputSectionDescription>(Base.get())) {
       for (InputSectionBase<ELFT> *S : getInputSections(Cmd)) {
         S->Live = false;
         reportDiscarded(S);
       }
     }
   }
 }
 
+static bool checkConstraint(uint64_t Flags, ConstraintKind Kind) {
+  bool RO = (Kind == ConstraintKind::ReadOnly);
+  bool RW = (Kind == ConstraintKind::ReadWrite);
+  bool Writable = Flags & SHF_WRITE;
+  return !((RO && Writable) || (RW && !Writable));
+}
+
 template <class ELFT>
 static bool matchConstraints(ArrayRef<InputSectionBase<ELFT> *> Sections,
                              ConstraintKind Kind) {
-  bool RO = (Kind == ConstraintKind::ReadOnly);
-  bool RW = (Kind == ConstraintKind::ReadWrite);
-  return !llvm::any_of(Sections, [=](InputSectionBase<ELFT> *Sec) {
-    bool Writable = Sec->getSectionHdr()->sh_flags & SHF_WRITE;
-    return (RO && Writable) || (RW && !Writable);
+  if (Kind == ConstraintKind::NoConstraint)
+    return true;
+  return llvm::all_of(Sections, [=](InputSectionBase<ELFT> *Sec) {
+    return checkConstraint(Sec->getSectionHdr()->sh_flags, Kind);
   });
 }
 
 template <class ELFT>
 std::vector<InputSectionBase<ELFT> *>
 LinkerScript<ELFT>::createInputSectionList(OutputSectionCommand &OutCmd) {
   std::vector<InputSectionBase<ELFT> *> Ret;
 
for (InputSection<ELFT> *I : OutSec->Sections) {
     }
     // Update section size inside for-loop, so that SIZEOF
     // works correctly in the case below:
     // .foo { *(.aaa) a = SIZEOF(.foo); *(.bbb) }
     Sec->setSize(Off);
   }
 }
 
+template <class ELFT>
+static OutputSectionBase<ELFT> *
+findSection(OutputSectionCommand &Cmd,
+            ArrayRef<OutputSectionBase<ELFT> *> Sections) {
+  for (OutputSectionBase<ELFT> *Sec : Sections) {
+    if (Sec->getName() != Cmd.Name)
+      continue;
+    if (checkConstraint(Sec->getFlags(), Cmd.Constraint))
+      return Sec;
+  }
+  return nullptr;
+}
+
 template <class ELFT> void LinkerScript<ELFT>::assignAddresses() {
   // Orphan sections are sections present in the input files which
   // are not explicitly placed into the output file by the linker script.
   // We place orphan sections at end of file.
   // Other linkers places them using some heuristics as described in
   // https://sourceware.org/binutils/docs/ld/Orphan-Sections.html#Orphan-Sections.
   for (OutputSectionBase<ELFT> *Sec : *OutputSections) {
     StringRef Name = Sec->getName();
for (const std::unique_ptr<BaseCommand> &Base : Opt.Commands) {
     }
 
     if (auto *Cmd = dyn_cast<AssertCommand>(Base.get())) {
       Cmd->Expression(Dot);
       continue;
     }
 
     auto *Cmd = cast<OutputSectionCommand>(Base.get());
-    auto I = llvm::find_if(*OutputSections, [&](OutputSectionBase<ELFT> *S) {
-      return S->getName() == Cmd->Name;
-    });
-    if (I == OutputSections->end())
+    OutputSectionBase<ELFT> *Sec = findSection<ELFT>(*Cmd, *OutputSections);
+    if (!Sec)
       continue;
-    OutputSectionBase<ELFT> *Sec = *I;
 
     if (Cmd->AddrExpr)
       Dot = Cmd->AddrExpr(Dot);
 
     if (Cmd->AlignExpr)
       Sec->updateAlignment(Cmd->AlignExpr(Dot));
 
     if ((Sec->getFlags() & SHF_TLS) && Sec->getType() == SHT_NOBITS) {

lld/trunk/test/ELF/linkerscript/linkerscript-multi-sections-constraint.s

 # CHECK-NEXT: Idx Name          Size      Address          Type
 # CHECK-NEXT:   0               00000000 0000000000000000
 # CHECK-NEXT:   1 .aaa          00000010 0000000000002000 DATA
 # CHECK-NEXT:   2 .text         00000001 0000000000002010 TEXT DATA
 # CHECK-NEXT:   3 .symtab       00000030 0000000000000000
 # CHECK-NEXT:   4 .shstrtab     00000026 0000000000000000
 # CHECK-NEXT:   5 .strtab       00000008 0000000000000000
 
+# RUN: echo "SECTIONS { \
+# RUN:  . = 0x1000; .aaa : ONLY_IF_RW { *(.aaa.*) } \
+# RUN:  . = 0x2000; .aaa : ONLY_IF_RO { *(.aaa.*) } } " > %t2.script
+# RUN: ld.lld -o %t2 --script %t2.script %t
+# RUN: llvm-objdump -section-headers %t2 | FileCheck %s --check-prefix=REV
+
+# REV:      Sections:
+# REV-NEXT: Idx Name          Size      Address          Type
+# REV-NEXT:   0               00000000 0000000000000000
+# REV-NEXT:   1 .aaa          00000010 0000000000001000 DATA
+# REV-NEXT:   2 .text         00000001 0000000000002000 TEXT DATA
+# REV-NEXT:   3 .symtab       00000030 0000000000000000
+# REV-NEXT:   4 .shstrtab     00000026 0000000000000000
+# REV-NEXT:   5 .strtab       00000008 0000000000000000
+
 .global _start
 _start:
  nop
 
 .section .aaa.1, "aw"
 .quad 1
 
 .section .aaa.2, "aw"
 .quad 1