llvm-objdump bug may hide a single-byte instruction near a label.

[srking]

Summary

An off-by-one bug in the target indepedent llvm-objdump code prevents display of a single byte instruction before a label.

[silvas]

Just a drive-by comment, but you probably need to put the test in the X86 directory if it depends on the x86 disassembler.

[srking]

I also wondered about the best location for the input file. The bug is not specific to x86 and there is precedence for x86 files in the Inputs directory, so I just chose that location. I don't care where the test input ultimately lands, but I'll leave it as-is unless somebody insists to move it.

[• rafael]

tools/llvm-objdump/llvm-objdump.cpp

499

The first line in the comment about the initialization of End. The second line is about the "if (Start == End)".

It would probably be better to split the comment.

[srking]

New patch uploaded:

1. Moved test file to X86 subdirectory
2. Ungrouped code comments.

Thanks reviewers.

[• rafael]

LGTM

[• rafael]

r215837

test/Object/objdump-label.test

+RUN: llvm-objdump -d %p/Inputs/trivial-label-test.elf-x86-64 \
+RUN:              | FileCheck %s -check-prefix ELF-x86-64
+
+ELF-x86-64: file format ELF64-x86-64
+ELF-x86-64: Disassembly of section .text:
+ELF-x86-64: foo:
+ELF-x86-64:        0: 90                                            nop
+ELF-x86-64: bum:
+ELF-x86-64:        1: 90                                            nop
+

tools/llvm-objdump/llvm-objdump.cpp

for (const SectionRef &Section : Obj->sections()) {
     StringRefMemoryObject memoryObject(Bytes, SectionAddr);
     uint64_t Size;
     uint64_t Index;
 
     std::vector<RelocationRef>::const_iterator rel_cur = Rels.begin();
     std::vector<RelocationRef>::const_iterator rel_end = Rels.end();
     // Disassemble symbol by symbol.
     for (unsigned si = 0, se = Symbols.size(); si != se; ++si) {
+
+      // The end is either the section end or the beginning of the next symbol.
+      // If this symbol has the same address as the next symbol, then skip it.
       uint64_t Start = Symbols[si].first;
-      uint64_t End;
-      // The end is either the size of the section or the beginning of the next
-      // symbol.
-      if (si == se - 1)
-        End = SectSize;
-      // Make sure this symbol takes up space.
-      else if (Symbols[si + 1].first != Start)
-        End = Symbols[si + 1].first - 1;
-      else
-        // This symbol has the same address as the next symbol. Skip it.
+      uint64_t End = (si == se - 1) ? SectSize : Symbols[si + 1].first;
+      if (Start == End)
         continue;
 
       outs() << '\n' << Symbols[si].second << ":\n";
 
 #ifndef NDEBUG
       raw_ostream &DebugOut = DebugFlag ? dbgs() : nulls();
 #else
       raw_ostream &DebugOut = nulls();