[lld] Add ability to have OUTPUT_FORMAT(binary) in linker script for ld.lld

[droptopx]

This fixes #87891.

[github-actions]

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]

@llvm/pr-subscribers-lld-elf

@llvm/pr-subscribers-lld

Author: Can (droptopx)

Changes

This fixes #87891.

---

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

1 Files Affected:

• (modified) lld/ELF/ScriptParser.cpp (+15-8)

diff --git a/lld/ELF/ScriptParser.cpp b/lld/ELF/ScriptParser.cpp
index db46263115242..77cbfae30d4d0 100644
--- a/lld/ELF/ScriptParser.cpp
+++ b/lld/ELF/ScriptParser.cpp
@@ -475,6 +475,12 @@ void ScriptParser::readOutputFormat() {
     consume(")");
   }
   s = config->bfdname;
+
+  if (s == "binary") {
+    config->oFormatBinary = true;
+    return;
+  }
+  
   if (s.consume_back("-freebsd"))
     config->osabi = ELFOSABI_FREEBSD;
 
@@ -837,8 +843,7 @@ Expr ScriptParser::readAssert() {
   };
 }
 
-#define ECase(X)                                                               \
-  { #X, X }
+#define ECase(X) {#X, X}
 constexpr std::pair<const char *, unsigned> typeMap[] = {
     ECase(SHT_PROGBITS),   ECase(SHT_NOTE),       ECase(SHT_NOBITS),
     ECase(SHT_INIT_ARRAY), ECase(SHT_FINI_ARRAY), ECase(SHT_PREINIT_ARRAY),
@@ -850,7 +855,8 @@ constexpr std::pair<const char *, unsigned> typeMap[] = {
 // "(TYPE=<value>)".
 // Tok1 and Tok2 are next 2 tokens peeked. See comment for
 // readSectionAddressType below.
-bool ScriptParser::readSectionDirective(OutputSection *cmd, StringRef tok1, StringRef tok2) {
+bool ScriptParser::readSectionDirective(OutputSection *cmd, StringRef tok1,
+                                        StringRef tok2) {
   if (tok1 != "(")
     return false;
   if (tok2 != "NOLOAD" && tok2 != "COPY" && tok2 != "INFO" &&
@@ -1349,10 +1355,10 @@ static std::optional<uint64_t> parseFlag(StringRef tok) {
 // Example: SHF_EXECINSTR & !SHF_WRITE means with flag SHF_EXECINSTR and
 // without flag SHF_WRITE.
 std::pair<uint64_t, uint64_t> ScriptParser::readInputSectionFlags() {
-   uint64_t withFlags = 0;
-   uint64_t withoutFlags = 0;
-   expect("(");
-   while (!errorCount()) {
+  uint64_t withFlags = 0;
+  uint64_t withoutFlags = 0;
+  expect("(");
+  while (!errorCount()) {
     StringRef tok = unquote(next());
     bool without = tok.consume_front("!");
     if (std::optional<uint64_t> flag = parseFlag(tok)) {
@@ -1490,7 +1496,8 @@ Expr ScriptParser::readPrimary() {
     readExpr();
     expect(")");
     script->seenRelroEnd = true;
-    return [=] { return alignToPowerOf2(script->getDot(), config->maxPageSize); };
+    return
+        [=] { return alignToPowerOf2(script->getDot(), config->maxPageSize); };
   }
   if (tok == "DEFINED") {
     StringRef name = unquote(readParenLiteral());

[droptopx]

Hang on while I fix the unintended automatic formatting.

[droptopx]

Should be good now!

[MaskRay]

This needs a test in oformat-binary.s. A different OUTPUT_FORMAT might change oFormatBinary to false.

[droptopx]

This needs a test in oformat-binary.s. A different OUTPUT_FORMAT might change oFormatBinary to false.

Okay, I want to write test(s) for this but I'm not sure what we need to be testing for here. Why would we want to test for the behaviors of OUTPUT_FORMATs other than binary? This PR only changes the behavior of OUTPUT_FORMAT(binary).

Is the test for the case where there are more than one OUTPUT_FORMAT directives present in a linker script? Does that even make sense for a linker script?

[MaskRay]

This needs a test in oformat-binary.s. A different OUTPUT_FORMAT might change oFormatBinary to false.

Okay, I want to write test(s) for this but I'm not sure what we need to be testing for here. Why would we want to test for the behaviors of OUTPUT_FORMATs other than binary? This PR only changes the behavior of OUTPUT_FORMAT(binary).

Is the test for the case where there are more than one OUTPUT_FORMAT directives present in a linker script? Does that even make sense for a linker script?

We want to test the behavior when more than one OUTPUT_FORMAT command is specified and the interaction with --oformat=binary (and when an ELF OUTPUT_FORMAT is specified).

After closer inspection of GNU ld's behavior, I think we probably should ignore extra OUTPUT_FORMAT commands. #98837

[droptopx]

Closing as #98837 has been merged. Thanks.