[PATCH] Fix "llvm-objdump -d -r" to show relocations inline for ELF files

Mark Seaborn mseaborn at chromium.org
Wed Jan 22 09:27:30 PST 2014



================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:395
@@ +394,3 @@
+    if (Sec2 != Obj->end_sections())
+      SectionRelocMap.insert(std::pair<SectionRef, SectionRef>(*Sec2, *i));
+  }
----------------
Rafael Ávila de Espíndola wrote:
> This assumes that only one section has relocations for another section. That is probably an OK assumption to start with, but please assert it.
> 
> If you are interested you might also want to add a test with a .so in the future. The relocations on shared objects have a slightly different representation from the ones in relocatable objects.
> 
OK, it's easy enough to change it to handle multiple relocation sections per section, so I've done that.

For testing shared objects, I'll bear that in mind.  For the time being, I just want to fix the regression so that this works for an out-of-tree test.

================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:392
@@ +391,3 @@
+                        i != e; i.increment(ec)) {
+    if (error(ec)) break;
+    section_iterator Sec2 = i->getRelocatedSection();
----------------
Rafael Ávila de Espíndola wrote:
> please use llvm naming style, so EC.
> clang-format. In particular, break in another line.
> 
I was copying the style of the adjacent code.  Do you want me to make it inconsistent with the following blocks, or should I send you another change to clean up that code first?

In particular, later code uses the "ec" var.  I could add "EC" alongside it, but that would be weird. :-)  What do you prefer here?


http://llvm-reviews.chandlerc.com/D2559



More information about the llvm-commits mailing list