[PATCH] D47493: [llvm-objdump] Add -R option

[Paul Semel via Phabricator via llvm-commits]

paulsemel added a comment.

Thanks for reviewing :)



================
Comment at: include/llvm/Object/ELFObjectFile.h:795
 uint64_t ELFObjectFile<ELFT>::getRelocationOffset(DataRefImpl Rel) const {
-  assert(EF.getHeader()->e_type == ELF::ET_REL &&
-         "Only relocatable object files have relocation offsets");
----------------
echristo wrote:
> paulsemel wrote:
> > I guess this assert was not here for no reason. But I didn't understand why we were forbidding this function for ELF files other than ET_REL.. Can someone explain me those lines ?
> I think it's that it was looking for a relocatable file and perhaps needs additional checks for any kind of relocatable rather than just relocatable object files.
Yes, there is probably more check needed. At least, at the ELF format file level, there is no difference between ELF file types 


================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:441
   int64_t addend = 0;
+  bool undef = false;
   switch (Sec->sh_type) {
----------------
echristo wrote:
> Can you add some explanatory comments here, it's not necessarily obvious what "undef" means in this context as it can be an overloaded term.
Sure !


Repository:
  rL LLVM

https://reviews.llvm.org/D47493