Expand =/$SYSROOT for INPUT/GROUP script commands#950
Expand =/$SYSROOT for INPUT/GROUP script commands#950parth-07 wants to merge 1 commit intoqualcomm:mainfrom
Conversation
b5640a0 to
c91146e
Compare
quic-seaswara
left a comment
There was a problem hiding this comment.
This is very cool feature!
| InputStrTok = | ||
| ThisScriptFile.createFileToken(Name.str(), ThisScriptFile.asNeeded()); | ||
| ThisScriptFile.createFileToken(ExpandedName, ThisScriptFile.asNeeded()); | ||
| } |
There was a problem hiding this comment.
This needs to be done in resolvePath
For example
ld.bfd --sysroot=/tmp/xxx/ =main.o
I could not find this expansion of $SYSROOT for input files, may be I am not using the right version of ld.
There was a problem hiding this comment.
ld.bfd --sysroot=/tmp/xxx/ =main.o
Thank you for adding this comment. I did not know GNU ld supported this. GNU ld docs does not mention anywhere that = and $SYSROOT are expanded for filenames specified directly in the link command. In my local experiments with GNU ld 2.38, I am able to reproduce SYSROOT expansion for both =main.o and $SYSROOT/main.o. I am updating the patch to include this functionality.
There was a problem hiding this comment.
Done. I have updated the documentation sysroot.rst.in as well.
test/Common/standalone/linkerscript/SysrootExpansion/Inputs/group_equals.t
Show resolved
Hide resolved
| RUN: %link %linkopts -o %t.out2 --sysroot=%t.dir -T %p/Inputs/script_sysroot.t %t.main.o --verbose 2>&1 | %filecheck %s --check-prefix=SYSROOT | ||
| RUN: %link %linkopts -o %t.out3 --sysroot=%t.dir -T %p/Inputs/group_equals.t %t.main.o --verbose 2>&1 | %filecheck %s --check-prefix=GROUP | ||
| RUN: %not %link %linkopts -o %t.out4 -T %p/Inputs/script_equals.t %t.main.o 2>&1 | %filecheck %s --check-prefix=NOSYSROOT | ||
| #END_TEST |
There was a problem hiding this comment.
Need verbose output to show how the files are resolved.
Also please annotate map file to show what files are being expanded.
There was a problem hiding this comment.
I have added the verbose output for the sysroot marker expansion. Can you please help with how we should annotate the sysroot marker expansion in the map-file? Should we add a new section in the map-file with all files that had sysroot markers and their corresponding expansions?
There was a problem hiding this comment.
We can use the map file that shows LOAD with
LOAD <user passed in file> #expanded file information
There was a problem hiding this comment.
I will make the map-file change as a follow-up.
| void ScriptParser::addFile(StringRef Name) { | ||
| StrToken *InputStrTok = nullptr; | ||
| if (Name.consume_front("-l")) | ||
| InputStrTok = ThisScriptFile.createNameSpecToken(Name.str(), |
There was a problem hiding this comment.
What does ld for namespec files ?
There was a problem hiding this comment.
I could not find any difference in interpretation for namespec files with explicit sysroot in GNU ld.
This commit improves the script parser to expand the = and $SYSROOT prefix in INPUT/GROUP inputs. Resolves qualcomm#927 Signed-off-by: Parth Arora <partaror@qti.qualcomm.com>
c91146e to
09d0207
Compare
| RUN: %link %linkopts -o %t.out2 --sysroot=%t.dir -T %p/Inputs/script_sysroot.t %t.main.o --verbose 2>&1 | %filecheck %s --check-prefix=SYSROOT | ||
| RUN: %link %linkopts -o %t.out3 --sysroot=%t.dir -T %p/Inputs/group_equals.t %t.main.o --verbose 2>&1 | %filecheck %s --check-prefix=GROUP | ||
| RUN: %not %link %linkopts -o %t.out4 -T %p/Inputs/script_equals.t %t.main.o 2>&1 | %filecheck %s --check-prefix=NOSYSROOT | ||
| #END_TEST |
There was a problem hiding this comment.
We can use the map file that shows LOAD with
LOAD <user passed in file> #expanded file information
| RUN: %link %linkopts -o %t.out2 --sysroot=%t.dir -T %p/Inputs/script_sysroot.t %t.main.o --verbose 2>&1 | %filecheck %s --check-prefix=SYSROOT | ||
| RUN: %link %linkopts -o %t.out3 --sysroot=%t.dir -T %p/Inputs/group_equals.t %t.main.o --verbose 2>&1 | %filecheck %s --check-prefix=GROUP | ||
| RUN: %not %link %linkopts -o %t.out4 -T %p/Inputs/script_equals.t %t.main.o 2>&1 | %filecheck %s --check-prefix=NOSYSROOT | ||
| #END_TEST |
| bool InputFileAction::activate(InputBuilder &PBuilder) { | ||
| const LinkerConfig &Config = PBuilder.getLinkerConfig(); | ||
| std::string ExpandedName = Input::expandSysrootMarkers( | ||
| Name, Config.directories(), *Config.getDiagEngine()); |
There was a problem hiding this comment.
Please open an issue to support these with reproduce.
| } | ||
| if (!llvm::sys::fs::exists(ResolvedPath->native())) { | ||
| const sys::fs::Path *P = PSearchDirs.find(FileName, Input::Script); | ||
| const sys::fs::Path *P = |
There was a problem hiding this comment.
Do we want to have it const ?
| else | ||
| return Name.str(); | ||
|
|
||
| std::string ExpandedPath; |
There was a problem hiding this comment.
Default ExpandedPath = suffix.str()
| if (Name.starts_with("=")) | ||
| Suffix = Name.substr(1); | ||
| else if (Name.starts_with("$SYSROOT")) | ||
| Suffix = Name.substr(strlen("$SYSROOT")); |
There was a problem hiding this comment.
Is $SYSROOT a reserved word ?
What about linker script file patterns ? Does that support = ?
This commit improves the script parser to expand
the = and $SYSROOT prefix in INPUT/GROUP inputs.
Resolves #927