[MC/ELF] - Fixed insufficient compression.s test

[grimar]

Summary

Main problem that .debug_info
section was used to check that llvm-dwarfdump is able to decompress
data that was compressed with llvm-mc tool. This section was not compressed
actually, because consumes more space in compressed view.

I changed testcase to use .debug_str section which is one that
is really compressed. So currently test do what is probably was expected to do:
checks that "data"->llvm-mc->llvm-dwarfdump->dumps back initial "data".

[grimar]

This seems to be the reason of why D20331 testcase passed fine, when it looks should have
been broken because of llvm-dwarfdump I think does not support "zlib" style format yet.
So I plan to fix that tool (and looks some others one too), will investigate how that should be done properly and where.
I guess LLVMObject is the place to look.

[• rafael]

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

[dblaikie]

compression.s

14–17 ↗

(On Diff #57920)

Testing the contents like this seems not good. Could you explain more why/how the original test was insufficient?

(I see the roundtrip example was insufficient, since debug_info wasn't compressed - so for sure we should switch that to a section that is compressed, like _str, which is good (we should make sure we test that same section for compression - so maybe switch the zdebug_line to test zdebug_str (or change the roundtrip test to test zdebug_line rather than info (rather than changing it to str)))

[grimar]

compression.s

14–17 ↗

(On Diff #57920)

Testing the contents like this seems not good. Could you explain more why/how the original test was insufficient?

Generally I would agree that too much content checking is not a good way, but for some tests it is very preferrable I believe. Like for this one.
Since this test is the main our test about compression, I think we want to test all possible logic involved and as much
possible problems as possible to imagine relative to compression.

Initial test was insufficient generally IMO:

1. For zlib-gnu style it should test that 'ZLIB' magic is present and was put at the start, that was not done actually, it just checked that 'ZLIB' was somewhere there, but in case if 'ZLIB' and for example next 4 bytes were swapped, it would not catch that.
2. Next thing it should check that big-endian size of uncompressed data goes right after that. Probably there is no really good way to check that, but my way defenitely do that.
3. I also can imaging case if some new code can corrupt the compressed data, or new zlib library we using can be buggy or something, I guess we would like to notice such little chanced but still possible problems.

(we should make sure we test that same section for compression - so maybe switch the zdebug_line to test zdebug_str (or change the roundtrip test to test zdebug_line rather than info (rather than changing it to str)))

With above changes, full compression-decompression circle is tested atm.

[grimar]

David, so are you ok with commiting that or .. ?

[grimar]

• Addressed review comments. Patch only perform change to check comression roundtrip and nothing more (+few comments).

[• rafael]

LGTM, please commit.

[grimar]

In D20466#437716, @rafael wrote:

LGTM, please commit.

Thanks !
r270560.

llvm/trunk/test/MC/ELF/compression.s

 // RUN: llvm-mc -filetype=obj -compress-debug-sections -triple x86_64-pc-linux-gnu < %s -o %t
 // RUN: llvm-objdump -s %t | FileCheck %s
-// RUN: llvm-dwarfdump -debug-dump=info %t | FileCheck --check-prefix=INFO %s
+// RUN: llvm-dwarfdump -debug-dump=str %t | FileCheck --check-prefix=STR %s
 // RUN: llvm-mc -filetype=obj -compress-debug-sections -triple i386-pc-linux-gnu < %s \
 // RUN:     | llvm-readobj -symbols - | FileCheck --check-prefix=386-SYMBOLS %s
 
 // REQUIRES: zlib
 
-// CHECK: Contents of section .zdebug_line:
-// Check for the 'ZLIB' file magic at the start of the section only
-// CHECK-NEXT: ZLIB
-// CHECK-NOT: ZLIB
-
 // Don't compress small sections, such as this simple debug_abbrev example
 // CHECK: Contents of section .debug_abbrev:
 // CHECK-NOT: ZLIB
 // CHECK-NOT: Contents of
 
 // CHECK: Contents of section .debug_info:
 
+// CHECK: Contents of section .zdebug_str:
+// Check for the 'ZLIB' file magic at the start of the section only
+// CHECK-NEXT: ZLIB
+// CHECK-NOT: ZLIB
+
 // FIXME: Handle compressing alignment fragments to support compressing debug_frame
 // CHECK: Contents of section .debug_frame:
 // CHECK-NOT: ZLIB
 // CHECK: Contents of
 
-// Decompress one valid dwarf section just to check that this roundtrips
-// INFO: 0x00000000: Compile Unit: length = 0x0000000c version = 0x0004 abbr_offset = 0x0000 addr_size = 0x08 (next unit at 0x00000010)
+// Decompress one valid dwarf section just to check that this roundtrips,
+// we use .zdebug_str section for that
+// STR: perfectly compressable data sample *****************************************
 
 // In x86 32 bit named symbols are used for temporary symbols in merge
 // sections, so make sure we handle symbols inside compressed sections
 // 386-SYMBOLS: Name: .Linfo_string0
 // 386-SYMBOLS-NOT: }
 // 386-SYMBOLS: Section: .zdebug_str
 
   .section  .debug_line,"",@progbits
.loc 1 6 0
         nop
   .loc 1 7 0
         nop
   .loc 1 8 0
         nop
   .cfi_endproc
   .cfi_sections .debug_frame
 
+# Below is the section we will use to check that after compression with llvm-mc,
+# llvm-dwarfdump tool will be able to decompress data back and dump it. Data sample
+# should be compressable enough, so it is filled with some amount of equal symbols at the end
   .section        .debug_str,"MS",@progbits,1
 .Linfo_string0:
-        .asciz  "compress this                                    "
+        .asciz  "perfectly compressable data sample *****************************************"