[PATCH] D62253: [llvm-objdump] Dump inline relocations if the relocated section is specified with --section

Wed May 22 08:02:02 PDT 2019

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

================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:932
       V.push_back(R);
-    // Sort relocations by address.
     llvm::sort(V, isRelocAddressLess);
   }
----------------
grimar wrote:
> MaskRay wrote:
> > jhenderson wrote:
> > > grimar wrote:
> > > > This part contains unneseccary variable renamings and also a comment removal.
> > > > I would keep the old style.
> > > I'm actually okay with the RelSec -> Relocated name as it is easier to follow, and don't mind it changing.
> > My first impression of `RelSec` is something similar to `.rel.sec`, so I thought it relocated `Section` which obviously is not the case.. So I still prefer `Relocated`.
> > 
> > The comment is redundant.
> > 
> > Regarding `Sec`, I'm inclined to use `Sec` in case someone asks me to rename `Relocated` to `RelocatedSec`. `RelocatedSection` would be too verbose.
> Ok, at least I would restore the comment. It was an innocent victim in this attack I think.
OK, restores, I probably wouldn't attach that if I ran `git blame` first... BTW, I think `llvm::stable_sort` is probably better.

Because gABI says: "If multiple consecutive relocation records are applied to the same relocation location (r_offset), they are composed instead of being applied independently, as described above."

Though I can't find a case to make this break.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62253