[PATCH] D109400: [LLD] Add archive Name to relocaiton overflow printout

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D109400#3109460 <https://reviews.llvm.org/D109400#3109460>, @ayermolo wrote:

> In D109400#3094169 <https://reviews.llvm.org/D109400#3094169>, @MaskRay wrote:
>
>> Thanks:)
>>
>> ---
>>
>> I think companies shipping large executables may have similar needs. Internally someone asked me whether LLD can record a section but I feel that `--emit-relocs` is the right solution.
>> I am always concerned with whether we may spend too much code on a rather specific use case where an alternative existing functionality exists.
>> I'd still say that watching relocations is like shooting a different target. The build system can rearrange dependencies/object files and the max relocation distance may not be representative.
>> (Say the longest relocation can disappear in the next build and the second longest relocation in the previous build can become the longest one.)
>
> I understand we don't want to chase too specialized use cases, but Relocation overflows do happen to multiple users of LLD. One can just search stack overflow with people asking "What does this relocation overflow mean". Adding extra information on none-critical path that will tell user at a glance, without re-linking, and doing extra steps, what the state of the binary is and what is the source and destination of relocation does improve user experience. Instead of every user who experiences relocation overflow being forced to repeat same steps over and over, LLD can produce that information with a cost of one function in Relocations.cpp. It is less to do with chasing any particular relocation, but providing a high level overview of what went wrong and where to look next. For example if reloc was between .gcc_except and .data, one can look at sections between those to see if maybe something there that can be moved out of the way, or one of the other sections is abnormally large, etc.

Extra steps are unavoidable because out-of-range relocations lead to errors, not warnings, and errors suppress the output.
The current diagnostic already tells the user the relocated location and the referenced symbol (I added the referenced symbol).
I don't agree that recording the information in a section can improve the general user experience.
It might make problems easier to analyze for a targeted small group of developers who know the existence of an analyzer and know how to use it.
As I said, `--emit-relocs` can basically do the same job, just requiring the interested small group of developers making the tool in a different way.
I also disagree that the cost is just one function in Relocations.cpp. Implementing it needs changes in multiple places and the complexity may not justify it if another tool exists and does the same job.
(Last, some may have tendency guessing the root cause of an out-of-range relocation. That would sometimes confuse users as the root cause might be incorrect (a "-Wl,-z,notext" diagnostic I removed belongs to the same broad category).)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109400