[PATCH] D58173: [llvm-objcopy] Add --set-start, --change-start, --adjust-start

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: test/tools/llvm-objcopy/ELF/change-entry-point.test:1
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objcopy --set-start 0x1000 %t %t2
----------------
I'm having trouble reading the below set of tests. Could you split them up with blank lines between the test cases, and also group together the switches so that all --set-start tests are first, then --change-start etc.


================
Comment at: test/tools/llvm-objcopy/ELF/change-entry-point.test:17
+# RUN: llvm-objcopy --change-start -4353 %t %t10
+# RUN: llvm-readobj --file-headers %t10 | FileCheck %s --check-prefix=ADD-UNDERFLOW
+
----------------
I don't think you've responded to this comment?

> Haven't checked GNU objcopy to see if this makes sense, but can you specify -0xabcd? If so, please add a test case for that.


================
Comment at: tools/llvm-objcopy/CopyConfig.cpp:287
+template <class T>
+static T getAsIntegerOrFail(StringRef Val, StringRef Err) {
+  T Result;
----------------
This should return an `Expected<T>` and not report the error in this function.


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

https://reviews.llvm.org/D58173