[PATCH] D45375: [ELF] - Introduce synthetic file for linker script symbols.

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

In https://reviews.llvm.org/D45375#1060199, @ruiu wrote:

> I'm not too excited about doing this. It looks like it opens a can of worms, even if the change itself is small.


I think it is opened now. See what we have with null input file:

1. We crash (see: https://reviews.llvm.org/D45440. That was a quick find, looking for more). It is hard to avoid such bugs in future

because having the null value for `File` is really not obvious.

2. We are not always able to report locations in errors properly (see relocation-relative-absolute.s of this patch),

it would simply report `<internal> location for script symbol.

3. Our -cref prints broken locations for linker script symbols. (cref.s test case of this patch).
4. Our -trace-symbol is broken for linker script symbols. I'll update the diff with the test case.
5. Our checks in code are more complicated (see in this patch).

We can avoid adding the new file class btw. Just a new type would be enough. I'll update the patch soon.



================
Comment at: ELF/ScriptParser.cpp:267
 void ScriptParser::readDefsym(StringRef Name) {
+  std::string Loc = getCurrentLocation();
   Expr E = readExpr();
----------------
espindola wrote:
> Why this change?
Otherwise in cref.s we would output `-defsym:1` for Location
(with that change it prints `-defsym`). Just a bit nicer.
(see `ScriptLexer::getCurrentLocation()`  https://github.com/llvm-mirror/lld/blob/master/ELF/ScriptLexer.cpp#L69).


https://reviews.llvm.org/D45375