[PATCH] D63820: [docs][llvm-objcopy] Write documentation for llvm-objcopy

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: docs/CommandGuide/llvm-objcopy.rst:130
+
+.. option:: --only-keep-debug
+
----------------
jakehehrlich wrote:
> This is half fair. Technically it meets the requirements for ELF it just doesn't work optimally. I think adding a caveat section under ELF-specific section might be warranted.
Config.OnlyKeepDebug is not used outside the COFF code, so this really is ignored for ELF as things stand. Given that the output of using this on ELF is not what GNU produces (i.e. it's a no-op), I'm not convinced we should advertise this more widely. We could make similar arguments about various options that are currently ELF-specific, and aren't checked by the COFF code for an error.

I've added a blanket comment to the top of the section saying that llvm-objcopy will either ignore or error for format-specific options (same for ELF).


================
Comment at: docs/CommandGuide/llvm-objcopy.rst:226
+
+ Remove local symbols starting with ".L" from the output.
+
----------------
jakehehrlich wrote:
> Might add "with local binding"
Isn't that what "local symbols" means?


================
Comment at: docs/CommandGuide/llvm-objcopy.rst:394
+ Remove from the output all section headers and all section data not within
+ segments.
+
----------------
jakehehrlich wrote:
> I'd note that many non-llvm tools are incompatible with this option and that it should only be used when a) you need the absolute smallest file possible and b) should never be used on ET_REL binaries.
I've added a more general note that "many tools will not be able to" handle such objects.

I don't see a reason to mention when you should or shouldn't do this beyond that statement because ultimately if a user is using this switch, they will know why they are using it, whether it be for size, obfuscation, or whatever. Telling people that it's always about size is like telling them what their motivation is.

If it's run on an ET_REL binary, then the entire section contents of the file will be removed. I don't think this needs explicitly stating, since that can be inferred from the reference to "not within segments".


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63820