[PATCH] D39207: [llvm-objcopy] Add support for dwarf fission

Wed Nov 1 03:32:33 PDT 2017

jhenderson added a comment.

In https://reviews.llvm.org/D39207#912415, @jakehehrlich wrote:

> As for using these in conjunction...man I have no clue what I expect out of that. Even if I just think about -split-dwo and -extract-dwo being used in combination I'm a bit confused as to what should happen. GNU objcopy's answer depends on the order that you give the arguments in. I would interpret the result of requesting both be done as a contradiction. e.g. it is demanded that the result of --strip-dwo should not have any .dwo sections. Likewise the result of --extract-dwo should be as small as possible while still maintaining any .dwo sections. I suppose one consistent way of interpreting this is that just the string table should be left. The other way would be to say this is an error. I think throwing errors like that checking for consistent sets of flags is kind of a nightmare and in this case there is a consistent result it's just somewhat unexpected (but so is what GNU objcopy does IMO). Right now as is llvm-objcopy will just give you the string table which seems consistent with "--strip-dwo guarantees no .dwo sections and --extract-dwo will preserve any .dwo sections it can but nothing else". I'm happy with that.

I agree. This sounds like a reasonable plan.

In https://reviews.llvm.org/D39207#912415, @jakehehrlich wrote:

> The other issue is how options in general should effect things like gsplit-dwarf (or now split-dwo). One option is to make no options apply to output to a separate file. This would mean such file outputting options would never compose with other options. In the case of GNU objcopy's --dump-section this is acceptable. The other idea is to make all options apply to them. This has the consequence of being too course and not allowing different options to apply to each file. I also think this could produce some extremely unexpected results now that you point it out. Neither of these seem like good ideas to me. In fact this seems like a general reason not to support these kind of options (options that output object files). Is removing this flag fine with you or do you think one of these options is consistent? GNU objcopy just has --strip-dwo and --extract-dwo. I think that's fine even though it's somewhat non-ideal in the number of invocations the compiler has to make.

I think it would be a shame to drop the extra switch, if we can come up with some sort of sensible scheme to implement it. I think my preference of the two ideas (apply all or no options to the secondary output) is for the options to not be applied to the DWO file. If a user wants to apply additional options to it, they can invoke objcopy again on the file.

However, that all being said, it could be dropped for now, if you prefer, since there isn't a compelling use case, so that the other switches can go in, since those are uncontroversially useful.

================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:71
+    GSplitDwarf("gsplit-dwarf",
+                cl::desc("equivliant to strip-dwo and then only-keep-dwo on "
+                         "the gsplit-dwaf file"));
----------------
jhenderson wrote:
> jakehehrlich wrote:
> > jhenderson wrote:
> > > What is "only-keep-dwo"? 
> > I had the option wrong to begin with and this is a remnant of that mistake. I meant "extract-dwo"
> Looking at the online documentation, gsplit-dwarf is a compiler switch, rather than an objcopy switch. I want to be sure with this switch name that you have chosen it deliberately to match this, because I'd have probably just called it "split-dwarf" or even "split-dwo" (to match the other two options).
> 
> It's also not clear to me what the "gsplit-dwarf file" is. I think you need to reword the help text here.
Still doesn't read right. Maybe "equivalent to -strip-dwo on the input file to the specified file, then -extract-dwo on the input file to the output file". Does the command-line code have the ability to set the display name of the option's argument? If so, it might be nice to use that in the help text instead of "specified file" to make it unambiguous. I don't think there is a "strip-dwo file".

Also, if you keep this option, you should rename the variable to SplitDWO, to match its new name.

================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:107
+  // Construct a second output file for the DWO sections.
+  ELFObject<ELFT> GSplitObj(ObjFile);
+
----------------
If you keep split-dwo, then GSplitObj -> SplitObj, or perhaps more clearly DWOFile.

Repository:
  rL LLVM

https://reviews.llvm.org/D39207