[PATCH] D83935: [llvm-readobj] - Move out the common code from printRelocations() methods.
George Rimar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 21 02:14:13 PDT 2020
grimar marked an inline comment as done.
grimar added inline comments.
================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:3636-3647
+void GNUStyle<ELFT>::printRelReloc(const ELFO *Obj, unsigned SecIndex,
+ const Elf_Shdr *SymTab, const Elf_Rel &R,
+ unsigned RelIndex) {
+ printRelRelaReloc(Obj, SecIndex, SymTab, R, RelIndex);
+}
+
+template <class ELFT>
----------------
jhenderson wrote:
> It seems to me like this pair of functions (and the equivalent LLVM pair) is unnecessary. You should be able to call `printRelRelaReloc` directly.
`printRelRelaReloc` is a non-virtual template method, defined for both `LLVMStyle<ELFT>` and `GNUStyle<ELFT>` output styles.
```
template <class ELFT>
template <class RelTy>
void LLVMStyle<ELFT>::printRelRelaReloc(const ELFO *Obj, unsigned SecIndex,
const RelTy &Rel, unsigned RelIndex,
const Elf_Shdr *SymTab)
```
```
template <class ELFT>
template <class RelTy>
void GNUStyle<ELFT>::printRelRelaReloc(const ELFO *Obj, unsigned SecIndex,
const Elf_Shdr *SymTab, const RelTy &R,
unsigned RelIndex)
```
The only way I can imagine how to call it directly from the base class is to detemplate it, make it virtual and return to having `Elf_Rela R, bool IsRela` or something alike in the signature, what didn't work good (D83871).
Or, may be we can pass both `Elf_Rela` and `Elf_Rel` at the same time. E.g:
```
void GNUStyle<ELFT>::printRelRelaReloc(const ELFO *Obj, unsigned SecIndex,
const Elf_Shdr *SymTab, unsigned RelIndex, const Elf_Rela *Rela, const Elf_Rela *Rel) {
assert((Rela || Rel) && !(Rela && Rel));
...
}
```
or introduce a helper struct to store either both relocations ot `Elf_Rel` + optional addend. In all that cases this needs `printRelRelaReloc` to be detemplated and virtualized. Want me to try this way?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83935/new/
https://reviews.llvm.org/D83935
More information about the llvm-commits
mailing list