[PATCH] D154777: [LLD] [COFF] Warn about pseudo relocations that are too narrow

Mon Jul 10 13:21:54 PDT 2023

mstorsjo added a comment.

In D154777#4483231 <https://reviews.llvm.org/D154777#4483231>, @alvinhochun wrote:

> Sounds like a good diagnostic to me. For the warning message perhaps it can be changed a bit to avoid using a full stop?
>
>   runtime pseudo relocation in [...] against symbol [...] is too narrow (only 32 bits wide); this will fail at runtime depending on memory layout

Nice, thanks!

> I also think it can use a slightly stronger tone than "can fail", maybe even worth being an error by default.

Yes, generally whenever you hit this, you have a serious issue anyway. On one hand, I'm worried of breaking cases where things actually do happen to work (for whatever reason - consider things like cygwin, with lots of loader/memory layout trickery - might be doing things differently?). On the other hand, a warning that doesn't break a build is very easily overlooked.

Wrt these issues, we've slowly progressed towards detecting them earlier. Initially, if you struck this, you'd have a strange runtime crash which was mysterious to debug. Nowadays, the mingw-w64 runtime errors out if the actual target symbol is too far away, with a cryptic error message (only giving addresses). With this in place, we'd at least have the symbol name more accessible if someone tries to dig in.

So I'm somewhat undecided between just making it a hard error right away, and merging it as a warning and maybe considering strengthening to an error later.

> With HEASLR being the default nowadays, 32-bit pseudo relocations seems like a recipe for failure, is it not?

Not necessarily; while ASLR does affect the load addresses, I think it primarily affects the base addresses where things are loaded, while most DLLs probably still are loaded close together. Liu Hao struck this issue in May (although in a build linked with binutils, so I'm unsure if HEASLR was enabled there or not), and in that case, the mingw-w64 loader only errored out occasionally, while it mostly worked out ok.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154777