[yaml2obj] - Do not skip zeroes blocks if there are relocations against them.

[grimar]

Summary

This is for -D -reloc combination.

Recently I inspected an ELF object and was confused, because of the whole .rodata section
was skipped for me. It contained zero bytes, but I expected to see the relocations there.
I did not instantly realize that that happens because we skip all zero bytes after the rL350823.

In this patch, I suggest to not skip the zero bytes that have a relocation against them when -reloc is used.
If -reloc is not used, then the behavior will be the same.

[grimar]

• Update the comment.

[MaskRay]

This behavior is reasonable, and is what GNU objdump has:

0000000000000000 <.rodata>:
	...
			0: R_X86_64_64	*ABS*
			a: R_X86_64_64	*ABS*

It doesn't use our 8-zeroes heuristic, though.

[grimar]

In D58174#1396199, @MaskRay wrote:

This behavior is reasonable, and is what GNU objdump has:

0000000000000000 <.rodata>:
	...
			0: R_X86_64_64	*ABS*
			a: R_X86_64_64	*ABS*

It doesn't use our 8-zeroes heuristic, though.

FWIW, I did not try to follow the objdump. I think we can do better :)

[jhenderson]

test/tools/llvm-objdump/disasm-zeroes-relocations.test

2 ↗

(On Diff #186637)

Could you also add a case to this test showing that disassembly does skip the zeroes in this case, if the -reloc switch is not used, please?

tools/llvm-objdump/llvm-objdump.cpp

1287–1288 ↗

(On Diff #186637)

I'd rephrase this slightly as "For -reloc: print zero blocks patched by relocations, so that relocations can be shown in the dump."

[grimar]

• Addressed review comments.

[jhenderson]

LGTM.

llvm/trunk/test/tools/llvm-objdump/disasm-zeroes-relocations.test

+# RUN: yaml2obj %s > %t
+
+## Check we do not skip zeroes blocks if have relocations pointed to these places.
+# RUN: llvm-objdump -D -reloc %t
+
+# CHECK:      Disassembly of section .rodata:
+# CHECK-NEXT: 0000000000000000 .rodata:
+# CHECK-NEXT:        0:       00 00   addb    %al, (%rax)
+# CHECK-NEXT:                 0000000000000000:  R_X86_64_64  *ABS*
+# CHECK-NEXT:        2:       00 00   addb    %al, (%rax)
+# CHECK-NEXT:        4:       00 00   addb    %al, (%rax)
+# CHECK-NEXT:        6:       00 00   addb    %al, (%rax)
+# CHECK-NEXT:        8:       00 00   addb    %al, (%rax)
+# CHECK-NEXT:                 0000000000000008:  R_X86_64_64  *ABS*
+# CHECK-NEXT:        a:       00 00   addb    %al, (%rax)
+# CHECK-NEXT:        c:       00 00   addb    %al, (%rax)
+# CHECK-NEXT:        e:       00 00   addb    %al, (%rax)
+
+## Check that without -reloc all zeroes would be omitted.
+# RUN: llvm-objdump -D %t | FileCheck %s --check-prefix=SKIP
+
+# SKIP:      Disassembly of section .rodata:
+# SKIP-NEXT: 0000000000000000 .rodata:
+# SKIP-NEXT:   ...
+# SKIP-NEXT: Disassembly of section .rela.rodata:
+
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .rodata
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    AddressAlign:    0x0000000000000001
+    Content:         '00000000000000000000000000000000'
+  - Name:            .rela.rodata
+    Type:            SHT_RELA
+    Flags:           [ SHF_INFO_LINK ]
+    Link:            .symtab
+    AddressAlign:    0x0000000000000008
+    EntSize:         0x0000000000000018
+    Info:            .rodata
+    Relocations:
+      - Offset:          0x0000000000000000
+        Symbol:          x
+        Type:            R_X86_64_64
+      - Offset:          0x0000000000000008
+        Symbol:          x
+        Type:            R_X86_64_64

llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp

if (auto *ElfObj = dyn_cast<ELFObjectFileBase>(Obj)) {
     }
   }
 }
 
 // Normally the disassembly output will skip blocks of zeroes. This function
 // returns the number of zero bytes that can be skipped when dumping the
 // disassembly of the instructions in Buf.
 static size_t countSkippableZeroBytes(ArrayRef<uint8_t> Buf) {
-  // When -z or --disassemble-zeroes are given we always dissasemble them.
-  if (DisassembleZeroes)
-    return 0;
-
   // Find the number of leading zeroes.
   size_t N = 0;
   while (N < Buf.size() && !Buf[N])
     ++N;
 
   // We may want to skip blocks of zero bytes, but unless we see
   // at least 8 of them in a row.
   if (N < 8)
#endif
               outs() << '\n';
               NumBytes = 0;
             }
           }
         }
         if (Index >= End)
           break;
 
+        // When -z or --disassemble-zeroes are given we always dissasemble them.
+        // Otherwise we might want to skip zero bytes we see.
+        if (!DisassembleZeroes) {
+          uint64_t MaxOffset = End - Index;
+          // For -reloc: print zero blocks patched by relocations, so that
+          // relocations can be shown in the dump.
+          if (RelCur != RelEnd)
+            MaxOffset = RelCur->getOffset() - Index;
+
           if (size_t N =
-                countSkippableZeroBytes(Bytes.slice(Index, End - Index))) {
+                  countSkippableZeroBytes(Bytes.slice(Index, MaxOffset))) {
             outs() << "\t\t..." << '\n';
             Index += N;
             if (Index >= End)
               break;
           }
+        }
 
         // Disassemble a real instruction or a data when disassemble all is
         // provided
         bool Disassembled = DisAsm->getInstruction(Inst, Size, Bytes.slice(Index),
                                                    SectionAddr + Index, DebugOut,
                                                    CommentStream);
         if (Size == 0)
           Size = 1;