[SymbolVersioning] fix version-script input ordering - #1672
[SymbolVersioning] fix version-script input ordering#1672Rachit Mehta (rachitmeht) wants to merge 1 commit into
Conversation
quic-areg
left a comment
quic-areg
left a comment
There was a problem hiding this comment.
it's a bit of a contrived case, but evaluating --version-script positionally made it become a --start-lib --end-lib member:
ld.eld -shared --whole-archive --start-lib a.o \
--version-script=vs --end-lib --no-whole-archive -o out.so
# Error: vs:1:1: unknown directive: V
# Fatal: Linker script <start-lib:1> has errors
| for (llvm::opt::Arg *arg : Args) { | ||
| switch (arg->getOption().getID()) { | ||
| // -T script, --default-script | ||
| case T::default_script: { |
There was a problem hiding this comment.
Should --default-script be evaluated positionally too?
Scripts should not be part of input tree (--start-lib --end-lib member). is it correct? this will fix the error. |
31ac472 to
475e6e7
Compare
|
Rachit Mehta (@rachitmeht) Can you please rebase this PR on top of origin/main? |
| ThisModule->setFailure(true); | ||
| return false; | ||
| auto *SA = llvm::dyn_cast<eld::ScriptAction>(Action); | ||
| if (SA->kind() == eld::ScriptFile::VersionScript) { |
There was a problem hiding this comment.
Can we please read version script along with the other scripts in ObjectLinker::readInputs? We should only read -T <script> scripts early, and too because some nodes in -T <script> scripts has to be processed early.
One of the previous patch added support for ScriptFile::ScriptActivationKind::Early and ScriptFile::ScriptActivationKind::Full which we can use in ObjectLinker::readLinkerScript while adding the version script to the Module object.
|
Rachit Mehta (@rachitmeht) Can you please check and report what is the behaviour of GNU ld and lld when a version script is present in between |
…behavior
ELD's version-script node order did not follow command-line order;
GNU ld processes both -T and --version-script inline in a single
option loop, so node order always equals CLI order.
Move --version-script into createInputActions() as a ScriptAction
(VersionScript) at its exact CLI position. Add readVersionScriptFile()
to parse it during activateInputs(), mark it setToSkip() to prevent
reprocessing in normalize(), and append to Module::VersionScripts.
Additionally, fix scripts being incorrectly added to the input tree.
ScriptAction::activate() now calls createInput() directly instead of
InputFileAction::activate(), avoiding createInputNode() which adds to
the tree. This prevents scripts from becoming lib members when inside
--start-lib, matching GNU ld behavior.
readLinkerScript() appends embedded VERSION{} blocks to the same
list via addVersionScript(). Module::LinkerScriptVersionScripts,
GeneralOptions::VersionScripts, and their accessors are removed.
Module::VersionScripts is now the single ordered list for both sources.
parseVersionScript() collapses to one loop over getVersionScripts()
calling registerVersionScriptNodes(). Registration stays deferred
as getTargetBackend() is unsafe during activateInputs().
applyVersionScriptScopes() guard changed from hasVersionScript()
to getVersionScriptNodes().empty(). readVersionScriptFile() sets
setParsed()/setToSkip() only after successful parse and null-guards
getVersionScript() before addVersionScript().
Add test VersionScriptInputOrder covering --version-script before/after
-T, two --version-script files in both orders, and embedded-only
VERSION{} local scope.
Signed-off-by: Rachit Mehta <rachmeht@qti.qualcomm.com>
475e6e7 to
b9a511e
Compare
|
GNU gold (bfd doesn't have --start-lib functionality) --start-lib creates an Input_file_lib and sets in_lib = true. --version-script is still treated as a linker option while inside the --end-lib closes the library range. Later, gold uses the parsed version Gold processes -T scripts and --version-script options in the same lld lld first calls readConfigs(), before createFiles(). In readConfigs(), it for (auto *arg : args.filtered(OPT_version_script)) These scripts are parsed before lld processes --start-lib, --end-lib, createFiles() later processes --start-lib and marks object inputs as lazy. Thus lld does not preserves the order between standalone version scripts and embedded VERSION{} For example: -T script-with-VERSION-B --version-script=A.map Gold processes B then A. Current lld processes A first in readConfigs(), then B while processing -T. |
|
Rachit Mehta (@rachitmeht) Thanks for reporting the behavior here across linkers. Can you update the patch such that the |
3 participants
ELD's version-script node order did not follow command-line order; GNU ld processes both -T and --version-script inline in a single option loop, so node order always equals CLI order.
Move --version-script into createInputActions() as a ScriptAction (VersionScript) at its exact CLI position. Add readVersionScriptFile() to parse it during activateInputs(), mark it setToSkip() to prevent reprocessing in normalize(), and append to Module::VersionScripts.
readLinkerScript() appends embedded VERSION{} blocks to the same list via addVersionScript(). Module::LinkerScriptVersionScripts, GeneralOptions::VersionScripts, and their accessors are removed. Module::VersionScripts is now the single ordered list for both sources.
parseVersionScript() collapses to one loop over getVersionScripts() calling registerVersionScriptNodes(). Registration stays deferred as getTargetBackend() is unsafe during activateInputs().
applyVersionScriptScopes() guard changed from hasVersionScript() to getVersionScriptNodes().empty(). readVersionScriptFile() sets setParsed()/setToSkip() only after successful parse and null-guards getVersionScript() before addVersionScript().
Add test VersionScriptInputOrder covering --version-script before/after -T, two --version-script files in both orders, and embedded-only VERSION{} local scope.
Resolves #1636