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

Thu Nov 4 18:31:29 PDT 2021

MaskRay added a comment.

In D109400#3110411 <https://reviews.llvm.org/D109400#3110411>, @wenlei wrote:

> Hey @MaskRay, overall I agree that any complexity added into any tool needs justification. There's always a trade off - sometimes it's obvious, others less so. However, I think in this case there's perhaps a misunderstanding. There were also discussions about related topic on other patches, so I chatted with @ayermolo off patch. Let me try to clarify things a bit.
>
> 1. Recording all potential out-of-range relocations you mentioned above. This is non-trivial complexity, and it's somewhat duplicated to `--emit-reloc` as you pointed out. I also agree that going after specific relocation isn't the right approach for dealing with this problem. But this is *not* what was proposed here or in other patches.

Thanks for understanding.

> 2. Better diagnostics when out-of-range relocation happens. This is what Alex was trying to do here and in other patches. I think this is reasonable - it helps developers understand the problem quicker, and the implementation is also simple. There're different things we can do here: 1). print from/to section in addition to from/to archive; 2) print the full section layout on error.

For the archive name, I also find it useful. I have implemented it in D112518 <https://reviews.llvm.org/D112518>.
If there are cases missing, we can discuss from there.

> Internally, we benefited a lot from printing out section layout when hitting out-of-range relocation. Yes, you can rebuild with `-M` added then `sed` to get the equivalent info. But having such info right there on error is just a much nicer workflow for error reporting or quick triaging. Note that LLVM/LLD is also a tool used by average non-compiler developers, so the knowledge it requires to get the equivalent info through `-M` + `sed` is honestly a high bar for many people, and yet compiler engineers don't often have the environment to easily rebuild and repro the issues. For this reason, I believe such improved diagnostics for print from/to section and section layout has value for others too. It is a diagnostics improvement for error case, not a new feature under a new switch. Print section layout will add some complexity, however it's a pretty mechanic change not touching any fundamental elements of linker. So I hope that this trade-off makes sense and can be accepted. Of course, different people may view the value of such diagnostics differently, but I hope there's empathy for accepting such improvements because 1) it has shown the need in some places, 2) improving diagnostics on error should be a net good thing for user-ability.
>
> WDYT?

The description is not what this patch does. Do you mean D94141 <https://reviews.llvm.org/D94141>?
As I said, I do not think auto -M or section output behavior is useful.
It is also not much to ask the user to re-link with an option.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109400