[PATCH] D49979: [llvm-objcopy] Add --dump-section

Jake Ehrlich via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 2 11:50:18 PDT 2018


jakehehrlich added a comment.

This LGTM other than one issue

In https://reviews.llvm.org/D49979#1184039, @jhenderson wrote:

> My first instinct on looking at the code as I considered this is that this should be handled in a way similar to how `SplitDWOToFile` works, i.e. to do it at the start of HandleArgs before any manipulation of `Object` takes place. I think this would mean that we wouldn't need any additional methods, because it would happen before any modifications occurred. And we have `ELFSectionWriter` that would achieve what we want, I believe? So just create a specific ELFSectionWriter for the dumping of sections and use that.
>
> I'm not completely opposed to using another method to do it, but it seems like anything else we do would just add additional complexity?


For the record I like this. It runs contrary to the requirement that --dump-section use the original unmodified data however. For instance despite the fact that we try our best (except on section order? I might fix that) to maintain the original file details we can still change things in doing things that way.



================
Comment at: test/tools/llvm-objcopy/dump-section.test:4
 # RUN: llvm-objcopy -O binary -only-keep .text %t %t3
+# RUN: llvm-objcopy --dump-section .text=%t4 %t %t5
 # RUN: od -t x1 %t2 | FileCheck %s
----------------
Can you also dump a non-allocated section with --dump-section to show why its far more useful than the -O binary -j hack?


================
Comment at: tools/llvm-objcopy/Object.cpp:858
+    Sec.OriginalData =
+        ArrayRef<uint8_t>(ElfFile.base() + Shdr.sh_offset, Shdr.sh_size);
   }
----------------
Check for nobits here and use a size of zero if the type is nobits.


Repository:
  rL LLVM

https://reviews.llvm.org/D49979





More information about the llvm-commits mailing list