[PATCH] D129728: [llvm-dwp] Add SHF_COMPRESSED support and remove .zdebug support

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 14 18:04:27 PDT 2022


dblaikie added a comment.

In D129728#3653775 <https://reviews.llvm.org/D129728#3653775>, @MaskRay wrote:

> In D129728#3653769 <https://reviews.llvm.org/D129728#3653769>, @dblaikie wrote:
>
>> In D129728#3653748 <https://reviews.llvm.org/D129728#3653748>, @MaskRay wrote:
>>
>>> In D129728#3653746 <https://reviews.llvm.org/D129728#3653746>, @dyung wrote:
>>>
>>>> Your change is causing the test 'LLVM :: tools/llvm-dwp/X86/nocompress.test' to fail due to one of the expected input files being deleted by your change. Perhaps you need to delete or fixup this test as well?
>>>>
>>>> https://lab.llvm.org/buildbot/#/builders/216/builds/7412
>>>
>>> I've removed it in 33aa374e597260dae739bb949892cff31ae31eb9 <https://reviews.llvm.org/rG33aa374e597260dae739bb949892cff31ae31eb9> . Someone feeling a need to have a coverage can add it.
>>
>> Please don't remove test coverage like this - this test covers the error path here: https://github.com/llvm/llvm-project/blob/33aa374e597260dae739bb949892cff31ae31eb9/llvm/lib/DWP/DWP.cpp#L288
>
> It's a fast way to get bots green. It takes time to construct a test.

The usual way to do that if this would take to long is to revert the causal patch while addressing the regression - not lower test coverage permanently.

> In lld/test/ELF I added tests testing `UNSUPPORTED: zlib`.

Sure - that doesn't test the DWP error handling codepath linked above, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129728



More information about the llvm-commits mailing list