[PATCH] D69093: [llvm-objcopy] --add-symbol: fix crash if SHT_SYMTAB does not exist
Fangrui Song via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 17 03:13:28 PDT 2019
MaskRay marked an inline comment as done.
MaskRay added inline comments.
================
Comment at: test/tools/llvm-objcopy/ELF/add-symbol-no-symtab.test:36
+# SEC2: Index: 1
+# SEC2-NEXT: Name: .shstrtab
+# SEC2-NEXT: Type: SHT_STRTAB
----------------
MaskRay wrote:
> grimar wrote:
> > ".shstrtab" != ".strtab"
> Yes, we overload `.shstrtab` (section names only) for more purposes (section names+symbol names). I think this is fine, but it definitely deserves a comment.
>
> (Similarly, clang reuse .strtab for both section names and symbol names.)
>
> GNU objcopy does not allow such object files (I haven't looked into details what sections (.strtab or .symtab) it expects).
>
> ```
> % objcopy --add-symbol=abs1=1 t t2
> objcopy: error: the input file 'a' has no sections
> ```
To expand a bit more on this topic.
For better test coverage, we should have a test that places .shstrtab before .strtab so that we can check the created .symtab picks .strtab as its sh_link field. But I don't know any way to reorder sections.
Since rL238073, clang no longer produces .shstrtab and uses a unified .strtab for both section names and symbol names. I cannot write something like
if (Sec.Type == ELF::SHT_STRTAB && Obj.SectionNames != &Sec)
because such use cases will fail. Again, I cannot test it with existing yaml2obj functionality (I am not sure it is useful enough to make it possible to test such scenarios... probably not worth it)
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69093/new/
https://reviews.llvm.org/D69093
More information about the llvm-commits
mailing list