[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