linkerscript: fix ENTRY() last-wins - #1642
Conversation
4d4ad88 to
3758b29
Compare
| CurModule.getConfig().options().setEntry(EntrySymbol); | ||
| GeneralOptions &Options = CurModule.getConfig().options(); | ||
| if (!EntrySymbol.empty() && !Options.isEntryFromCmdLine()) | ||
| Options.setEntry(EntrySymbol); |
There was a problem hiding this comment.
how do we handle
ld.eld -e -T <script>
and the script has ENTRY ?
Can we add a warning with -Wlinker-script when we see multiple ENTRY commands mentioned in the link line either in command line or linker script ?
There was a problem hiding this comment.
It is handled similarly, how GNU ld handles, the command line -e will always take precedence, irrespective of linker script's ENTRY(). To keep track of this we need bool EntryFromCmdLine.
For multiple -e currently last one is considered
| # Case 4: -e with a raw address overrides script ENTRY(). | ||
| # Case 5: Across multiple -T scripts, the last ENTRY() activated wins. | ||
| # Case 6: -e overrides script ENTRY() regardless of command-line order | ||
| # (i.e. -e placed before -T still wins). |
There was a problem hiding this comment.
Please document this as well.
| unsigned NumEntryArgs = 0; | ||
| for (auto *Arg : Args.filtered(T::entrypoint)) | ||
| ++NumEntryArgs; | ||
| if (NumEntryArgs > 1 && Config.showLinkerScriptWarnings()) |
There was a problem hiding this comment.
Why is multiple -e command-line options part of the linker script warnings?
| unsigned NumEntryArgs = 0; | ||
| for (auto *Arg : Args.filtered(T::entrypoint)) | ||
| ++NumEntryArgs; | ||
| if (NumEntryArgs > 1 && Config.showLinkerScriptWarnings()) | ||
| Config.raise(Diag::warn_multiple_entry); | ||
|
|
There was a problem hiding this comment.
I do not think we should warn for multiple -e options, and if we should, then it should be under some command-line warning category.
There was a problem hiding this comment.
I understood the warning should be under -Wcommand-line, but if we are warning on multiple ENTRY() then we should also warn for multiple -e. aren't they the same case?
There was a problem hiding this comment.
but if we are warning on multiple ENTRY() then we should also warn for multiple -e. aren't they the same case?
They are similar for sure, but passing same options multiple times is well-defined (and often well-used) behavior. And if we are giving warning on multiple -e options, then we should likely do the same for other options as well which can be repeated and last one takes effect. This being said, I personally do not think we should report warning on repeated command-line options. Shankar Easwaran (@quic-seaswara) what do you think about it?
There was a problem hiding this comment.
Rachit Mehta (@rachitmeht) for now can you please remove warning from here?
| if ((!EntrySymbol.empty()) && (!(CurModule.getConfig().options().hasEntry()))) | ||
| CurModule.getConfig().options().setEntry(EntrySymbol); | ||
| GeneralOptions &Options = CurModule.getConfig().options(); | ||
| if (!EntrySymbol.empty()) { |
There was a problem hiding this comment.
I know it was part of the original code, but do you know if there is any valid case where EntrySymbol may be empty?
There was a problem hiding this comment.
Syntactically we can pass ENTRY("") it will result in EntrySymbol.empty(),
lld do gives warning in this case but ld.eld and ld.bfd silently ignores it
ld.lld: warning: cannot find entry symbol _start; not setting start address
Entry point address: 0x0
There was a problem hiding this comment.
This is interesting. We should also give warning for it. Can you please raise an issue for this?
There was a problem hiding this comment.
ENTRY("") is treated as having no ENTRY() at all.
lld gives this warning if we have not specified an entry point at all.
91742df to
127d917
Compare
EntryCmd::activate() guarded with hasEntry(), making the first ENTRY() win over later ones and allowing script ENTRY() to overwrite a -e value set on the command line. Similar to GNU ld, replace the guard with !isEntryFromCmdLine() so each ENTRY() overwrites the previous (last wins), while -e is always preserved regardless of script processing order. Add EntryCmdPrecedence.test covering: last-wins within one script, -e override, last -e wins, raw-address -e 0, cross-script last-wins, and -e placed before -T. Resolves qualcomm#1538 Signed-off-by: Rachit Mehta <rachmeht@qti.qualcomm.com>
Add EntryCmdWarn.test covering rules and warning behaviour. Document entry point precedence and the new warning in linker_script.rst, linker_faq.rst, and diagnostic_reports.rst. Resolves qualcomm#1538 Signed-off-by: Rachit Mehta <rachmeht@qti.qualcomm.com>
127d917 to
22086fe
Compare
3 participants
EntryCmd::activate() guarded with hasEntry(), making the first ENTRY() win over later ones and allowing script ENTRY() to overwrite a -e value set on the command line.
Similar to GNU ld, replace the guard with !isEntryFromCmdLine() so each ENTRY() overwrites the previous (last wins), while -e is always preserved regardless of script processing order.
Add EntryCmdPrecedence.test covering: last-wins within one script, -e override, last -e wins, raw-address -e 0, cross-script last-wins, and -e placed before -T.
Resolves #1538