[DebugInfo] Add check for zero debug line opcode_base

[jhenderson]

Summary

The number of standard opcodes is defined to be opcode_base - 1, so a value of 0 for the opcode_base caused a crash as an attempt was made to reserve many entries in a vector. This change fixes the crash, by issuing a warning and skipping reading of standard opcode lengths in the event of an opcode_base of 0.

[dblaikie]

Please add an assembly+dumper test or a unit test, not both, in this instance (& usually preferred in general - there are some cases where it's worthwhile having both).

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

358–367

I don't /think/ this merits a warning. It seems to be unexceptionally DWARF conformant to have an opcode base of 0:

"Opcode base is typically one greater than the highest-numbered standard opcode defined for the specified version of the line number information (12 in DWARF Versions 3, 4 and 5, and 9 in Version 2). If opcode_base is less than the typical value, then standard opcode numbers greater than or equal to the opcode base are not used in the line number table of this unit (and the codes are treated as special opcodes)."

llvm/test/tools/llvm-dwarfdump/X86/Inputs/debug_line_malformed.s

398–411

Since this is only testing opcode base - perhaps it could have fewer/shorter details here? One directory and one file, perhaps? (I guess you've added/plan to add sufficient validation that it wouldn't be acceptable (would produce other warnings that would complicate the test case) to have zero files/directories and a line table program that's only a DW_LNE_end_sequence?)

[probinson]

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

358–367

That non-normative comment seems to have a reasonable interpretation for an opcode base of 0, but then the normative definition of standard_opcode_lengths in the next item isn't really written in a way that supports an opcode base of less than 2.

Opcode base is really "the first special opcode" and zero can't be a special opcode because zero introduces an extended opcode. So I agree with James, it is reasonable to have this warning.

[dblaikie]

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

358–367

Ah, sorry - I think I meant to delete this comment when I read further.

I think it supports the opcode base of 1, maybe? (at least there's no interpretation I can find that would differentiate the behavior of opcode base 1 or 0, at least? Which to me, means it makes sense that the reasonable thing to do is to use 1 and never use zero from a normalization perspective - and that zero is weird enough that warning seems OK to me)

[probinson]

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

358–367

That's reasonable, to accept 1 (which implies the standard_opcode_lengths array has zero elements) but warn on 0.

[jhenderson]

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

358–367

Yup, that all matches my understanding as I read the spec. An opcode_base of 0 would imply "-1" standard opcodes, and a first special opcode value of 0 (which would clash with the definition of an extended opcode). so, 1 is fine, but 0 isn't.

llvm/test/tools/llvm-dwarfdump/X86/Inputs/debug_line_malformed.s

398–411

I'd like to keep at least one directory and file table, as it shows what happens with the prologue parsing after the bad opcode_base (i.e. it keeps going, assuming no opcode lengths). There's probably no need for more than one of each though. Also, I've deliberately got a special opcode, because a) the opcode_base is used in the calculation this triggers, and b) 0x1 would normally be a standard opcode.

[jhenderson]

Rebase, remove unit test and remove redundant parts of lit test.

[dblaikie]

Looks good, thanks!

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

Error DWARFDebugLine::Prologue::parse(
   MinInstLength = DebugLineData.getU8(OffsetPtr);
   if (getVersion() >= 4)
     MaxOpsPerInst = DebugLineData.getU8(OffsetPtr);
   DefaultIsStmt = DebugLineData.getU8(OffsetPtr);
   LineBase = DebugLineData.getU8(OffsetPtr);
   LineRange = DebugLineData.getU8(OffsetPtr);
   OpcodeBase = DebugLineData.getU8(OffsetPtr);
 
+  if (OpcodeBase == 0) {
+    // If the opcode base is 0, we cannot read the standard opcode lengths (of
+    // which there are supposed to be one fewer than the opcode base). Assume
+    // there are no standard opcodes and continue parsing.
+    RecoverableErrorCallback(createStringError(
+        errc::invalid_argument,
+        "parsing line table prologue at offset 0x%8.8" PRIx64
+        " found opcode base of 0. Assuming no standard opcodes",
+        PrologueOffset));
+  } else {
     StandardOpcodeLengths.reserve(OpcodeBase - 1);
     for (uint32_t I = 1; I < OpcodeBase; ++I) {
       uint8_t OpLen = DebugLineData.getU8(OffsetPtr);
       StandardOpcodeLengths.push_back(OpLen);
     }
+  }
 
   if (getVersion() >= 5) {
     if (Error E =
             parseV5DirFileTables(DebugLineData, OffsetPtr, FormParams, Ctx, U,
                                  ContentTypes, IncludeDirectories, FileNames)) {
       RecoverableErrorCallback(joinErrors(
           createStringError(
               errc::invalid_argument,

llvm/test/tools/llvm-dwarfdump/X86/Inputs/debug_line_malformed.s

 .asciz  "xyz"           # File name                       | 3 special opcodes + DW_LNE_set_address start
 .byte   9               # MD5 hash value                  | DW_LNE_set_address length
 # Header end
 .byte   2               # DW_LNE_set_address opcode
 .quad   0x4321432143214321
 .byte   0, 1, 1         # DW_LNE_end_sequence
 .Linvalid_md5_end1:
 
+# Zero opcode base.
+.long   .Lzero_opcode_base_end - .Lzero_opcode_base_start # unit length
+.Lzero_opcode_base_start:
+.short  4               # version
+.long   .Lzero_opcode_base_prologue_end-.Lzero_opcode_base_prologue_start # Length of Prologue
+.Lzero_opcode_base_prologue_start:
+.byte   1               # Minimum Instruction Length
+.byte   1               # Maximum Operations per Instruction
+.byte   1               # Default is_stmt
+.byte   0               # Line Base
+.byte   1               # Line Range
+.byte   0               # Opcode Base
+.asciz "dir1"           # Include table
+.asciz "dir2"
+.byte   0
+.asciz "file1"          # File table
+.byte   0, 0, 0
+.asciz "file2"
+.byte   1, 2, 3
+.byte   0
+.Lzero_opcode_base_prologue_end:
+.byte   0, 9, 2        # DW_LNE_set_address
+.quad   0xffffeeeeddddcccc
+.byte   0x1            # Special opcode
+.byte   0, 1, 1        # DW_LNE_end_sequence
+.Lzero_opcode_base_end:
+
 # Trailing good section.
 .long   .Lunit_good_end - .Lunit_good_start # Length of Unit (DWARF-32 format)
 .Lunit_good_start:
 .short  4               # DWARF version number
 .long   .Lprologue_good_end-.Lprologue_good_start # Length of Prologue
 .Lprologue_good_start:
 .byte   1               # Minimum Instruction Length
 .byte   1               # Maximum Operations per Instruction

llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test

 # RUN: FileCheck %s --input-file=%t-malformed-verbose.err --check-prefixes=ALL,OTHER
 
 ## We should still produce warnings for malformed tables after the specified unit.
 # RUN: llvm-dwarfdump -debug-line=0 %t-malformed.o 2> %t-malformed-off-first.err \
 # RUN:   | FileCheck %s --check-prefixes=FIRST,NOLATER
 # RUN: FileCheck %s --input-file=%t-malformed-off-first.err --check-prefix=ALL
 
 ## Don't stop looking for the later unit if non-fatal issues are found.
-# RUN: llvm-dwarfdump -debug-line=0x2ec %t-malformed.o 2> %t-malformed-off-last.err \
+# RUN: llvm-dwarfdump -debug-line=0x329 %t-malformed.o 2> %t-malformed-off-last.err \
 # RUN:   | FileCheck %s --check-prefix=LAST --implicit-check-not='debug_line[{{.*}}]'
 # RUN: FileCheck %s --input-file=%t-malformed-off-last.err --check-prefix=ALL
 
 # FIRST:       debug_line[0x00000000]
 # FIRST:       0x000000000badbeef {{.*}} end_sequence
 # NOFIRST-NOT: debug_line[0x00000000]
 # NOFIRST-NOT: 0x000000000badbeef {{.*}} end_sequence
 # NOLATER-NOT: debug_line[{{.*}}]

 ## been read before the MD5 problem is identified.
 # NONFATAL: debug_line[0x000002ae]
 # NONFATAL-NEXT: Line table prologue
 # NONFATAL:      include_directories[  0] = "/tmp"
 # NONFATAL-NOT:  file_names
 # NONFATAL:      0x0000000000000000 {{.*}} epilogue_begin
 # NONFATAL:      0x4321432143214321 {{.*}} is_stmt end_sequence
 
-# LAST:          debug_line[0x000002ec]
+## Case 13: Opcode base field of value zero.
+# NONFATAL:      debug_line[0x000002ec]
+# NONFATAL-NEXT: Line table prologue
+# NONFATAL:      include_directories[  1] = "dir1"
+# NONFATAL:      include_directories[  2] = "dir2"
+# NONFATAL:      file_names[  1]:
+# NONFATAL-NEXT:            name: "file1"
+# NONFATAL-NEXT:       dir_index: 0
+# NONFATAL-NEXT:        mod_time: 0x00000000
+# NONFATAL-NEXT:          length: 0x00000000
+# NONFATAL-NEXT: file_names[  2]:
+# NONFATAL-NEXT:            name: "file2"
+# NONFATAL-NEXT:       dir_index: 1
+# NONFATAL-NEXT:        mod_time: 0x00000002
+# NONFATAL-NEXT:          length: 0x00000003
+# NONFATAL:      0xffffeeeeddddcccd 1 0 1 0 0 is_stmt{{$}}
+# NONFATAL:      0xffffeeeeddddcccd 1 0 1 0 0 is_stmt end_sequence{{$}}
+
+# LAST:          debug_line[0x00000329]
 # LAST:          0x00000000cafebabe {{.*}} end_sequence
 
 # RESERVED: warning: parsing line table prologue at offset 0x00000048 unsupported reserved unit length found of value 0xfffffffe
 
 # ALL-NOT:  warning:
 # ALL:      warning: parsing line table prologue at offset 0x00000048 found unsupported version 0x00
 # ALL-NEXT: warning: parsing line table prologue at offset 0x0000004e found unsupported version 0x01
 # ALL-NEXT: warning: parsing line table prologue at 0x00000054 found an invalid directory or file table description at 0x00000073
 # ALL-NEXT: warning: failed to parse entry content descriptions because no path was found
 # ALL-NEXT: warning: parsing line table prologue at 0x00000081 should have ended at 0x000000b9 but it ended at 0x000000ba
 # ALL-NEXT: warning: parsing line table prologue at 0x000000c8 should have ended at 0x00000103 but it ended at 0x00000102
 # OTHER-NEXT: warning: unexpected line op length at offset 0x00000158 expected 0x02 found 0x01
 # OTHER-NEXT: warning: unexpected line op length at offset 0x0000015c expected 0x01 found 0x02
 # OTHER-NEXT: warning: last sequence in debug line table at offset 0x0000016c is not terminated
 # ALL-NEXT: warning: parsing line table prologue at 0x000001b2 should have ended at 0x000001ce but it ended at 0x000001e1
 # ALL-NEXT: warning: parsing line table prologue at 0x000001ee should have ended at 0x00000219 but it ended at 0x00000220
 # ALL-NEXT: warning: parsing line table prologue at 0x0000022f should have ended at 0x00000251 but it ended at 0x0000025e
 # ALL-NEXT: warning: parsing line table prologue at 0x0000026b found an invalid directory or file table description at 0x0000029f
 # ALL-NEXT: warning: failed to parse file entry because the MD5 hash is invalid
 # ALL-NEXT: warning: parsing line table prologue at 0x000002ae found an invalid directory or file table description at 0x000002e0
 # ALL-NEXT: warning: failed to parse file entry because the MD5 hash is invalid
 # ALL-NEXT: warning: parsing line table prologue at 0x000002ae should have ended at 0x000002d9 but it ended at 0x000002e0
+# ALL-NEXT: warning: parsing line table prologue at offset 0x000002ec found opcode base of 0. Assuming no standard opcodes
 # ALL-NOT:  warning:

llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

TEST_F(DebugLineBasicFixture, ErrorForLowVersion) {
   generate();
 
   checkGetOrParseLineTableEmitsFatalError(
       "parsing line table prologue at offset "
       "0x00000000 found unsupported version "
       "0x01");
 }
 
+TEST_F(DebugLineBasicFixture, ErrorAndContinueForZeroOpcodeBase) {
+  if (!setupGenerator())
+    return;
+
+  LineTable &Padding = Gen->addLineTable();
+  Padding.setCustomPrologue({{0, LineTable::Byte}});
+  LineTable &LT = Gen->addLineTable();
+  DWARFDebugLine::Prologue Prologue = LT.createBasicPrologue();
+  // Save default state to allow for using checkDefaultPrologue later.
+  uint64_t DefaultOpcodeBase = Prologue.OpcodeBase;
+  auto DefaultLengths = Prologue.StandardOpcodeLengths;
+  Prologue.OpcodeBase = 0;
+  Prologue.StandardOpcodeLengths.clear();
+  Prologue.TotalLength -= DefaultLengths.size();
+  Prologue.PrologueLength -= DefaultLengths.size();
+  LT.setPrologue(Prologue);
+
+  generate();
+
+  DWARFDebugLine::Prologue Result;
+  uint64_t Offset = 1;
+  Error Err = Result.parse(LineData, &Offset, RecordRecoverable, *Context);
+  EXPECT_THAT_ERROR(std::move(Err), Succeeded());
+  checkError("parsing line table prologue at offset 0x00000001 found opcode "
+             "base of 0. Assuming no standard opcodes",
+             std::move(Recoverable));
+  EXPECT_EQ(0, Result.OpcodeBase);
+  EXPECT_TRUE(Result.StandardOpcodeLengths.empty());
+
+  Result.OpcodeBase = DefaultOpcodeBase;
+  Result.StandardOpcodeLengths = DefaultLengths;
+  Result.TotalLength += DefaultLengths.size();
+  Result.PrologueLength += DefaultLengths.size();
+  checkDefaultPrologue(Result.getVersion(), DWARF32, Result, 0);
+}
+
 TEST_F(DebugLineBasicFixture, ErrorForInvalidV5IncludeDirTable) {
   if (!setupGenerator(5))
     return;
 
   LineTable &LT = Gen->addLineTable();
   LT.setCustomPrologue({
       {19, LineTable::Long}, // unit length
       {5, LineTable::Half},  // version