[PATCH] D59025: Add --add-ghashes to llvm-objcopy to append a .debug$H to coff objects

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added reviewers: jhenderson, jakehehrlich.
rupprecht added a comment.

In D59025#1420566 <https://reviews.llvm.org/D59025#1420566>, @santagada wrote:

> In D59025#1420056 <https://reviews.llvm.org/D59025#1420056>, @rupprecht wrote:
>
> > In D59025#1420039 <https://reviews.llvm.org/D59025#1420039>, @santagada wrote:
> >
> > > In D59025#1420024 <https://reviews.llvm.org/D59025#1420024>, @rupprecht wrote:
> > >
> > > > General question: why not just use llvm-objcopy --add-section to do this?
> > >
> > >
> > > How? I need to compute the ghashes, how would I just add a section to it?
> >
> >
> > --add-section adds a given file as a section name, e.g. assuming you have a separate script to compute hashes:
> >  ./compute_ghashes input.o > ghash.txt
> >  llvm-objcopy input.o --add-section .debug$H=ghash.txt
>
>
> llvm-objcopy doesn't supoprt --add-section for coff, so this wouldn't be possible.


I can't think of any reason it shouldn't be implemented -- I assume @mstorsjo just hasn't found time (or a volunteer) to implement that yet :)
i.e. for purposes of design, I think we should pretend that --add-section (and in fact all objcopy methods) are implemented for all object formats.

> Even if it was this is a clean way to add a section that is intended to follow the same format that lld expects and that clang generates.

I think that's subjective. It's definitely looks simple here, but it adds complexity to objcopy that I think might not be in the right place. Instead of teaching N llvm tools how to use ghashes, it might be better to just create one, and then tools like objcopy can remain generic.

Anyway, I'm not really that opposed to this change, I just want to figure out what's the threshold for adding flags like this. It's probably totally fine, especially as it seems fairly isolated. I added a few other reviewers who might have an idea. I've never heard of ghashes, so I might not be the best person to comment on this change (other than my existing comments).

Also, if we do add a flag, is it too verbose to call it `--add-codeview-ghash` or something for consistency? I found three references to ghash CLI options:

1. clang uses `--gcodeview-ghash`
2. llvm-readobj uses `--codeview-ghash`
3. lld uses `/debug:ghash`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59025