[PATCH] D103303: [ELF] Add --overwrite-script

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

Sorry for the delay in responding. I'm in favour of adding this as it should combine well with LLD when no default linker script is used.

I'm thinking overwrite-section-script may be a better name than --overwrite-script. It would reinforce the narrow nature of the script, to overwrite specific SECTIONS commands, and also help people not seeing it as overwriting their whole linker script.

With respect to Roland's comment in https://sourceware.org/bugzilla/show_bug.cgi?id=26404#c10 one way of having this work by adding another script fragment would be to add a keyword at the top of the linker script for example:

  /* This linker script fragment is an overwrite script */
  OVERWRITE_SCRIPT()

Will be good to have a test to show how --overwrite-script combines with INSERT AFTER and INSERT BEFORE



================
Comment at: lld/ELF/Driver.cpp:222
   case file_magic::unknown:
-    readLinkerScript(mbref);
+    readLinkerScript(mbref, false);
     return;
----------------
Could we put /* overwrite */ before the false.


================
Comment at: lld/ELF/Options.td:369
 defm script: Eq<"script", "Read linker script">;
+defm overwrite_script: EEq<"overwrite-script", "Read a script which overrides the main script">;
 
----------------
I'd recommend being more specific here
"Read a script containing SECTION commands that replace SECTIONS in the main or default linker script."

Will be good to update docs/ld.lld.1 and perhaps the release notes?


================
Comment at: lld/ELF/ScriptParser.cpp:282
+    expect("}");
+  }
+}
----------------
expect("SECTIONS") will do a good job of preventing other linker script commands, but it may leave people wondering why? Would it be possible to do something like:
```
  expect("SECTIONS")
  if (!errorCount())
    // Diagnostic "overwrite script can only contain SECTIONS commands."
``` 


================
Comment at: lld/test/ELF/linkerscript/overwrite-script.test:106
+  A = 1;
+}
----------------
Will be good to have a diagnostic for an overwrite script that doesn't contain SECTIONS.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103303/new/

https://reviews.llvm.org/D103303