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

Thu Oct 7 02:31:42 PDT 2021

jhenderson added a comment.

In D111181#3047360 <https://reviews.llvm.org/D111181#3047360>, @ardb wrote:

> In D111181#3047338 <https://reviews.llvm.org/D111181#3047338>, @jhenderson wrote:
>
>> 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.
>
> This case is actually covered by my latest revision: SHF_ALLOC is taken into account as before for non-ET_REL type files.

Yes, I see. I haven't had the time today unfortunately to think about the code change itself in this patch. It's on my backlog.

>> 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.
>
> In that case, does it even matter? If there is no symtab to begin with, there is no risk of misidentifying a static one as a dynamic one, righr?

Not sure if you're referring to the symtab here or the relocation section. At the moment, I believe there are three cases that are handled (I might be wrong though - I'm going off memory, as I don't have time to dig into the code):

1. Relocation sections with sh_link = 0. In this case, don't attempt to look for a symbol table.
2. Static relocation sections should have a sh_link pointing to a static symbol table.
3. Dynamic relocation sections should have a sh_link pointing to a dynamic symbol table.

In the latter two cases, we can reverse the logic and say that the symbol table kind (dynamic or static) identifies the relocation section kind. However, in the former case, we don't know by looking at sh_link whether the symbol table is static or dynamic. sh_link = 0 is fine for some relocation sections: they need to have all relocation have an r_sym of 0, i.e. be relocation types that aren't relative to a symbol. We do need to still distinguish betewen whether the relocation seciton is static or dynamic, as the llvm-objcopy code for the two is different.

> But if we disregard this patch, there is still an issue where llvm-objcopy is able to generate ELF files that are malformed according to its own standards. I.e.,
>
> $ llvm-objcopy --set-section-flags=.rela.text=alloc,readonly <object file>
>
> will generate a file that llvm-objcopy itself is not able to read.

That's a fair point. I don't think it's unreasonable to allow llvm-objcopy to generate invalid objects (in the sense that it wouldn't be useful/the linker might reject it etc), by misusing some switches, but it should probably be able to consume such objects itself, so that it can do //something// sensible, in particular so that a user could reverse the option.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111181