lld: add support for NOCROSSREFS(_TO) #95714

pskrgag · 2024-06-16T17:53:46Z

Patch introduces supprot for NOCROSSREFS_(TO) linker script commands. These commands specify which cross-section references should be threated as errors. See more in ld documenmtation [0]

Implementation is straightforward -- traverse all relocations in all object files and report an error if there is prohibited one.

[0] https://sourceware.org/binutils/docs/ld/Miscellaneous-Commands.html

Closes: #41825

github-actions · 2024-06-16T17:54:02Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2024-06-16T17:54:32Z

@llvm/pr-subscribers-lld-elf

Author: Pavel Skripkin (pskrgag)

Changes

Patch introduces supprot for NOCROSSREFS_(TO) linker script commands. These commands specify which cross-section references should be threated as errors. See more in ld documenmtation [0]

Implementation is straightforward -- traverse all relocations in all object files and report an error if there is prohibited one.

[0] https://sourceware.org/binutils/docs/ld/Miscellaneous-Commands.html

Closes: #41825

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

6 Files Affected:

(modified) lld/ELF/LinkerScript.h (+8)
(modified) lld/ELF/Relocations.cpp (+59)
(modified) lld/ELF/Relocations.h (+1)
(modified) lld/ELF/ScriptParser.cpp (+20)
(modified) lld/ELF/Writer.cpp (+3)
(added) lld/test/ELF/gnu-nocrossrefs.s (+110)

diff --git a/lld/ELF/LinkerScript.h b/lld/ELF/LinkerScript.h
index 734d4e7498aa2..5821b0fc587aa 100644
--- a/lld/ELF/LinkerScript.h
+++ b/lld/ELF/LinkerScript.h
@@ -256,6 +256,11 @@ struct InsertCommand {
   StringRef where;
 };
 
+struct CrossRefList {
+  SmallVector<StringRef, 2> refs;
+  bool firstOnly;
+};
+
 struct PhdrsCommand {
   StringRef name;
   unsigned type = llvm::ELF::PT_NULL;
@@ -394,6 +399,9 @@ class LinkerScript final {
   // OutputSections specified by OVERWRITE_SECTIONS.
   SmallVector<OutputDesc *, 0> overwriteSections;
 
+  // Nocrossrefs sections.
+  SmallVector<CrossRefList, 0> nocrossrefs;
+
   // Sections that will be warned/errored by --orphan-handling.
   SmallVector<const InputSectionBase *, 0> orphanSections;
 
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index 2c02c2e572bfd..400013adf53dd 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -2358,3 +2358,62 @@ template void elf::scanRelocations<ELF32LE>();
 template void elf::scanRelocations<ELF32BE>();
 template void elf::scanRelocations<ELF64LE>();
 template void elf::scanRelocations<ELF64BE>();
+
+static bool isNoCrefFromSection(const CrossRefList &list,
+                                const OutputSection *section) {
+  const auto *begin =
+      list.firstOnly ? list.refs.begin() + 1 : list.refs.begin();
+
+  return std::find(begin, list.refs.end(), section->name) != list.refs.end();
+}
+
+static bool isNoCrefToSection(const CrossRefList &list,
+                              const OutputSection *section) {
+  if (list.firstOnly)
+    return list.refs[0] == section->name;
+
+  return std::find(list.refs.begin(), list.refs.end(), section->name) !=
+         list.refs.end();
+}
+
+void elf::checkNoCrossRefs() {
+  // Basic brute-force algorithm, since in reality NOCROSSRES lists are quite
+  // small.
+  //
+  // Idea is to traverse all relocations in all sections and report if
+  // prohibited reference was found. Note that NOCROSSREFS works with output section
+  // names.
+  for (ELFFileBase *file : ctx.objectFiles) {
+    auto sections = file->getSections();
+    std::string message = "";
+
+    for (size_t i = 0; i < sections.size(); ++i) {
+      if (sections[i]) {
+        StringRef sectionName = sections[i]->name;
+
+        if (const auto *outSec = dyn_cast_or_null<OutputSection>(sections[i]->parent)) {
+          for (const auto &refs : script->nocrossrefs) {
+            if (isNoCrefFromSection(refs, outSec)) {
+
+              for (const auto &j : sections[i]->relocations) {
+                if (auto *def = dyn_cast<Defined>(j.sym)) {
+                  const auto *outSecSym =
+                      cast<InputSection>(def->section)->getParent();
+
+                  if (isNoCrefToSection(refs, outSecSym)) {
+                    message +=
+                        (sections[i]->getLocation(j.offset) +
+                         ": prohibited cross reference from " +
+                         sectionName.str() + " to " + def->getName().str() +
+                         " in " + outSecSym->name.str());
+                    error(message);
+                  }
+                }
+              }
+            }
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/lld/ELF/Relocations.h b/lld/ELF/Relocations.h
index b7b9c09e1b892..eabae26283c24 100644
--- a/lld/ELF/Relocations.h
+++ b/lld/ELF/Relocations.h
@@ -141,6 +141,7 @@ struct JumpInstrMod {
 // the diagnostics.
 template <class ELFT> void scanRelocations();
 void reportUndefinedSymbols();
+void checkNoCrossRefs();
 void postScanRelocations();
 void addGotEntry(Symbol &sym);
 
diff --git a/lld/ELF/ScriptParser.cpp b/lld/ELF/ScriptParser.cpp
index f90ce6fa74075..0c14f43fc5f05 100644
--- a/lld/ELF/ScriptParser.cpp
+++ b/lld/ELF/ScriptParser.cpp
@@ -87,6 +87,7 @@ class ScriptParser final : ScriptLexer {
   void readTarget();
   void readVersion();
   void readVersionScriptCommand();
+  void readNoCrossRefs(bool to = false);
 
   SymbolAssignment *readSymbolAssignment(StringRef name);
   ByteCommand *readByteCommand(StringRef tok);
@@ -235,6 +236,21 @@ void ScriptParser::readVersionScriptCommand() {
   }
 }
 
+void ScriptParser::readNoCrossRefs(bool to) {
+  expect("(");
+
+  script->nocrossrefs.push_back({});
+
+  script->nocrossrefs.back().firstOnly = to;
+
+  while (!atEOF() && !errorCount() && peek() != ")") {
+    StringRef section = next();
+    script->nocrossrefs.back().refs.push_back(section);
+  }
+
+  expect(")");
+}
+
 void ScriptParser::readVersion() {
   expect("{");
   readVersionScriptCommand();
@@ -279,6 +295,10 @@ void ScriptParser::readLinkerScript() {
       readTarget();
     } else if (tok == "VERSION") {
       readVersion();
+    } else if (tok == "NOCROSSREFS") {
+      readNoCrossRefs();
+    } else if (tok == "NOCROSSREFS_TO") {
+      readNoCrossRefs(true);
     } else if (SymbolAssignment *cmd = readAssignment(tok)) {
       script->sectionCommands.push_back(cmd);
     } else {
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index fe2e1900520a4..9466d383607c4 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -309,6 +309,9 @@ template <class ELFT> void Writer<ELFT>::run() {
   finalizeSections();
   checkExecuteOnly();
 
+  if (script->nocrossrefs.size())
+    checkNoCrossRefs();
+
   // If --compressed-debug-sections is specified, compress .debug_* sections.
   // Do it right now because it changes the size of output sections.
   for (OutputSection *sec : outputSections)
diff --git a/lld/test/ELF/gnu-nocrossrefs.s b/lld/test/ELF/gnu-nocrossrefs.s
new file mode 100644
index 0000000000000..67d98482cad78
--- /dev/null
+++ b/lld/test/ELF/gnu-nocrossrefs.s
@@ -0,0 +1,110 @@
+// REQUIRES: x86
+
+// RUN: llvm-mc -filetype=obj -o %t.o %s
+// RUN: echo "NOCROSSREFS(.text .text1); \
+// RUN:       SECTIONS { \
+// RUN:         .text  : { *(.text) } \
+// RUN:         .text1 : { *(.text1) } \
+// RUN:         .text2 : { *(.text2) } \
+// RUN: } " > %t.script
+// RUN: not ld.lld %t.o -o %t --script %t.script 2>&1 | FileCheck -check-prefix=ERR %s
+// ERR: ld.lld: error: {{.*}}.o:(.text+{{.*}}): prohibited cross reference from .text to in .text1
+
+// RUN: llvm-mc -filetype=obj -o %t.o %s
+// RUN: echo "NOCROSSREFS_TO(.text1 .text); \
+// RUN:       SECTIONS { \
+// RUN:         .text  : { *(.text) } \
+// RUN:         .text1 : { *(.text1) } \
+// RUN:         .text2 : { *(.text2) } \
+// RUN: } " > %t.script
+// RUN: not ld.lld %t.o -o %t --script %t.script 2>&1 | FileCheck -check-prefix=ERR1 %s
+// ERR1: ld.lld: error: {{.*}}.o:(.text+{{.*}}): prohibited cross reference from .text to in .text1
+
+// RUN: llvm-mc -filetype=obj -o %t.o %s
+// RUN: echo "NOCROSSREFS(.text1 .text .text2); \
+// RUN:       SECTIONS { \
+// RUN:         .text  : { *(.text) } \
+// RUN:         .text1 : { *(.text1) } \
+// RUN:         .text2 : { *(.text2) } \
+// RUN: } " > %t.script
+// RUN: not ld.lld %t.o -o %t --script %t.script 2>&1 | FileCheck -check-prefix=ERR2 %s
+// ERR2: ld.lld: error: {{.*}}.o:(.text+{{.*}}): prohibited cross reference from .text to in .text1
+
+// RUN: llvm-mc -filetype=obj -o %t.o %s
+// RUN: echo "NOCROSSREFS_TO(.text .text1); \
+// RUN:       SECTIONS { \
+// RUN:         .text  : { *(.text) } \
+// RUN:         .text1 : { *(.text1) } \
+// RUN:         .text2 : { *(.text2) } \
+// RUN: } " > %t.script
+// RUN: ld.lld %t.o -o %t --script %t.script 2>&1
+
+// RUN: llvm-mc -filetype=obj -o %t.o %s
+// RUN: echo "NOCROSSREFS_TO(); \
+// RUN:       SECTIONS { \
+// RUN:         .text  : { *(.text) } \
+// RUN:         .text1 : { *(.text1) } \
+// RUN:         .text2 : { *(.text2) } \
+// RUN: } " > %t.script
+// RUN: ld.lld %t.o -o %t --script %t.script 2>&1
+
+// RUN: llvm-mc -filetype=obj -o %t.o %s
+// RUN: echo "NOCROSSREFS(); \
+// RUN:       SECTIONS { \
+// RUN:         .text  : { *(.text) } \
+// RUN:         .text1 : { *(.text1) } \
+// RUN:         .text2 : { *(.text2) } \
+// RUN: } " > %t.script
+// RUN: ld.lld %t.o -o %t --script %t.script 2>&1
+
+// RUN: llvm-mc -filetype=obj -o %t.o %s
+// RUN: echo "NOCROSSREFS(.text); \
+// RUN:       SECTIONS { \
+// RUN:         .text  : { *(.text) } \
+// RUN:         .text1 : { *(.text1) } \
+// RUN:         .text2 : { *(.text2) } \
+// RUN: } " > %t.script
+// RUN: ld.lld %t.o -o %t --script %t.script 2>&1
+
+// RUN: llvm-mc -filetype=obj -o %t.o %s
+// RUN: echo "NOCROSSREFS_TO(.text); \
+// RUN:       SECTIONS { \
+// RUN:         .text  : { *(.text) } \
+// RUN:         .text1 : { *(.text1) } \
+// RUN:         .text2 : { *(.text2) } \
+// RUN: } " > %t.script
+// RUN: ld.lld %t.o -o %t --script %t.script 2>&1
+
+// RUN: llvm-mc -filetype=obj -o %t.o %s
+// RUN: echo "NOCROSSREFS_TO(.text2 .text); \
+// RUN:       SECTIONS { \
+// RUN:         .text  : { *(.text) } \
+// RUN:         .text1 : { *(.text1) } \
+// RUN:         .text2 : { *(.text2) } \
+// RUN: } " > %t.script
+// RUN: ld.lld %t.o -o %t --script %t.script 2>&1
+
+// RUN: llvm-mc -filetype=obj -o %t.o %s
+// RUN: echo "NOCROSSREFS(.text .text2); \
+// RUN:       SECTIONS { \
+// RUN:         .text  : { *(.text) } \
+// RUN:         .text1 : { *(.text1) } \
+// RUN:         .text2 : { *(.text2) } \
+// RUN: } " > %t.script
+// RUN: ld.lld %t.o -o %t --script %t.script 2>&1
+
+.global _start
+_start:
+	nop
+
+.section .text
+test:
+	call test1
+
+.section .text2
+test2:
+	nop
+
+.section .text1
+test1:
+	nop

llvmbot · 2024-06-16T17:54:33Z

@llvm/pr-subscribers-lld

Author: Pavel Skripkin (pskrgag)

Changes

Patch introduces supprot for NOCROSSREFS_(TO) linker script commands. These commands specify which cross-section references should be threated as errors. See more in ld documenmtation [0]

Implementation is straightforward -- traverse all relocations in all object files and report an error if there is prohibited one.

[0] https://sourceware.org/binutils/docs/ld/Miscellaneous-Commands.html

Closes: #41825

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

6 Files Affected:

(modified) lld/ELF/LinkerScript.h (+8)
(modified) lld/ELF/Relocations.cpp (+59)
(modified) lld/ELF/Relocations.h (+1)
(modified) lld/ELF/ScriptParser.cpp (+20)
(modified) lld/ELF/Writer.cpp (+3)
(added) lld/test/ELF/gnu-nocrossrefs.s (+110)

diff --git a/lld/ELF/LinkerScript.h b/lld/ELF/LinkerScript.h
index 734d4e7498aa2..5821b0fc587aa 100644
--- a/lld/ELF/LinkerScript.h
+++ b/lld/ELF/LinkerScript.h
@@ -256,6 +256,11 @@ struct InsertCommand {
   StringRef where;
 };
 
+struct CrossRefList {
+  SmallVector<StringRef, 2> refs;
+  bool firstOnly;
+};
+
 struct PhdrsCommand {
   StringRef name;
   unsigned type = llvm::ELF::PT_NULL;
@@ -394,6 +399,9 @@ class LinkerScript final {
   // OutputSections specified by OVERWRITE_SECTIONS.
   SmallVector<OutputDesc *, 0> overwriteSections;
 
+  // Nocrossrefs sections.
+  SmallVector<CrossRefList, 0> nocrossrefs;
+
   // Sections that will be warned/errored by --orphan-handling.
   SmallVector<const InputSectionBase *, 0> orphanSections;
 
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index 2c02c2e572bfd..400013adf53dd 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -2358,3 +2358,62 @@ template void elf::scanRelocations<ELF32LE>();
 template void elf::scanRelocations<ELF32BE>();
 template void elf::scanRelocations<ELF64LE>();
 template void elf::scanRelocations<ELF64BE>();
+
+static bool isNoCrefFromSection(const CrossRefList &list,
+                                const OutputSection *section) {
+  const auto *begin =
+      list.firstOnly ? list.refs.begin() + 1 : list.refs.begin();
+
+  return std::find(begin, list.refs.end(), section->name) != list.refs.end();
+}
+
+static bool isNoCrefToSection(const CrossRefList &list,
+                              const OutputSection *section) {
+  if (list.firstOnly)
+    return list.refs[0] == section->name;
+
+  return std::find(list.refs.begin(), list.refs.end(), section->name) !=
+         list.refs.end();
+}
+
+void elf::checkNoCrossRefs() {
+  // Basic brute-force algorithm, since in reality NOCROSSRES lists are quite
+  // small.
+  //
+  // Idea is to traverse all relocations in all sections and report if
+  // prohibited reference was found. Note that NOCROSSREFS works with output section
+  // names.
+  for (ELFFileBase *file : ctx.objectFiles) {
+    auto sections = file->getSections();
+    std::string message = "";
+
+    for (size_t i = 0; i < sections.size(); ++i) {
+      if (sections[i]) {
+        StringRef sectionName = sections[i]->name;
+
+        if (const auto *outSec = dyn_cast_or_null<OutputSection>(sections[i]->parent)) {
+          for (const auto &refs : script->nocrossrefs) {
+            if (isNoCrefFromSection(refs, outSec)) {
+
+              for (const auto &j : sections[i]->relocations) {
+                if (auto *def = dyn_cast<Defined>(j.sym)) {
+                  const auto *outSecSym =
+                      cast<InputSection>(def->section)->getParent();
+
+                  if (isNoCrefToSection(refs, outSecSym)) {
+                    message +=
+                        (sections[i]->getLocation(j.offset) +
+                         ": prohibited cross reference from " +
+                         sectionName.str() + " to " + def->getName().str() +
+                         " in " + outSecSym->name.str());
+                    error(message);
+                  }
+                }
+              }
+            }
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/lld/ELF/Relocations.h b/lld/ELF/Relocations.h
index b7b9c09e1b892..eabae26283c24 100644
--- a/lld/ELF/Relocations.h
+++ b/lld/ELF/Relocations.h
@@ -141,6 +141,7 @@ struct JumpInstrMod {
 // the diagnostics.
 template <class ELFT> void scanRelocations();
 void reportUndefinedSymbols();
+void checkNoCrossRefs();
 void postScanRelocations();
 void addGotEntry(Symbol &sym);
 
diff --git a/lld/ELF/ScriptParser.cpp b/lld/ELF/ScriptParser.cpp
index f90ce6fa74075..0c14f43fc5f05 100644
--- a/lld/ELF/ScriptParser.cpp
+++ b/lld/ELF/ScriptParser.cpp
@@ -87,6 +87,7 @@ class ScriptParser final : ScriptLexer {
   void readTarget();
   void readVersion();
   void readVersionScriptCommand();
+  void readNoCrossRefs(bool to = false);
 
   SymbolAssignment *readSymbolAssignment(StringRef name);
   ByteCommand *readByteCommand(StringRef tok);
@@ -235,6 +236,21 @@ void ScriptParser::readVersionScriptCommand() {
   }
 }
 
+void ScriptParser::readNoCrossRefs(bool to) {
+  expect("(");
+
+  script->nocrossrefs.push_back({});
+
+  script->nocrossrefs.back().firstOnly = to;
+
+  while (!atEOF() && !errorCount() && peek() != ")") {
+    StringRef section = next();
+    script->nocrossrefs.back().refs.push_back(section);
+  }
+
+  expect(")");
+}
+
 void ScriptParser::readVersion() {
   expect("{");
   readVersionScriptCommand();
@@ -279,6 +295,10 @@ void ScriptParser::readLinkerScript() {
       readTarget();
     } else if (tok == "VERSION") {
       readVersion();
+    } else if (tok == "NOCROSSREFS") {
+      readNoCrossRefs();
+    } else if (tok == "NOCROSSREFS_TO") {
+      readNoCrossRefs(true);
     } else if (SymbolAssignment *cmd = readAssignment(tok)) {
       script->sectionCommands.push_back(cmd);
     } else {
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index fe2e1900520a4..9466d383607c4 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -309,6 +309,9 @@ template <class ELFT> void Writer<ELFT>::run() {
   finalizeSections();
   checkExecuteOnly();
 
+  if (script->nocrossrefs.size())
+    checkNoCrossRefs();
+
   // If --compressed-debug-sections is specified, compress .debug_* sections.
   // Do it right now because it changes the size of output sections.
   for (OutputSection *sec : outputSections)
diff --git a/lld/test/ELF/gnu-nocrossrefs.s b/lld/test/ELF/gnu-nocrossrefs.s
new file mode 100644
index 0000000000000..67d98482cad78
--- /dev/null
+++ b/lld/test/ELF/gnu-nocrossrefs.s
@@ -0,0 +1,110 @@
+// REQUIRES: x86
+
+// RUN: llvm-mc -filetype=obj -o %t.o %s
+// RUN: echo "NOCROSSREFS(.text .text1); \
+// RUN:       SECTIONS { \
+// RUN:         .text  : { *(.text) } \
+// RUN:         .text1 : { *(.text1) } \
+// RUN:         .text2 : { *(.text2) } \
+// RUN: } " > %t.script
+// RUN: not ld.lld %t.o -o %t --script %t.script 2>&1 | FileCheck -check-prefix=ERR %s
+// ERR: ld.lld: error: {{.*}}.o:(.text+{{.*}}): prohibited cross reference from .text to in .text1
+
+// RUN: llvm-mc -filetype=obj -o %t.o %s
+// RUN: echo "NOCROSSREFS_TO(.text1 .text); \
+// RUN:       SECTIONS { \
+// RUN:         .text  : { *(.text) } \
+// RUN:         .text1 : { *(.text1) } \
+// RUN:         .text2 : { *(.text2) } \
+// RUN: } " > %t.script
+// RUN: not ld.lld %t.o -o %t --script %t.script 2>&1 | FileCheck -check-prefix=ERR1 %s
+// ERR1: ld.lld: error: {{.*}}.o:(.text+{{.*}}): prohibited cross reference from .text to in .text1
+
+// RUN: llvm-mc -filetype=obj -o %t.o %s
+// RUN: echo "NOCROSSREFS(.text1 .text .text2); \
+// RUN:       SECTIONS { \
+// RUN:         .text  : { *(.text) } \
+// RUN:         .text1 : { *(.text1) } \
+// RUN:         .text2 : { *(.text2) } \
+// RUN: } " > %t.script
+// RUN: not ld.lld %t.o -o %t --script %t.script 2>&1 | FileCheck -check-prefix=ERR2 %s
+// ERR2: ld.lld: error: {{.*}}.o:(.text+{{.*}}): prohibited cross reference from .text to in .text1
+
+// RUN: llvm-mc -filetype=obj -o %t.o %s
+// RUN: echo "NOCROSSREFS_TO(.text .text1); \
+// RUN:       SECTIONS { \
+// RUN:         .text  : { *(.text) } \
+// RUN:         .text1 : { *(.text1) } \
+// RUN:         .text2 : { *(.text2) } \
+// RUN: } " > %t.script
+// RUN: ld.lld %t.o -o %t --script %t.script 2>&1
+
+// RUN: llvm-mc -filetype=obj -o %t.o %s
+// RUN: echo "NOCROSSREFS_TO(); \
+// RUN:       SECTIONS { \
+// RUN:         .text  : { *(.text) } \
+// RUN:         .text1 : { *(.text1) } \
+// RUN:         .text2 : { *(.text2) } \
+// RUN: } " > %t.script
+// RUN: ld.lld %t.o -o %t --script %t.script 2>&1
+
+// RUN: llvm-mc -filetype=obj -o %t.o %s
+// RUN: echo "NOCROSSREFS(); \
+// RUN:       SECTIONS { \
+// RUN:         .text  : { *(.text) } \
+// RUN:         .text1 : { *(.text1) } \
+// RUN:         .text2 : { *(.text2) } \
+// RUN: } " > %t.script
+// RUN: ld.lld %t.o -o %t --script %t.script 2>&1
+
+// RUN: llvm-mc -filetype=obj -o %t.o %s
+// RUN: echo "NOCROSSREFS(.text); \
+// RUN:       SECTIONS { \
+// RUN:         .text  : { *(.text) } \
+// RUN:         .text1 : { *(.text1) } \
+// RUN:         .text2 : { *(.text2) } \
+// RUN: } " > %t.script
+// RUN: ld.lld %t.o -o %t --script %t.script 2>&1
+
+// RUN: llvm-mc -filetype=obj -o %t.o %s
+// RUN: echo "NOCROSSREFS_TO(.text); \
+// RUN:       SECTIONS { \
+// RUN:         .text  : { *(.text) } \
+// RUN:         .text1 : { *(.text1) } \
+// RUN:         .text2 : { *(.text2) } \
+// RUN: } " > %t.script
+// RUN: ld.lld %t.o -o %t --script %t.script 2>&1
+
+// RUN: llvm-mc -filetype=obj -o %t.o %s
+// RUN: echo "NOCROSSREFS_TO(.text2 .text); \
+// RUN:       SECTIONS { \
+// RUN:         .text  : { *(.text) } \
+// RUN:         .text1 : { *(.text1) } \
+// RUN:         .text2 : { *(.text2) } \
+// RUN: } " > %t.script
+// RUN: ld.lld %t.o -o %t --script %t.script 2>&1
+
+// RUN: llvm-mc -filetype=obj -o %t.o %s
+// RUN: echo "NOCROSSREFS(.text .text2); \
+// RUN:       SECTIONS { \
+// RUN:         .text  : { *(.text) } \
+// RUN:         .text1 : { *(.text1) } \
+// RUN:         .text2 : { *(.text2) } \
+// RUN: } " > %t.script
+// RUN: ld.lld %t.o -o %t --script %t.script 2>&1
+
+.global _start
+_start:
+	nop
+
+.section .text
+test:
+	call test1
+
+.section .text2
+test2:
+	nop
+
+.section .text1
+test1:
+	nop

github-actions · 2024-06-16T21:59:47Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 253c28fa829cee0104c2fc59ed1a958980b5138c d7baadb71a0590e1950cd4eb744103ece8690eb3 -- lld/ELF/LinkerScript.h lld/ELF/Relocations.cpp lld/ELF/Relocations.h lld/ELF/ScriptParser.cpp lld/ELF/Writer.cpp

View the diff from clang-format here.

diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index 400013adf5..008b8f7efb 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -2381,8 +2381,8 @@ void elf::checkNoCrossRefs() {
   // small.
   //
   // Idea is to traverse all relocations in all sections and report if
-  // prohibited reference was found. Note that NOCROSSREFS works with output section
-  // names.
+  // prohibited reference was found. Note that NOCROSSREFS works with output
+  // section names.
   for (ELFFileBase *file : ctx.objectFiles) {
     auto sections = file->getSections();
     std::string message = "";
@@ -2391,7 +2391,8 @@ void elf::checkNoCrossRefs() {
       if (sections[i]) {
         StringRef sectionName = sections[i]->name;
 
-        if (const auto *outSec = dyn_cast_or_null<OutputSection>(sections[i]->parent)) {
+        if (const auto *outSec =
+                dyn_cast_or_null<OutputSection>(sections[i]->parent)) {
           for (const auto &refs : script->nocrossrefs) {
             if (isNoCrefFromSection(refs, outSec)) {

smithp35

Thanks for submitting this. I'd like to see NOCROSSREFS support as OVERLAY is difficult to use without it.

I've not had a lot of time to go through the code in detail, but I've put some initial comments there based on a quick read. My apologies if I've misinterpreted anything.

lld/ELF/Relocations.cpp

lld/ELF/ScriptParser.cpp

lld/ELF/Relocations.cpp

smithp35 · 2024-06-17T17:08:36Z

lld/ELF/Relocations.cpp

+                      cast<InputSection>(def->section)->getParent();
+
+                  if (isNoCrefToSection(refs, outSecSym)) {
+                    message +=


This could give you an awful lot of error messsages for large sections with lots of relocations. Does GNU ld give an error per relocation or only one per output section cross-reference?

GNU ld reports error per-symbol reference. I.e if symbol is referenced 2+ times in one place, it will be reported once.

Will try to modify reporting scheme, thanks!

Actually I misunderstood ld output.

For input like

int __section(bla) test(void) { return 0; } int test1(void) { test(); return test(); }

GNU ld built locally from master outputs following:

/home/paskripkin/Documents/git/binutils-gdb/ld/ld-new: test.o: in function `test1': test.c:(.text+0x5): prohibited cross reference from .text to `test' in .bla /home/paskripkin/Documents/git/binutils-gdb/ld/ld-new: test.c:(.text+0xa): prohibited cross reference from .text to `test' in .bla

So looks like it indeed reports per-relocation.

smithp35 · 2024-06-17T17:14:34Z

lld/ELF/ScriptParser.cpp

+  script->nocrossrefs.back().firstOnly = to;
+
+  while (!atEOF() && !errorCount() && peek() != ")") {
+    StringRef section = next();


Do we do any checking that the token is a valid OutputSection name?

Although not possible at this point, it may be worth validating that the names given actually match the names of OutputSections in the file (probably warning if it doesn't match).

GNU ld silently ignores unknown section names. Maybe this is because some sections may appear only depending on some build options and warning may abort build process with --fatal-warnings specified.

Anyway, i don't have strong opinion here, I just think it might be a good idea to be fully compatible with GNU ld.

For an initial version I agree that being compatible with GNU ld is worthwhile.

lld/test/ELF/gnu-nocrossrefs.s

lld/ELF/Relocations.cpp

lld/ELF/LinkerScript.h

lld/ELF/Relocations.cpp

mysterymath · 2024-06-18T00:05:06Z

Oh, forgot to mention this with the code review. Big +1 to having NOCROSSREFS in LLD. I was actually about to start implementing it this morning, so this seeing this PR in my email was a very pleasant surprise.

pskrgag · 2024-06-18T15:56:03Z

@mysterymath @smithp35 Thank you a lot for a review!

I hope, I understood correctly your comments.

I apologize for first "dirty" version, I should have worked more with names and comments.

mysterymath · 2024-06-18T18:14:28Z

lld/ELF/Relocations.cpp

+
+    for (const auto &inSection : inSecDescr->sections) {
+      for (const auto &relocation : inSection->relocations) {
+        if (auto *destOutSec = relocation.sym->getOutputSection()) {


Suggested change

if (auto *destOutSec = relocation.sym->getOutputSection()) {

auto *destOutSec = relocation.sym->getOutputSection());

if (!destOutSec)

continue;

lld/ELF/Relocations.cpp

lld/ELF/ScriptParser.cpp

smithp35

Thanks for the update, looks a lot better now, I've not got any more significant comments.

smithp35 · 2024-06-19T13:13:09Z

lld/ELF/ScriptParser.cpp

+  script->nocrossrefs.back().firstOnly = to;
+
+  while (!atEOF() && !errorCount() && peek() != ")") {
+    StringRef section = next();


For an initial version I agree that being compatible with GNU ld is worthwhile.

lld/ELF/Relocations.cpp

pskrgag · 2024-06-19T18:39:38Z

Thank you for a review!
Addressed your comments in the latest version

mysterymath

I'm happy with this change on my end too, pending a few style nits.

lld/ELF/LinkerScript.h

lld/ELF/ScriptParser.cpp

MaskRay · 2024-06-20T21:52:07Z

lld/test/ELF/gnu-nocrossrefs.s

@@ -0,0 +1,150 @@
+// REQUIRES: x86
+// UNSUPPORTED: system-windows


Why is windows prohibited? The test doesn't seem to use backslashes.

A better place is linkerscript/nocrossrefs.test using rm -rf %t && split-file %s %t && cd %t

For some reason this test failed on windows with smth like unsupported file type gnu-crossrefs.o during link stage.

Maybe you have an idea why that happened?

Rewrote test and dropped windows part. Let's see if it will work

lld/ELF/LinkerScript.cpp

MaskRay · 2024-06-21T18:42:08Z

lld/test/ELF/linkerscript/nocrossrefs.s

+// RUN:         .bss   : { *(.unused) } \
+// RUN: } " > %t.script
+// RUN: not ld.lld %t.o -o %t --script %t.script 2>&1 | FileCheck -check-prefix=ERR3 %s
+// ERR3: ld.lld: error: {{.*}}.o:(.text+{{.*}}): prohibited cross reference from .text to unused in .bss


Omit ld.lld: . We don't test the string (which might change) before error:

If you use the # RUN: rm -rf %t && split-file %s %t && cd %t pattern, which is popular in new tests, you can replace {{.*}} with the exact name.
The section offset is important. Don't omit it.

I tried split-file pattern, but it has problems on windows.

In this test split-file + rm should be run before each test, since each script file has its own NOCROSSREFS. On windows this fails, since it's not possible to remove cwd.

I will add checks for offsets

In this test split-file + rm should be run before each test, since each script file has its own NOCROSSREFS. On windows this fails, since it's not possible to remove cwd.

You currently create scripts with echo. With split-file, you place scripts as split-file parts. See addr.test

I guess, I get it, thanks for the tip.
Please check newest version

Sorry to bother, but did you have time to look into new version?

@MaskRay sorry to bother you, but did you have time to check newest version?

@MaskRay sorry to bother you again, but did you have time to check newest version?

Patch introduces supprot for NOCROSSREFS_(TO) linker script commands. These commands specify which cross-section references should be threated as errors. See more in ld documenmtation [0] Implementation is straightforward -- traverse all relocations in all object files and report an error if there is prohibited one. [0] https://sourceware.org/binutils/docs/ld/Miscellaneous-Commands.html Closes: llvm#41825 Signed-off-by: Pavel Skripkin <[email protected]>

MaskRay

Thanks for implementing the feature and sorry for the delay.
I think I would leave a large number of style comments. I realized that I probably should just provide the fixes in my branch for you to cherry pick.

MaskRay · 2024-07-12T21:16:00Z

lld/test/ELF/linkerscript/nocrossrefs.test

+# REQUIRES: x86
+# RUN: rm -rf %t && split-file %s %t && cd %t
+
+# RUN: llvm-mc --triple=x86_64-unknown-linux -filetype=obj -o main.o main.s


remove -unknown-linux. make it clear this doesn't use Linux specific features.

MaskRay · 2024-07-12T21:16:21Z

lld/test/ELF/linkerscript/nocrossrefs.test

+# RUN: rm -rf %t && split-file %s %t && cd %t
+
+# RUN: llvm-mc --triple=x86_64-unknown-linux -filetype=obj -o main.o main.s
+# RUN: not ld.lld main.o -o main --script script1.ld 2>&1 | FileCheck -check-prefix=ERR %s


-o is unneeded when using not ld.lld
(Since we have done cd %t, the unused output file will not clutter the current directory)

MaskRay · 2024-07-12T21:17:09Z

lld/test/ELF/linkerscript/nocrossrefs.test

+
+# RUN: llvm-mc --triple=x86_64-unknown-linux -filetype=obj -o main.o main.s
+# RUN: not ld.lld main.o -o main --script script1.ld 2>&1 | FileCheck -check-prefix=ERR %s
+# ERR: {{.*}} error: main.o:(.text+0x6): prohibited cross reference from .text to in .text1


something is omitted after to

MaskRay · 2024-07-12T21:21:01Z

lld/ELF/ScriptParser.cpp

@@ -235,6 +236,26 @@ void ScriptParser::readVersionScriptCommand() {
  }
 }

+void ScriptParser::readNoCrossRefs(bool to) {
+  expect("(");
+


Our code style is quite terse. We only blank lines when there is a significant "pause" need.

MaskRay · 2024-07-12T21:33:30Z

lld/ELF/Relocations.cpp

@@ -2358,3 +2358,45 @@ template void elf::scanRelocations<ELF32LE>();
 template void elf::scanRelocations<ELF32BE>();
 template void elf::scanRelocations<ELF64LE>();
 template void elf::scanRelocations<ELF64BE>();
+


We don't add code after explicit template instantiations. I would place the checks to LinkerScript.cpp.
Relocations.cpp code should minimize uses of outputSections.

MaskRay · 2024-07-12T22:46:54Z

Sorry, with closer inspection, I believe a large of portion of code and tests need reworking. I think it's best if I create a different PR, but I'll give you credits as a co-author.

My current draft is at https://github.com/MaskRay/llvm-project/tree/nocrossrefs , which still needs a lot of work.

MaskRay · 2024-07-12T22:49:57Z

lld/ELF/Relocations.cpp

+            !list.matchesRefToSection(destOutSec))
+          continue;
+
+        error(inSection->getLocation(relocation.offset) +


In my branch I'll use this:

+ std::string toSymName; + if (!r.sym->isSection()) + toSymName = toString(*r.sym); + else if (auto *d = dyn_cast<Defined>(r.sym)) + toSymName = d->section->name; + errorOrWarn(sec->getLocation(r.offset) + + ": prohibited cross reference to '" + toSymName + "' in " + + dstOsec->name.str());

I think from section can be omitted since getLocation carries the location information.

MaskRay · 2024-07-12T22:50:45Z

lld/test/ELF/linkerscript/nocrossrefs.test

+#--- script1.ld
+NOCROSSREFS(.text .text1);
+SECTIONS {
+	.text  : { *(.text) }


I'll clean up the tests. We actually don't need SECTIONS since the default orphan section placement does the right thing. We need to make these sections SHF_ALLOC, though ("a").

MaskRay · 2024-07-12T23:31:58Z

lld/ELF/Relocations.cpp

+    ArrayRef<OutputSection *> outputSections,
+    llvm::function_ref<void(OutputSection *, InputSectionDescription *)> fn) {
+  for (OutputSection *os : outputSections) {
+    if (!(os->flags & SHF_ALLOC))


GNU ld reports violations due to non-SHF_ALLOC sections. I'll fix it in my branch.

MaskRay · 2024-07-12T23:51:36Z

Note: sec->relocations doesn't work for non-SHF_ALLOC sections. I think we should scan the raw relocations instead.

pskrgag · 2024-07-13T11:10:38Z

Sorry, with closer inspection, I believe a large of portion of code and tests need reworking. I think it's best if I create a different PR, but I'll give you credits as a co-author.

Ok, I am fine with that. Thanks for guidance anyway, maybe next time I would do better. =)

My current draft is at https://github.com/MaskRay/llvm-project/tree/nocrossrefs , which still needs a lot of work.

Should I just close this PR, or you'd like to keep it open for a while?

MaskRay · 2024-07-13T21:01:02Z

Sorry, with closer inspection, I believe a large of portion of code and tests need reworking. I think it's best if I create a different PR, but I'll give you credits as a co-author.

Ok, I am fine with that. Thanks for guidance anyway, maybe next time I would do better. =)

My current draft is at MaskRay/llvm-project@nocrossrefs , which still needs a lot of work.

Should I just close this PR, or you'd like to keep it open for a while?

I think this feature requires more experience with the internals. Perhaps it's not the ideal starter project. However, I appreciate you suggesting the patch! I think the PR can be closed.

Created #98773

pskrgag · 2024-07-13T21:34:10Z

Sorry, with closer inspection, I believe a large of portion of code and tests need reworking. I think it's best if I create a different PR, but I'll give you credits as a co-author.

Ok, I am fine with that. Thanks for guidance anyway, maybe next time I would do better. =)

My current draft is at MaskRay/llvm-project@nocrossrefs , which still needs a lot of work.

Should I just close this PR, or you'd like to keep it open for a while?

I think this feature requires more experience with the internals. Perhaps it's not the ideal starter project. However, I appreciate you suggesting the patch! I think the PR can be closed.

Created #98773

Sure.

Thank you for patience and explanations! I learned a lot =)

Implement the two commands described by https://sourceware.org/binutils/docs/ld/Miscellaneous-Commands.html After `outputSections` is available, check each output section described by at least one `NOCROSSREFS`/`NOCROSSERFS_TO` command. For each checked output section, scan relocations from its input sections. This step is slow, therefore utilize `parallelForEach(isd->sections, ...)`. To support non SHF_ALLOC sections, `InputSectionBase::relocations` (empty) cannot be used. In addition, we may explore eliminating this member to speed up relocation scanning. Some parse code is adapted from #95714. Close #41825 Pull Request: #98773

llvmbot added lld lld:ELF labels Jun 16, 2024

pskrgag mentioned this pull request Jun 16, 2024

"NOCROSSREFS_TO" doesn't supported by LLVM's ld.lld #41825

Closed

dtcxzyw changed the title ~~ldd: add support for NOCROSSREFS(_TO)~~ lld: add support for NOCROSSREFS(_TO) Jun 16, 2024

smithp35 reviewed Jun 17, 2024

View reviewed changes

mysterymath reviewed Jun 18, 2024

View reviewed changes

pskrgag force-pushed the crosrefs branch from d7baadb to 7a8fa49 Compare June 18, 2024 15:50

pskrgag force-pushed the crosrefs branch from 7a8fa49 to 42d3cec Compare June 18, 2024 16:01

mysterymath reviewed Jun 18, 2024

View reviewed changes

smithp35 reviewed Jun 19, 2024

View reviewed changes

pskrgag force-pushed the crosrefs branch from 42d3cec to 6b1be5e Compare June 19, 2024 18:39

mysterymath reviewed Jun 20, 2024

View reviewed changes

lld/ELF/LinkerScript.h Outdated Show resolved Hide resolved

lld/ELF/ScriptParser.cpp Outdated Show resolved Hide resolved

lld/ELF/ScriptParser.cpp Outdated Show resolved Hide resolved

lld/ELF/ScriptParser.cpp Outdated Show resolved Hide resolved

pskrgag force-pushed the crosrefs branch from 6b1be5e to 9abef28 Compare June 20, 2024 18:43

MaskRay reviewed Jun 20, 2024

View reviewed changes

lld/ELF/LinkerScript.cpp Outdated Show resolved Hide resolved

pskrgag force-pushed the crosrefs branch 4 times, most recently from bbc0b3b to fe3f494 Compare June 21, 2024 17:16

MaskRay reviewed Jun 21, 2024

View reviewed changes

pskrgag force-pushed the crosrefs branch from fe3f494 to 05e0a55 Compare June 21, 2024 19:25

MaskRay reviewed Jul 12, 2024

View reviewed changes

MaskRay mentioned this pull request Jul 13, 2024

[ELF] Support NOCROSSREFS and NOCROSSERFS_TO #98773

Merged

pskrgag closed this Jul 13, 2024

		@@ -0,0 +1,150 @@
		// REQUIRES: x86
		// UNSUPPORTED: system-windows

lld: add support for NOCROSSREFS(_TO) #95714

lld: add support for NOCROSSREFS(_TO) #95714

Uh oh!

Conversation

pskrgag commented Jun 16, 2024

Uh oh!

github-actions bot commented Jun 16, 2024

Uh oh!

llvmbot commented Jun 16, 2024

Uh oh!

llvmbot commented Jun 16, 2024

Uh oh!

github-actions bot commented Jun 16, 2024

Uh oh!

smithp35 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mysterymath commented Jun 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pskrgag commented Jun 18, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smithp35 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pskrgag commented Jun 19, 2024

Uh oh!

mysterymath left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

mysterymath commented Jun 18, 2024 •

edited

Loading

MaskRay commented Jul 12, 2024 •

edited

Loading