[PATCH] D57146: [llvm-objdump] - Print LMAs when dumping section headers.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

The code change looks basically fine, but I'm wondering if we really want this? For many (most?) use cases, the LMA is the same as the VMA, so the extra column is just useless noise. I know we want to broadly be consistent with GNU tools, but I wonder if that's the case when it doesn't make that much sense?

What do you think about the following approach:

1. If there are any segments with different p_vaddr and p_paddr, go with your suggestion.
2. If a user specifies a switch, e.g. --show-lma, also go with your suggestion.
3. Otherwise, only print the VMA.

The issue with this is that it might break parsers which are written against GNU objdump's behaviour. I'm not sure if this is an issue or not? The switch is designed to mitigate that problem.

It's probably worth getting some others to give their opinion on this.



================
Comment at: test/tools/llvm-objdump/X86/phdrs-lma.test:23
+## Check we print both VMA and LMA correctly when dumping section headers.
+# RUN: llvm-objdump --section-headers %p/Inputs/phdrs-lma.elf-x86-64 | FileCheck %s
+
----------------
Does yaml2obj provide the ability to specify a different LMA to VMA for segments? That would be better than a pre-built binary, if we can use it.


================
Comment at: tools/llvm-objdump/ELFDump.cpp:142-143
+
+  // We are going to search for PT_LOAD segment where the requested section
+  // lives. That will allow us to calculate it's LMA.
+  for (const typename ELFFile<ELFT>::Elf_Phdr &Phdr : *PhdrOrErr)
----------------
I don't think we need to phrase this comment in the future tense. I think the following would be a better re-wording:

"Search for a PT_LOAD segment containing the requested section. Use this segment's p_addr to calculate the section's LMA".


================
Comment at: tools/llvm-objdump/ELFDump.cpp:149
+
+  // Return section's VMA if we did not find its LMA.
+  return Sec.getAddress();
----------------
Rather than saying we didn't find its LMA, let's be more precise by saying, "if it isn't in a PT_LOAD segment."


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

https://reviews.llvm.org/D57146