[DebugInfo] Report unsupported maximum_operations_per_instruction values

[jhenderson]

Summary

This patch adds a check which reports an unsupported value of the maximum_operations_per_instruction field in a debug line table header. This is reported once per line table, at most, and only if the table would otherwise need to use it (i.e. never for tables with version 3 or less, or for tables which don't use DW_LNS_const_add_pc or special opcodes). Unsupported values are currently any apart from 1.

See also D43470, which takes a similar approach for bad line_range values (i.e. 0). I may refactor the pair of them to avoid the code duplication.

Depends on D75188.

[jhenderson]

There are a couple of issues in this change, and I also want to refactor it and D43470 to share some code.

[probinson]

llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

9

Should not have moved this; it's not the header for this module.

[jhenderson]

llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

9

Weird. I don't know how that happened. I certainly wouldn't have deliberately moved it.

[jhenderson]

Rewrite to allow for other similar checks to be added in subsequent changes. See D43470 and another patch TBD. See the full patch series for why the test unit test is written the way it is (the other two patches reuse the fixture).

[ikudrin]

llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

757

clang-9 shows a warning here:

DWARFDebugLineTest.cpp:752:8: warning: '(anonymous namespace)::AdjustAddressFixtureBase' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor]

933

Yet another warning:

DWARFDebugLineTest.cpp:928:8: warning: 'SetUp' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]

The similar warnings are also shown for SetUp methods in the dependent patches.

[jhenderson]

llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

933

Does this SetUp warning also apply for the DebugLineParameterisedFixture or DebugLineUnsupportedVersionFixture classes? They aren't new, but are missing overrides on their SetUp functions.

[ikudrin]

llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

933

Nope. The warning appears if some methods in a class are missing override.

[jhenderson]

Rebase and move some of the parameters to the ParsingState class to reduce the number of arguments required in the functions. Also fix warnings mentioned by @ikudrin.

[probinson]

DWARFDebugLine.h has a pre-merge clang-format note, did you miss that?

[jhenderson]

In D74819#1903147, @probinson wrote:

DWARFDebugLine.h has a pre-merge clang-format note, did you miss that?

Oops, my bad (a result of a bad rebase/merge that was). I'll fix it.

[jhenderson]

Fix formatting.

[probinson]

llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLine.h

374

As a future API simplification, think about using dwarf::LNStandardName() to compute the names as needed, instead of passing around strings. LNStandardName() would need an opcode_base parameter to know which ones are special for a particular line-number program, but it's used in only one place that I can find, so that should be an easy update.
I know there's the weird case of DW_LNS_const_add_pc where you're basing the advance on 255 but want to use the real opcode for reporting; but the weird cases are why we pay you the big bucks.

llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

9

Did it move again? If you ran clang-format, I wonder if there's a bug in it (it might think it's moving the "module's" header to the top, but have incorrect module-header detection).

[jhenderson]

Fix formatting again. I blame Visual Studio...!

[jhenderson]

llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLine.h

374

I had a play around with this, and LNStandardName isnt't suitable for passing in opcode_base with directly, as it needs to share the interface with the equivalent functions for other DWARF enum types. I can workaround this with a little extra logic though, I think.

llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

9

Weird...

[jhenderson]

Redo name handling to remove the need for specifying it explicitly.

[probinson]

Thanks for simplifying the API. One more bit of code tidying and I think I will be happy.

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

581

Sink this under the if as you don't need the name unless you're reporting the warning.

595

After sinking the getOpcodeName call, you have a block under the if instead of just a single statement, in which case you might as well move ReportAdvanceAddrProblem = false into that block as well.
These moves help distinguish the error-reporting stuff from the functional stuff as well.

[jhenderson]

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

581

It's used in the second error added in D75189, so I don't think sinking it inside the if makes much sense.

595

Same response as above. Given the additional error message in D75189, I can't put it inside the if.

[jhenderson]

Fix formatting.

[probinson]

LGTM

[MaskRay]

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

511

struct can be omitted (and conventional in C++).

llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

933

public can be omitted for struct.

[jhenderson]

Get rid of some unnecessary public usage.

[jhenderson]

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

511

I took a look at this, but LineTable is both a member name and struct name here. We'd have to explicitly qualify the type name to be DWARFDebugLine::LineTable. There are several similar maybe unnecessary struct specifiers in the header too, but these are all because various members have the same name as their type. I'd recommend leaving this until the variable case changes are made in this code, after which there should be no ambiguity.

llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLine.h

private:
     DWARFDataExtractor &DebugLineData;
     const DWARFContext &Context;
     uint64_t Offset = 0;
     bool Done = false;
   };
 
 private:
   struct ParsingState {
-    ParsingState(struct LineTable *LT);
+    ParsingState(struct LineTable *LT, uint64_t TableOffset,
+                 function_ref<void(Error)> ErrorHandler);
 
     void resetRowAndSequence();
     void appendRowToMatrix();
 
     /// Advance the address by the \p OperationAdvance value. \returns the
     /// amount advanced by.
-    uint64_t advanceAddr(uint64_t OperationAdvance);
+    uint64_t advanceAddr(uint64_t OperationAdvance, uint8_t Opcode,
+                         uint64_t OpcodeOffset);
 
     struct AddrAndAdjustedOpcode {
       uint64_t AddrDelta;
       uint8_t AdjustedOpcode;
     };
 
     /// Advance the address as required by the specified \p Opcode.
     /// \returns the amount advanced by and the calculated adjusted opcode.
-    AddrAndAdjustedOpcode advanceAddrForOpcode(uint8_t Opcode);
+    AddrAndAdjustedOpcode advanceAddrForOpcode(uint8_t Opcode,
+                                               uint64_t OpcodeOffset);
 
     struct AddrAndLineDelta {
       uint64_t Address;
       int32_t Line;
     };
 
     /// Advance the line and address as required by the specified special \p
     /// Opcode. \returns the address and line delta.
-    AddrAndLineDelta handleSpecialOpcode(uint8_t Opcode);
+    AddrAndLineDelta handleSpecialOpcode(uint8_t Opcode, uint64_t OpcodeOffset);
 
     /// Line table we're currently parsing.
     struct LineTable *LineTable;
     struct Row Row;
     struct Sequence Sequence;
+
+  private:
+    uint64_t LineTableOffset;
+
+    bool ReportAdvanceAddrProblem = true;
+    function_ref<void(Error)> ErrorHandler;
   };
 
   using LineTableMapTy = std::map<uint64_t, LineTable>;
   using LineTableIter = LineTableMapTy::iterator;
   using LineTableConstIter = LineTableMapTy::const_iterator;
 
   LineTableMapTy LineTableMap;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_DEBUGINFO_DWARFDEBUGLINE_H

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

 }
 
 void DWARFDebugLine::LineTable::clear() {
   Prologue.clear();
   Rows.clear();
   Sequences.clear();
 }
 
-DWARFDebugLine::ParsingState::ParsingState(struct LineTable *LT)
-    : LineTable(LT) {
+DWARFDebugLine::ParsingState::ParsingState(
+    struct LineTable *LT, uint64_t TableOffset,
+    function_ref<void(Error)> ErrorHandler)
+    : LineTable(LT), LineTableOffset(TableOffset), ErrorHandler(ErrorHandler) {
   resetRowAndSequence();
 }
 
 void DWARFDebugLine::ParsingState::resetRowAndSequence() {
   Row.reset(LineTable->Prologue.DefaultIsStmt);
   Sequence.reset();
 }
 
if (Pos.second) {
     if (Error Err =
             LT->parse(DebugLineData, &Offset, Ctx, U, RecoverableErrorHandler))
       return std::move(Err);
     return LT;
   }
   return LT;
 }
 
-uint64_t DWARFDebugLine::ParsingState::advanceAddr(uint64_t OperationAdvance) {
+static StringRef getOpcodeName(uint8_t Opcode, uint8_t OpcodeBase) {
+  assert(Opcode != 0);
+  if (Opcode < OpcodeBase)
+    return LNStandardString(Opcode);
+  return "special";
+}
+
+uint64_t DWARFDebugLine::ParsingState::advanceAddr(uint64_t OperationAdvance,
+                                                   uint8_t Opcode,
+                                                   uint64_t OpcodeOffset) {
+  StringRef OpcodeName = getOpcodeName(Opcode, LineTable->Prologue.OpcodeBase);
+  // For versions less than 4, the MaxOpsPerInst member is set to 0, as the
+  // maximum_operations_per_instruction field wasn't introduced until DWARFv4.
+  // Don't warn about bad values in this situation.
+  if (ReportAdvanceAddrProblem && LineTable->Prologue.getVersion() >= 4 &&
+      LineTable->Prologue.MaxOpsPerInst != 1)
+    ErrorHandler(createStringError(
+        errc::not_supported,
+        "line table program at offset 0x%8.8" PRIx64
+        " contains a %s opcode at offset 0x%8.8" PRIx64
+        ", but the prologue maximum_operations_per_instruction value is %" PRId8
+        ", which is unsupported. Assuming a value of 1 instead",
+        LineTableOffset, OpcodeName.data(), OpcodeOffset,
+        LineTable->Prologue.MaxOpsPerInst));
+  ReportAdvanceAddrProblem = false;
   uint64_t AddrOffset = OperationAdvance * LineTable->Prologue.MinInstLength;
   Row.Address.Address += AddrOffset;
   return AddrOffset;
 }
 
 DWARFDebugLine::ParsingState::AddrAndAdjustedOpcode
-DWARFDebugLine::ParsingState::advanceAddrForOpcode(uint8_t Opcode) {
+DWARFDebugLine::ParsingState::advanceAddrForOpcode(uint8_t Opcode,
+                                                   uint64_t OpcodeOffset) {
   assert(Opcode == DW_LNS_const_add_pc ||
          Opcode >= LineTable->Prologue.OpcodeBase);
   uint8_t OpcodeValue = Opcode;
   if (Opcode == DW_LNS_const_add_pc)
     OpcodeValue = 255;
   uint8_t AdjustedOpcode = OpcodeValue - LineTable->Prologue.OpcodeBase;
   uint64_t OperationAdvance = AdjustedOpcode / LineTable->Prologue.LineRange;
-  uint64_t AddrOffset = advanceAddr(OperationAdvance);
+  uint64_t AddrOffset = advanceAddr(OperationAdvance, Opcode, OpcodeOffset);
   return {AddrOffset, AdjustedOpcode};
 }
 
 DWARFDebugLine::ParsingState::AddrAndLineDelta
-DWARFDebugLine::ParsingState::handleSpecialOpcode(uint8_t Opcode) {
+DWARFDebugLine::ParsingState::handleSpecialOpcode(uint8_t Opcode,
+                                                  uint64_t OpcodeOffset) {
   // A special opcode value is chosen based on the amount that needs
   // to be added to the line and address registers. The maximum line
   // increment for a special opcode is the value of the line_base
   // field in the header, plus the value of the line_range field,
   // minus 1 (line base + line range - 1). If the desired line
   // increment is greater than the maximum line increment, a standard
   // opcode must be used instead of a special opcode. The "address
   // advance" is calculated by dividing the desired address increment
DWARFDebugLine::ParsingState::handleSpecialOpcode(uint8_t Opcode,
   //                      minimum_instruction_length
   //
   // The amount to increment the line register is the line_base plus
   // the result of the adjusted opcode modulo the line_range. That is:
   //
   // line increment = line_base + (adjusted opcode % line_range)
 
   DWARFDebugLine::ParsingState::AddrAndAdjustedOpcode AddrAdvanceResult =
-      advanceAddrForOpcode(Opcode);
+      advanceAddrForOpcode(Opcode, OpcodeOffset);
   int32_t LineOffset =
       LineTable->Prologue.LineBase +
       (AddrAdvanceResult.AdjustedOpcode % LineTable->Prologue.LineRange);
   Row.Line += LineOffset;
   return {AddrAdvanceResult.AddrDelta, LineOffset};
 }
 
 Error DWARFDebugLine::LineTable::parse(
Error DWARFDebugLine::LineTable::parse(
 
   // See if we should tell the data extractor the address size.
   if (DebugLineData.getAddressSize() == 0)
     DebugLineData.setAddressSize(Prologue.getAddressSize());
   else
     assert(Prologue.getAddressSize() == 0 ||
            Prologue.getAddressSize() == DebugLineData.getAddressSize());
 
-  ParsingState State(this);
+  ParsingState State(this, DebugLineOffset, RecoverableErrorHandler);
 
   while (*OffsetPtr < EndOffset) {
     if (OS)
       *OS << format("0x%08.08" PRIx64 ": ", *OffsetPtr);
 
+    uint64_t OpcodeOffset = *OffsetPtr;
     uint8_t Opcode = DebugLineData.getU8(OffsetPtr);
 
     if (OS)
       *OS << format("%02.02" PRIx8 " ", Opcode);
 
     if (Opcode == 0) {
       // Extended Opcodes always start with a zero opcode followed by
       // a uleb128 length so you can skip ones you don't know about
if (Opcode == 0) {
         State.appendRowToMatrix();
         break;
 
       case DW_LNS_advance_pc:
         // Takes a single unsigned LEB128 operand, multiplies it by the
         // min_inst_length field of the prologue, and adds the
         // result to the address register of the state machine.
         {
-          uint64_t AddrOffset =
-              State.advanceAddr(DebugLineData.getULEB128(OffsetPtr));
+          uint64_t AddrOffset = State.advanceAddr(
+              DebugLineData.getULEB128(OffsetPtr), Opcode, OpcodeOffset);
           if (OS)
             *OS << " (" << AddrOffset << ")";
         }
         break;
 
       case DW_LNS_advance_line:
         // Takes a single signed LEB128 operand and adds that value to
         // the line register of the state machine.
if (Opcode == 0) {
         // small amount, it can use a single special opcode, which occupies
         // a single byte. When it needs to advance the address by up to
         // twice the range of the last special opcode, it can use
         // DW_LNS_const_add_pc followed by a special opcode, for a total
         // of two bytes. Only if it needs to advance the address by more
         // than twice that range will it need to use both DW_LNS_advance_pc
         // and a special opcode, requiring three or more bytes.
         {
-          uint64_t AddrOffset = State.advanceAddrForOpcode(Opcode).AddrDelta;
+          uint64_t AddrOffset =
+              State.advanceAddrForOpcode(Opcode, OpcodeOffset).AddrDelta;
           if (OS)
             *OS << format(" (0x%16.16" PRIx64 ")", AddrOffset);
         }
         break;
 
       case DW_LNS_fixed_advance_pc:
         // Takes a single uhalf operand. Add to the address register of
         // the state machine the value of the (unencoded) operand. This
if (Opcode == 0) {
               *OS << format("Skipping ULEB128 value: 0x%16.16" PRIx64 ")\n",
                             Value);
           }
         }
         break;
       }
     } else {
       // Special Opcodes.
-      ParsingState::AddrAndLineDelta Delta = State.handleSpecialOpcode(Opcode);
+      ParsingState::AddrAndLineDelta Delta =
+          State.handleSpecialOpcode(Opcode, OpcodeOffset);
 
       if (OS) {
         *OS << "address += " << Delta.Address << ",  line += " << Delta.Line
             << "\n";
         OS->indent(12);
         State.Row.dump(*OS);
       }
 

llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

 //===- DWARFDebugLineTest.cpp ---------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "DwarfGenerator.h"
 #include "DwarfUtils.h"
 #include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/DebugInfo/DWARF/DWARFDebugLine.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
EXPECT_THAT_ERROR(std::move(Recoverable),
                     FailedWithMessage("last sequence in debug line table at "
                                       "offset 0x00000000 is not terminated"));
   ASSERT_THAT_EXPECTED(ExpectedLineTable, Succeeded());
   EXPECT_EQ((*ExpectedLineTable)->Rows.size(), 6u);
   // The unterminated sequence is not added to the sequence list.
   EXPECT_EQ((*ExpectedLineTable)->Sequences.size(), 1u);
 }
 
+struct AdjustAddressFixtureBase : public CommonFixture {
+  virtual ~AdjustAddressFixtureBase() {}
+
+  // Create and update the prologue as specified by the subclass, then return
+  // the length of the table.
+  virtual uint64_t editPrologue(LineTable &LT) = 0;
+
+  virtual uint64_t getAdjustedAddr(uint64_t Base, uint64_t ConstIncrs,
+                                   uint64_t SpecialIncrs,
+                                   uint64_t AdvanceIncrs) {
+    return Base + ConstIncrs + SpecialIncrs + AdvanceIncrs;
+  }
+
+  virtual uint64_t getAdjustedLine(uint64_t Base, uint64_t Incr) {
+    return Base + Incr;
+  }
+
+  uint64_t setupNoProblemTable() {
+    LineTable &NoProblem = Gen->addLineTable();
+    NoProblem.addExtendedOpcode(9, DW_LNE_set_address,
+                                {{0xabcd, LineTable::Quad}});
+    NoProblem.addExtendedOpcode(1, DW_LNE_end_sequence, {});
+    return editPrologue(NoProblem);
+  }
+
+  uint64_t setupConstAddPcFirstTable() {
+    LineTable &ConstAddPCFirst = Gen->addLineTable();
+    ConstAddPCFirst.addExtendedOpcode(9, DW_LNE_set_address,
+                                      {{ConstAddPCAddr, LineTable::Quad}});
+    ConstAddPCFirst.addStandardOpcode(DW_LNS_const_add_pc, {});
+    ConstAddPCFirst.addStandardOpcode(DW_LNS_const_add_pc, {});
+    ConstAddPCFirst.addStandardOpcode(DW_LNS_advance_pc,
+                                      {{0x10, LineTable::ULEB}});
+    ConstAddPCFirst.addByte(0x21); // Special opcode, +1 op, +1 line.
+    ConstAddPCFirst.addExtendedOpcode(1, DW_LNE_end_sequence, {});
+    return editPrologue(ConstAddPCFirst);
+  }
+
+  uint64_t setupSpecialFirstTable() {
+    LineTable &SpecialFirst = Gen->addLineTable();
+    SpecialFirst.addExtendedOpcode(9, DW_LNE_set_address,
+                                   {{SpecialAddr, LineTable::Quad}});
+    SpecialFirst.addByte(0x22); // Special opcode, +1 op, +2 line.
+    SpecialFirst.addStandardOpcode(DW_LNS_const_add_pc, {});
+    SpecialFirst.addStandardOpcode(DW_LNS_advance_pc,
+                                   {{0x20, LineTable::ULEB}});
+    SpecialFirst.addByte(0x23); // Special opcode, +1 op, +3 line.
+    SpecialFirst.addExtendedOpcode(1, DW_LNE_end_sequence, {});
+    return editPrologue(SpecialFirst);
+  }
+
+  uint64_t setupAdvancePcFirstTable() {
+    LineTable &AdvancePCFirst = Gen->addLineTable();
+    AdvancePCFirst.addExtendedOpcode(9, DW_LNE_set_address,
+                                     {{AdvancePCAddr, LineTable::Quad}});
+    AdvancePCFirst.addStandardOpcode(DW_LNS_advance_pc,
+                                     {{0x30, LineTable::ULEB}});
+    AdvancePCFirst.addStandardOpcode(DW_LNS_const_add_pc, {});
+    AdvancePCFirst.addStandardOpcode(DW_LNS_advance_pc,
+                                     {{0x40, LineTable::ULEB}});
+    AdvancePCFirst.addByte(0x24); // Special opcode, +1 op, +4 line.
+    AdvancePCFirst.addExtendedOpcode(1, DW_LNE_end_sequence, {});
+    return editPrologue(AdvancePCFirst);
+  }
+
+  void setupTables(bool AddAdvancePCFirstTable) {
+    LineTable &Padding = Gen->addLineTable();
+    Padding.setCustomPrologue({{0, LineTable::Byte}});
+    NoProblemOffset = 1;
+
+    // Show that no warning is generated for the case where no
+    // DW_LNS_const_add_pc or special opcode is used.
+    ConstAddPCOffset = setupNoProblemTable() + NoProblemOffset;
+
+    // Show that the warning is emitted for the first DW_LNS_const_add_pc opcode
+    // and then not again.
+    SpecialOffset = setupConstAddPcFirstTable() + ConstAddPCOffset;
+
+    // Show that the warning is emitted for the first special opcode and then
+    // not again.
+    AdvancePCOffset = setupSpecialFirstTable() + SpecialOffset;
+
+    // Show that the warning is emitted for the first DW_LNS_advance_pc opcode
+    // (if requested) and then not again.
+    if (AddAdvancePCFirstTable)
+      setupAdvancePcFirstTable();
+  }
+
+  Expected<const DWARFDebugLine::LineTable *>
+  checkTable(uint64_t Offset, StringRef OpcodeType, const Twine &MsgSuffix) {
+    auto ExpectedTable = Line.getOrParseLineTable(LineData, Offset, *Context,
+                                                  nullptr, RecordRecoverable);
+    EXPECT_THAT_ERROR(std::move(Unrecoverable), Succeeded());
+    if (!IsErrorExpected) {
+      EXPECT_THAT_ERROR(std::move(Recoverable), Succeeded());
+    } else {
+      if (!ExpectedTable)
+        return ExpectedTable;
+      uint64_t ExpectedOffset = Offset +
+                                (*ExpectedTable)->Prologue.getLength() +
+                                11; // 11 == size of DW_LNE_set_address.
+      std::string OffsetHex = Twine::utohexstr(Offset).str();
+      std::string OffsetZeroes = std::string(8 - OffsetHex.size(), '0');
+      std::string ExpectedHex = Twine::utohexstr(ExpectedOffset).str();
+      std::string ExpectedZeroes = std::string(8 - ExpectedHex.size(), '0');
+      EXPECT_THAT_ERROR(
+          std::move(Recoverable),
+          FailedWithMessage(("line table program at offset 0x" + OffsetZeroes +
+                             OffsetHex + " contains a " + OpcodeType +
+                             " opcode at offset 0x" + ExpectedZeroes +
+                             ExpectedHex + ", " + MsgSuffix)
+                                .str()));
+    }
+    return ExpectedTable;
+  }
+
+  void runTest(bool CheckAdvancePC, Twine MsgSuffix) {
+    if (!setupGenerator(Version))
+      return;
+
+    setupTables(/*AddAdvancePCFirstTable=*/CheckAdvancePC);
+
+    generate();
+
+    auto ExpectedNoProblem = Line.getOrParseLineTable(
+        LineData, NoProblemOffset, *Context, nullptr, RecordRecoverable);
+    EXPECT_THAT_ERROR(std::move(Recoverable), Succeeded());
+    EXPECT_THAT_ERROR(std::move(Unrecoverable), Succeeded());
+    ASSERT_THAT_EXPECTED(ExpectedNoProblem, Succeeded());
+
+    auto ExpectedConstAddPC =
+        checkTable(ConstAddPCOffset, "DW_LNS_const_add_pc", MsgSuffix);
+    ASSERT_THAT_EXPECTED(ExpectedConstAddPC, Succeeded());
+    ASSERT_EQ((*ExpectedConstAddPC)->Rows.size(), 2u);
+    EXPECT_EQ((*ExpectedConstAddPC)->Rows[0].Address.Address,
+              getAdjustedAddr(ConstAddPCAddr, ConstIncr * 2, 0x1, 0x10));
+    EXPECT_EQ((*ExpectedConstAddPC)->Rows[0].Line, getAdjustedLine(1, 1));
+    EXPECT_THAT_ERROR(std::move(Unrecoverable), Succeeded());
+
+    auto ExpectedSpecial = checkTable(SpecialOffset, "special", MsgSuffix);
+    ASSERT_THAT_EXPECTED(ExpectedSpecial, Succeeded());
+    ASSERT_EQ((*ExpectedSpecial)->Rows.size(), 3u);
+    EXPECT_EQ((*ExpectedSpecial)->Rows[0].Address.Address,
+              getAdjustedAddr(SpecialAddr, 0, 1, 0));
+    EXPECT_EQ((*ExpectedSpecial)->Rows[0].Line, getAdjustedLine(1, 2));
+    EXPECT_EQ((*ExpectedSpecial)->Rows[1].Address.Address,
+              getAdjustedAddr(SpecialAddr, ConstIncr, 0x2, 0x20));
+    EXPECT_EQ((*ExpectedSpecial)->Rows[1].Line, getAdjustedLine(1, 5));
+    EXPECT_THAT_ERROR(std::move(Unrecoverable), Succeeded());
+
+    if (!CheckAdvancePC)
+      return;
+
+    auto ExpectedAdvancePC =
+        checkTable(AdvancePCOffset, "DW_LNS_advance_pc", MsgSuffix);
+    ASSERT_THAT_EXPECTED(ExpectedAdvancePC, Succeeded());
+    ASSERT_EQ((*ExpectedAdvancePC)->Rows.size(), 2u);
+    EXPECT_EQ((*ExpectedAdvancePC)->Rows[0].Address.Address,
+              getAdjustedAddr(AdvancePCAddr, ConstIncr, 0x1, 0x70));
+    EXPECT_EQ((*ExpectedAdvancePC)->Rows[0].Line, getAdjustedLine(1, 4));
+  }
+
+  uint64_t ConstIncr = 0x11;
+  uint64_t ConstAddPCAddr = 0x1234;
+  uint64_t SpecialAddr = 0x5678;
+  uint64_t AdvancePCAddr = 0xabcd;
+  uint64_t NoProblemOffset;
+  uint64_t ConstAddPCOffset;
+  uint64_t SpecialOffset;
+  uint64_t AdvancePCOffset;
+
+  uint16_t Version = 4;
+  bool IsErrorExpected;
+};
+
+struct MaxOpsPerInstFixture
+    : TestWithParam<std::tuple<uint16_t, uint8_t, bool>>,
+      AdjustAddressFixtureBase {
+  void SetUp() override {
+    std::tie(Version, MaxOpsPerInst, IsErrorExpected) = GetParam();
+  }
+
+  uint64_t editPrologue(LineTable &LT) override {
+    DWARFDebugLine::Prologue Prologue = LT.createBasicPrologue();
+    Prologue.MaxOpsPerInst = MaxOpsPerInst;
+    LT.setPrologue(Prologue);
+    return Prologue.TotalLength + Prologue.sizeofTotalLength();
+  }
+
+  uint8_t MaxOpsPerInst;
+};
+
+TEST_P(MaxOpsPerInstFixture, MaxOpsPerInstProblemsReportedCorrectly) {
+  runTest(/*CheckAdvancePC=*/true,
+          "but the prologue maximum_operations_per_instruction value is " +
+              Twine(unsigned(MaxOpsPerInst)) +
+              ", which is unsupported. Assuming a value of 1 instead");
+}
+
+INSTANTIATE_TEST_CASE_P(
+    MaxOpsPerInstParams, MaxOpsPerInstFixture,
+    Values(std::make_tuple(3, 0, false), // Test for version < 4 (no error).
+           std::make_tuple(4, 0, true),  // Test zero value for V4 (error).
+           std::make_tuple(4, 1, false), // Test good value for V4 (no error).
+           std::make_tuple(
+               4, 2, true)), ); // Test one higher than permitted V4 (error).
+
 TEST_F(DebugLineBasicFixture, ParserParsesCorrectly) {
   if (!setupGenerator())
     return;
 
   DWARFDebugLine::SectionParser Parser = setupParser();
 
   EXPECT_EQ(Parser.getOffset(), 0u);
   ASSERT_FALSE(Parser.done());