Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LLD][ELF] Support 'i' and 'l' memory region attributes #110937

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mysterymath
Copy link
Contributor

This patch adds support for 'i' and 'l' memory region attributes for initialized sections. This decreases friction when porting linker scripts from GNU LD.

This patch adds support for 'i' and 'l' memory region attributes for
initialized sections. This decreases friction when porting linker
scripts from GNU LD.
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 2, 2024

@llvm/pr-subscribers-lld-elf

@llvm/pr-subscribers-lld

Author: Daniel Thornburgh (mysterymath)

Changes

This patch adds support for 'i' and 'l' memory region attributes for initialized sections. This decreases friction when porting linker scripts from GNU LD.


Full diff: https://github.com/llvm/llvm-project/pull/110937.diff

4 Files Affected:

  • (modified) lld/ELF/LinkerScript.cpp (+10-1)
  • (modified) lld/ELF/LinkerScript.h (+24-21)
  • (modified) lld/ELF/ScriptParser.cpp (+19-24)
  • (modified) lld/test/ELF/linkerscript/memory-attr.test (+17-9)
diff --git a/lld/ELF/LinkerScript.cpp b/lld/ELF/LinkerScript.cpp
index cce584ae4d8679..ee58922d4cd77d 100644
--- a/lld/ELF/LinkerScript.cpp
+++ b/lld/ELF/LinkerScript.cpp
@@ -404,6 +404,15 @@ bool InputSectionDescription::matchesFile(const InputFile *file) const {
   return matchesFileCache->second;
 }
 
+bool MemoryRegion::Attrs::matches(OutputSection *sec) const {
+  return (sec->flags & flags) || (~sec->flags & invFlags) ||
+         (initialized && sec->type != SHT_NOBITS);
+}
+
+bool MemoryRegion::compatibleWith(OutputSection *sec) const {
+  return !negAttrs.matches(sec) && attrs.matches(sec);
+}
+
 bool SectionPattern::excludesFile(const InputFile *file) const {
   if (excludedFilePat.empty())
     return false;
@@ -1105,7 +1114,7 @@ LinkerScript::findMemoryRegion(OutputSection *sec, MemoryRegion *hint) {
   // See if a region can be found by matching section flags.
   for (auto &pair : memoryRegions) {
     MemoryRegion *m = pair.second;
-    if (m->compatibleWith(sec->flags))
+    if (m->compatibleWith(sec))
       return {m, nullptr};
   }
 
diff --git a/lld/ELF/LinkerScript.h b/lld/ELF/LinkerScript.h
index db6a8c8e147ac6..e460d4615c1833 100644
--- a/lld/ELF/LinkerScript.h
+++ b/lld/ELF/LinkerScript.h
@@ -139,36 +139,39 @@ enum class ConstraintKind { NoConstraint, ReadOnly, ReadWrite };
 // target memory. Instances of the struct are created by parsing the
 // MEMORY command.
 struct MemoryRegion {
-  MemoryRegion(StringRef name, Expr origin, Expr length, uint32_t flags,
-               uint32_t invFlags, uint32_t negFlags, uint32_t negInvFlags)
-      : name(std::string(name)), origin(origin), length(length), flags(flags),
-        invFlags(invFlags), negFlags(negFlags), negInvFlags(negInvFlags) {}
+  // A collection of attributes that match a set of sections.
+  struct Attrs {
+    // Match if any of these ELF section flags are present.
+    uint32_t flags = 0;
+    // Match if any of these ELF section flags are absent.
+    // For example, the memory region attribute "r" maps to SHF_WRITE.
+    uint32_t invFlags = 0;
+    // Match if the section type isn't SHT_NOBITS.
+    bool initialized = false;
+
+    bool matches(OutputSection *sec) const;
+  };
+
+  MemoryRegion(StringRef name, Expr origin, Expr length, Attrs attrs,
+               Attrs negAttrs)
+      : name(std::string(name)), origin(origin), length(length), attrs(attrs),
+        negAttrs(negAttrs) {}
 
   std::string name;
   Expr origin;
   Expr length;
-  // A section can be assigned to the region if any of these ELF section flags
-  // are set...
-  uint32_t flags;
-  // ... or any of these flags are not set.
-  // For example, the memory region attribute "r" maps to SHF_WRITE.
-  uint32_t invFlags;
-  // A section cannot be assigned to the region if any of these ELF section
-  // flags are set...
-  uint32_t negFlags;
-  // ... or any of these flags are not set.
-  // For example, the memory region attribute "!r" maps to SHF_WRITE.
-  uint32_t negInvFlags;
+
+  // Attributes that cause a section to match the region.
+  Attrs attrs;
+  // Attributes that cause a section not to match the region.
+  Attrs negAttrs;
+
   uint64_t curPos = 0;
 
   uint64_t getOrigin() const { return origin().getValue(); }
   uint64_t getLength() const { return length().getValue(); }
 
-  bool compatibleWith(uint32_t secFlags) const {
-    if ((secFlags & negFlags) || (~secFlags & negInvFlags))
-      return false;
-    return (secFlags & flags) || (~secFlags & invFlags);
-  }
+  bool compatibleWith(OutputSection *sec) const;
 };
 
 // This struct represents one section match pattern in SECTIONS() command.
diff --git a/lld/ELF/ScriptParser.cpp b/lld/ELF/ScriptParser.cpp
index b16b2e56473adc..c189558fd165ae 100644
--- a/lld/ELF/ScriptParser.cpp
+++ b/lld/ELF/ScriptParser.cpp
@@ -107,8 +107,8 @@ class ScriptParser final : ScriptLexer {
   Expr getPageSize();
 
   Expr readMemoryAssignment(StringRef, StringRef, StringRef);
-  void readMemoryAttributes(uint32_t &flags, uint32_t &invFlags,
-                            uint32_t &negFlags, uint32_t &negInvFlags);
+  void readMemoryAttributes(MemoryRegion::Attrs &attrs,
+                            MemoryRegion::Attrs &negAttrs);
 
   Expr combine(StringRef op, Expr l, Expr r);
   Expr readExpr();
@@ -1839,12 +1839,10 @@ void ScriptParser::readMemory() {
       continue;
     }
 
-    uint32_t flags = 0;
-    uint32_t invFlags = 0;
-    uint32_t negFlags = 0;
-    uint32_t negInvFlags = 0;
+    MemoryRegion::Attrs attrs;
+    MemoryRegion::Attrs negAttrs;
     if (consume("(")) {
-      readMemoryAttributes(flags, invFlags, negFlags, negInvFlags);
+      readMemoryAttributes(attrs, negAttrs);
       expect(")");
     }
     expect(":");
@@ -1854,44 +1852,41 @@ void ScriptParser::readMemory() {
     Expr length = readMemoryAssignment("LENGTH", "len", "l");
 
     // Add the memory region to the region map.
-    MemoryRegion *mr = make<MemoryRegion>(tok, origin, length, flags, invFlags,
-                                          negFlags, negInvFlags);
+    MemoryRegion *mr = make<MemoryRegion>(tok, origin, length, attrs, negAttrs);
     if (!ctx.script->memoryRegions.insert({tok, mr}).second)
       setError("region '" + tok + "' already defined");
   }
 }
 
-// This function parses the attributes used to match against section
-// flags when placing output sections in a memory region. These flags
+// This function parses the attributes used to match against sections
+// when placing output sections in a memory region. These attributes
 // are only used when an explicit memory region name is not used.
-void ScriptParser::readMemoryAttributes(uint32_t &flags, uint32_t &invFlags,
-                                        uint32_t &negFlags,
-                                        uint32_t &negInvFlags) {
+void ScriptParser::readMemoryAttributes(MemoryRegion::Attrs &attrs,
+                                        MemoryRegion::Attrs &negAttrs) {
   bool invert = false;
 
   for (char c : next().lower()) {
     if (c == '!') {
       invert = !invert;
-      std::swap(flags, negFlags);
-      std::swap(invFlags, negInvFlags);
+      std::swap(attrs, negAttrs);
       continue;
     }
     if (c == 'w')
-      flags |= SHF_WRITE;
+      attrs.flags |= SHF_WRITE;
     else if (c == 'x')
-      flags |= SHF_EXECINSTR;
+      attrs.flags |= SHF_EXECINSTR;
     else if (c == 'a')
-      flags |= SHF_ALLOC;
+      attrs.flags |= SHF_ALLOC;
     else if (c == 'r')
-      invFlags |= SHF_WRITE;
+      attrs.invFlags |= SHF_WRITE;
+    else if (c == 'i' || c == 'l')
+      attrs.initialized = true;
     else
       setError("invalid memory region attribute");
   }
 
-  if (invert) {
-    std::swap(flags, negFlags);
-    std::swap(invFlags, negInvFlags);
-  }
+  if (invert)
+    std::swap(attrs, negAttrs);
 }
 
 void elf::readLinkerScript(Ctx &ctx, MemoryBufferRef mb) {
diff --git a/lld/test/ELF/linkerscript/memory-attr.test b/lld/test/ELF/linkerscript/memory-attr.test
index 5ea6e77ee2540b..89c92c48a39395 100644
--- a/lld/test/ELF/linkerscript/memory-attr.test
+++ b/lld/test/ELF/linkerscript/memory-attr.test
@@ -15,6 +15,9 @@ RUN: llvm-mc -filetype=obj -triple=x86_64 %ts/s -o %t.o
   .data
   .zero 8
 
+  .bss
+  .zero 8
+
 #--- t1
 ## Check memory region attribute 'a'.
 
@@ -25,6 +28,7 @@ RUN: llvm-mc -filetype=obj -triple=x86_64 %ts/s -o %t.o
 # TEST1:      .text    PROGBITS  0000000000002000
 # TEST1-NEXT: .rodata  PROGBITS  0000000000002008
 # TEST1-NEXT: .data    PROGBITS  0000000000002010
+# TEST1-NEXT: .bss     NOBITS    0000000000002018
 
 MEMORY
 {
@@ -42,8 +46,8 @@ SECTIONS
 }
 
 #--- t2
-## Check that memory region attributes 'r', 'w', and 'x' are supported both in
-## positive and negative forms.
+## Check that memory region attributes 'r', 'w', 'x', 'i', and 'l' are supported
+## both in positive and negative forms.
 
 # RUN: ld.lld -o %t2 -T %ts/t2 %t.o
 # RUN: llvm-readelf -S %t2 | FileCheck %s --check-prefix=TEST2
@@ -51,20 +55,24 @@ SECTIONS
 # TEST2:      Name     Type      Address
 # TEST2:      .text    PROGBITS  0000000000004000
 # TEST2-NEXT: .rodata  PROGBITS  0000000000003000
-# TEST2-NEXT: .data    PROGBITS  0000000000002000
+# TEST2-NEXT: .data    PROGBITS  0000000000005000
+# TEST2-NEXT: .bss     NOBITS    0000000000002000
 
 MEMORY
 {
   ## While all sections are allocatable, '.text' and '.rodata' are read-only and
-  ## '.data' is writable, so no sections should be assigned to this region.
-  NOMEM (a!rw) : org = 0x1000, l = 1K
-  ## Only writable section '.data' is allowed here.
-  RAM   (w)    : org = 0x2000, l = 1K
+  ## '.data' and '.bss' are writable, so no sections should be assigned to this
+  ## region.
+  NOMEM  (a!rw) : org = 0x1000, l = 1K
+  ## Only writable section '.bss' is allowed here, but not '.data' (initialized).
+  BSS    (w!i)  : org = 0x2000, l = 1K
   ## Executable sections are not allowed here, so only '.rodata' should be
   ## assigned to the region.
-  ROM   (r!x)  : org = 0x3000, l = 1K
+  ROM    (r!x)  : org = 0x3000, l = 1K
   ## An executable section '.text' comes here.
-  EXEC  (x)    : org = 0x4000, l = 1K
+  EXEC   (x)    : org = 0x4000, l = 1K
+  ## Only '.data' is allowed here (initialized).
+  DATA   (l)    : org = 0x5000, l = 1K
 }
 
 SECTIONS

@MaskRay
Copy link
Member

MaskRay commented Oct 3, 2024

I know one RISC-V firmware that uses i and it is semi-erroneous. Perhaps we were looking at the same instance.

i in BFD corresponds to SEC_LOAD but IMO the flag would only lead to confusion and erroneous behaviors. If .bss and the preceding section are not in the same memory region, very likely the BSS size optimization will not be effective. Therefore, I don't think we should add i.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants