[PATCH] D67137: [llvm-objcopy] Implement --only-keep-debug for ELF

[Jake Ehrlich via Phabricator via llvm-commits]

jakehehrlich added a comment.

I'd still like to avoid changing the section type for the reason mentioned, at least until the appropriate point in finalization. I don't want this dangling assumption that casts don't always do what you think they should do.



================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:585-586
+    for (auto &Sec : Obj.sections())
+      if (Sec.Flags & SHF_ALLOC && Sec.Type != SHT_NOTE)
+        Sec.Type = SHT_NOBITS;
+
----------------
MaskRay wrote:
> jakehehrlich wrote:
> > jhenderson wrote:
> > > If the symbol or string tables have SHF_ALLOC set, this will make them SHT_NOBITS. I'm not sure if that matters or not, but wanted to point it out.
> > It matters. Thanks for bringing that up.
> For SHF_ALLOC .shstrtab, we will get: `error: e_shstrndx field value 5 in elf header  is not a string table`
> 
> GNU objcopy handles the section by dropping SHF_ALLOC. I haven't ever seen a tool that can make e_shstrndx SHF_ALLOC, so it probably does not matter.
> 
> -------
> 
> Added a test for SHF_ALLOC .symtab and .strtab. The behavior is consistent with GNU objcopy. The ELF spec says this is allowed but I don't think GNU ld or lld can create such components.
The issue is more the dynamic string table. The dynamic string table *should* be SHT_NOBITS but what matters it that the type information changes. Similar things need to happen for .dynamic and such. At the very least I don't want to carry around the assumption that the type information can change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67137