[PATCH] D86539: [Debuginfo][llvm-dwarfutil] llvm-dwarfutil dsymutil-like tool for ELF.

[Alexey Lapshin via Phabricator via llvm-commits]

avl added a comment.

In D86539#3434567 <https://reviews.llvm.org/D86539#3434567>, @clayborg wrote:

> In D86539#3434454 <https://reviews.llvm.org/D86539#3434454>, @avl wrote:
>
>> In D86539#3434419 <https://reviews.llvm.org/D86539#3434419>, @clayborg wrote:
>>
>>> Right now if any DWARF section isn't supported, or if a DW_FORM is not supported, this patch will still emit a file and return a non-zero exit status.
>>>
>>> So if this patch isn't going to support any of the DWARF sections that are required, it must exit with a valid error message and return a non zero exit status. The things that come to mind are:
>>>
>>> - anything with an older .debug_types section should return an error stating that debug types support is not supported
>>> - any newer DWARF with type units in the .debug_info section should return an error stating that debug types support is not supported
>>> - If fission is not supported, we need to return an error if we find any fission related sections (".debug_addr", etc)
>>> - if any DWARF section is required for debugging (like ".debug_loclists" or ".debug_rnglists") and not getting re-linked by this tool and if those sections become invalid after the DWARF is changed, then we need to return an error
>>> - if the line tables are too new and the DWARF linker can't update them return an error
>>>
>>> All of this stems from Apple not having to support anything but what the Apple compilers emit. Many compilers that emit ELF files and linkers that link ELF have many other options that can be enabled for DWARF. If we aren't going to support them, we can't just emit some warnings and hope the user knows better, we need to return an error and set the exit status to a non zero value. We will need tests to cover these errors as well.
>>
>> My intention was to limit first version to functionality presented in current dsymutil/DWARFLinker. And to add improvements incrementally.  But If we want to have that extended check in the first version then I will add it.
>>
>> My own preference is to integrate that first version of the tool and add improvements incrementally. f.e. adding such extended check would require changing interface of DWARFLinker. Use "Error DWARFLinker::link()" instead of "bool DWARFLinker::link()". It would be better to make such change separately.
>
> A little background: I am very interested in this patch and I have been trying to get statistics with the resulting binary from this patch so I can evaluate if we want to use it in production on linked binaries produced by our build system. I'd like to see how long it takes to run and also how much smaller debug info becomes. If I run this currently I have no way to know if we are producing anything valid and wether or not I can log the results since we always produce something and have no exit status. I also can't trust the resulting debug info sizes at all since we might be removing all sorts of extra DWARF sections that the existing DWARF linker doesn't handle, or worse yet we just don't clone an attribute and we are missing inline debug info. So it is hard to evaluate this right now as it is. I do think we should try to return errors for files we can't handle though as it would be great if people trying this out could evaluate it and know if it is working for them or if there are things that aren't handled.

@clayborg The D123623 <https://reviews.llvm.org/D123623>  implements displaying error messages for unsupported tables. Would you mind to check it, please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86539