[PATCH] D96613: [lld] Add options to trace all symbols and to trace all symbols originated from a file

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Feb 28 12:38:41 PST 2021


MaskRay added a comment.

In D96613#2593029 <https://reviews.llvm.org/D96613#2593029>, @hoy wrote:

> In D96613#2592675 <https://reviews.llvm.org/D96613#2592675>, @MaskRay wrote:
>
>> I have several concerns.
>>
>> - The slowdown. You can test some internal projects.
>
> Our internal projects mostly use thinLTO thus the overhead here is negligible. Without thinLTO, generated code becomes much smaller (due to lack of cross-module inlining). Anyway, I'll continue my measurement. If there's any regression, I think we can optimize the checks to just favor non-debug scenario, like another bool flag to identify if `config->traceSymbolsFromFile` is empty?
>
>> - This does not work with versioned symbols in shared objects.
>
> Does the exiting `--trace-symbol` handle versioned symbols?

Yes. See `verneed-shared.s`

>> - `The patch uses this->file, however, the value may change if the symbol gets resolved to other files. In the cases I can conceive we want the trace of the full lifetime of a symbol, not only when it is bound to a specific file.`
>
> Is this for weak symbols? `--trace-symbols-from-file` aims to trace where a definition of a symbol is finally picked up.

Not just weak symbols. See `resolveUndefined` where an undefined symbol's file can be replaced.
By using `Symbol::file`, the semantics of the patch are not clear.

>> - The usefulness is not justified. This looks like a debug option. I raised `ld.lld @response.txt $(llvm-nm -Du usr/lib64/libc.so.6 | awk '{print "-y"substr($0,20)}')` as an example how this can be trivially implemented with tool composing.
>
> As @smeenai pointed out, that approach also has limitations.

You are proposing new options, so you need to justify the options. I've also asked other folks and many feel that this is unnecessary.

>> https://sourceware.org/bugzilla/show_bug.cgi?id=27407 If you can make arguments there, it will probably make this proposal more competitive.
>
> Sorry, I don't quite get this. Are we trying to have the gnu linker implement the two switches as well? I don't see an objection on their side.

When things are in doubts, one tie-breaking thing you can try is to make other implementations accept your proposal. That could justify the merit of the proposal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96613



More information about the llvm-commits mailing list