[PATCH] D79300: [ELF] Demote lazy symbols relative to a discarded section to Undefined

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D79300#2035662 <https://reviews.llvm.org/D79300#2035662>, @ruiu wrote:

> There might be a better way of doing this. As far as I know, directly referring a non-group-leader symbol is not allowed, so the problem we have here is to find an object file that violates the spec. Do you think you can directly implement that logic? I mean, when a symbol is being resolved, we can check if a symbol is directly referring a group internal symbol, and if that's the case, we can emit an error as soon as such a bad symbol use is found.


This approach is the best I can find.

I have thought about restoring rL330869 <https://reviews.llvm.org/rL330869>. It can give us an error, but not a specific error like "relocations refer to a symbol in a discarded section". So it is inferior.

Our current symbol resolution errs that some symbols are incorrectly considered Lazy while should be Undefined. This patch fixes the bug. More precisely, some symbols which should be considered undefined according to the spec:

> A symbol table entry with STB_GLOBAL or STB_WEAK binding that is defined relative to one of a group's sections, and that is contained in a symbol table section that is not part of the group, must be converted to an undefined symbol (its section index must be changed to SHN_UNDEF) if the group members are discarded. References to this symbol table entry from outside the group are allowed.



================
Comment at: lld/ELF/InputFiles.cpp:1124
+      // fetch. We should demote the lazy symbol to an Undefined.
+      if ((sym->symbolKind == Symbol::LazyArchiveKind &&
+           !cast<ArchiveFile>(sym->file)->parsed) ||
----------------
psmith wrote:
> It took me a while to understand what was going on here. Some suggestions
> ```
> ArchiveFile::parsed or !LazyObjFile::fetched means that the file containing this object has not finished processing, i.e. this symbol is a result of a lazy symbol fetch. We should demote the lazy symbol to an Undefined so that any relocations to it will trigger a discarded section error.
> ```
> 
Thanks the suggestion! Adapted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79300