[PATCH] D128705: [llvm-objdump] Create fake sections for a ELF core file

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.

Looks good, once the remaining nits have been addressed.

Do you need assistance merging this patch into the main branch?



================
Comment at: llvm/test/Object/objdump-no-sectionheaders.test:5-7
+; CHECK: Idx Name          Size     VMA              Type
+; CHECK:   0               000006ec 0000000000400000 TEXT
+; CHECK:   1               00000000 0000000000000000 TEXT
----------------
Use CHECK-NEXT, rather than simply CHECK, since these lines should be in order. Probably still want the CHECK-NOT at the end though, to catch further sections being added.


================
Comment at: llvm/test/tools/llvm-objdump/X86/disassemble-no-section.test:19
+## Check disassembly with an address range.
+
+# RUN: llvm-objdump -d --start-address=0xffffffff00000000 \
----------------
Nit: Since this comment is relevant to the test case immediately following, rather than a batch of test cases, I'd typically not include a blank line between the comment and run lines.


================
Comment at: llvm/test/tools/llvm-objdump/X86/disassemble-no-section.test:18
+
+# RUN: llvm-objdump -d --start-address=0xffffffff00000000 \
+# RUN:   --stop-address=0xffffffff00000004 %t | \
----------------
namhyung wrote:
> MaskRay wrote:
> > There is a warning (may be spurious) but you don't need to fix it in this patch.
> > 
> > Use `2>&1 | ` and test the warning to catch changes when someone fixes it.
> Sure.
I think @MaskRay suggested also checking the contents of the printed out warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128705