[PATCH] D72992: [llvm-objdump] - Add column headers for relocation printing

Liad Mordekoviz via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 31 04:13:31 PST 2020


liadz0rz marked 2 inline comments as done.
liadz0rz added inline comments.


================
Comment at: llvm/test/tools/llvm-objdump/relocations-elf.test:184
+      Symbol: global
+      Type:   R_386_NONE
+Symbols:
----------------
grimar wrote:
> liadz0rz wrote:
> > grimar wrote:
> > > Why there is no relocation for `local` here?
> > Since you asked for a minimal YAML, I don't mind adding one.
> The YAML description I've suggested earlier tested both local and global:
> 
> ```
>   Relocations:
>     - Offset: 0x1
>       Symbol: global
>       Type:   R_386_PC32
>     - Offset: 0x2
>       Symbol: loc
>       Type:   R_386_32
> ```
> 
> Also, I see no reason to test both local and global symbol for `SHT_RELA`/`SHT_REL` in the `ELFCLASS64` test,
> test both for `SHT_REL` in the `ELFCLASS32` test, but omit the local symbol case for the `SHT_RELA`.
> We want to be consistent usually.
> 
> So yes, while we test both local and global everywhere, this place should not be an exception I think.
Fixed :)


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

https://reviews.llvm.org/D72992





More information about the llvm-commits mailing list