[PATCH] D101996: [LLD] Improve reporting unresolved symbols in shared libraries

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin updated this revision to Diff 343634.
ikudrin marked 3 inline comments as done.
ikudrin retitled this revision from "[LLD] Improve reporting of unresolved symbols in shared libraries" to "[LLD] Improve reporting unresolved symbols in shared libraries".
ikudrin edited the summary of this revision.
ikudrin added a comment.

- Updated a comment for `SharedFile::requiredSymbols`
- Moved the test into `allow-shlib-undefined.s`
- Added a test to check that the error is reported for each shared library where the symbol is referenced

In D101996#2742309 <https://reviews.llvm.org/D101996#2742309>, @MaskRay wrote:

>> The current implementation checks the file where the symbol was seen for the first time. That might be a file that does not meet the requirements for reporting, e.g. it might have some DT_NEEDED missed or it might be a relocatable file that also references the symbol. As a result, the actual issue is not detected, the error is not reported, and an incorrect output file is generated whereas it should not.
>
> "it might have some DT_NEEDED missed" -> We don't recursively load DT_NEEDED so this is expected. This patch doesn't address the issue as well. The description may give a false impression that the issue is tackled.

I meant that, as we do not check such files, they may prevent us to detect the issue in files that come after them, just like if a symbol is first seen in a regular object file. The case is roughly the same as with object files, so I did not add a distinct test for that. Probably, it would be more clear to remove that from the description.

> I think the issue is entirely: an undefined symbol in a regular object file shadows an undefined symbol in a shared object, and that regular object file doesn't get diagnosed (`-z defs`) because --gc-sections discards the referencing sections.
> You can amend the description to mention this.

Thanks for the suggestions. I'll update the description.

> Are there other types of missing --no-allow-shlib-undefined diagnostics?

Haven't noticed yet.


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

https://reviews.llvm.org/D101996

Files:
  lld/ELF/InputFiles.cpp
  lld/ELF/InputFiles.h
  lld/ELF/Writer.cpp
  lld/test/ELF/allow-shlib-undefined.s

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D101996.343634.patch
Type: text/x-patch
Size: 4392 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210507/cf4b8cad/attachment.bin>