[PATCH] D111181: [llvm-objcopy][ELF] Don't assume RELA sections are dynamic if they carry the SHF_ALLOC flag

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

If there's a real use-case, I'm not opposed to handling more unusual code, such as SHF_ALLOC relocations that are not intended for patching by the dynamic loader - if there's a still a need for it, I'd be happy to support working on a solution in llvm-objcopy/llvm-strip. The problem as I see it is that the only true way of determining whether a relocation section is really a static or dynamic relocation section is the dynamic table, which llvm-objcopy traditionally has not had a need of. We can't rely on the ELF type for two reasons:

1. Custom ELF types could be either relocatable or non-relocatable (this is less important - producers of such custom ELF objects usually have to rely on downstream patches to keep things working for them anyway).
2. Static relocations may exist in fully linked objects.

A simpler approach than inspecting the dynamic table might be to inspect the type of the sh_link-ed section: SHT_SYMTAB implies a static relocation section, and SHT_DYNSYM a dynamic one. There is a small corner case this approach doesn't cover, namely relocation sections with sh_link=0, but in that case, falling back to another heuristic like SHF_ALLOC or parsing the dynamic section may be sufficient.

> I guess you are saying that llvm-objcopy only targets the subset of well-formed, compliant ELF files that are likely to be emitted by tools created for building user space programs.

As the person who reviewed 99% of the original ELF llvm-objcopy implementation, I can safely say that the intent of llvm-objcopy was to handle as many cases as made sense. At the time, Jake and I were unaware of any static relocation sections with SHF_ALLOC, hence it was implemented this way. Had we known of actual situations, we would have implemented it differently from the get-go: it certainly wasn't the intent for llvm-objcopy to be limited to handling a specific subset, but at the same time, we couldn't invest the time to handle every single corner case.

There's another side-issue with SHF_ALLOC static relocations, at least in llvm-objcopy: llvm-objcopy tries very hard to not change contents of memory-allocated data, because it can impact address references. Having said that, I think in an ET_REL case, we can probably ignore this specific issue. It's therefore probably safe to assume that if the type is ET_REL, all relocations are static, but not necessarily otherwise. (I'm assuming the partially linked ET_RELs in the Linux kernel still have relocations to patch all inter-section references?).

> I think we can all agree that the error message here could be more helpful to the end user.

I'd be happy to review a simple patch that simply added the words "static" or "dynamic" as appropriate to the message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111181