[PATCH] D20466: [MC/ELF] - Fixed incorrect compression.s test

Rafael EspĂ­ndola via llvm-commits llvm-commits at lists.llvm.org
Fri May 20 06:40:16 PDT 2016


I am a bit afraid of this test checking too much and being brittle.

On the other hand, llvm-mc should be pretty stable, so LGTM. We can
check less if turns out to be a problem.

Cheers,
Rafael


On 20 May 2016 at 08:43, George Rimar <grimar at accesssoftek.com> wrote:
>>This does two things:
>>
>>* Change dwarfdump to print a section that is actually compressed, that is good.
>>* Change the CHECK patters to check the full content of the sections.
>>I am not sure why you are doing that.
>>
>>Cheers,
>>Rafael
>
> Since test is about compression, I am pretty sure it is a good idea to check the full content of the sections here
> That potentially can help to catch here:
>
> 1) Algoritm change. If zlib library we are using be changed or becomes corrupted, the check of content would show that instantly.
> That a not great chance for that, but probably we might want to know about that.
> 2) More real case, I guess: currently zlib-gnu style used in this test consists from 'zlib' magic and uncompressed size
> of data in big endian format. So at least we want to check that. But check of first 2 fileds are not always safe.
> What if some code changes on llvm side will corrupt the content in some random way ?
>
> Main what I want to say is since that is special test directly about compression - it would be good I believe to check as much as possible cases,
> that of course would be excessive for others not directly related tests.
>
> George.


More information about the llvm-commits mailing list