[PATCH] D78786: [llvm-objcopy] Don't special the all zero p_paddr case

Mon Apr 27 00:29:29 PDT 2020

jhenderson accepted this revision.
jhenderson added a subscriber: jakehehrlich.
jhenderson added a comment.
This revision is now accepted and ready to land.

> I believe this behavior was copied from https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=6ffd79000b45e77b3625143932ffbf781b6aecab (2008-05)

>From reading the original review history etc of the change that implemented this in llvm-objcopy, I couldn't see evidence that this is true, except that I recall @jakehehrlich was trying to copy GNU objcopy's behaviour in all things that made some degree of sense. I believe he did this from a mixture of trial and error and reading objcopy code, but I'm not sure.

Assuming that it was to mirror the GNU behaviour, and the GNU behaviour is as you described (I'm getting timeout errors when I go to that link), I'm fine with this, but I'd like @jakehehrlich to chime in if he's still around. If he doesn't do so in the next few days, this can go in.

================
Comment at: llvm/test/tools/llvm-objcopy/ELF/binary-no-paddr.test:9-10
+## https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=6ffd79000b45e77b3625143932ffbf781b6aecab
+## We don't implement this special rule. The p_vaddr=0 output is the same as
+## the p_vaddr=1 case.
+# RUN: yaml2obj -D PADDR=0 %s -o %t0
----------------
I'm not sure this sentence is right - I assume you mean p_paddr, not p_paddr.

================
Comment at: llvm/test/tools/llvm-objcopy/ELF/binary-no-paddr.test:13-14
+# RUN: llvm-objcopy -O binary %t0 %t0.out
+# RUN: od -t x2 -v %t0.out | FileCheck %s --ignore-case
+# RUN: wc -c < %t0.out | FileCheck %s --check-prefix=SIZE

----------------
If the two outputs should be the same, would simply `cmp %t1.out %t0.out1` be simpler?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78786