llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: MrEven132 <details> <summary>Changes</summary> `DWARFExpression::Operation::extract` used unchecked ULEB128 and SLEB128 reads. When an operand exceeded the decoder's 64-bit representation, `DataExtractor` returned zero without advancing the offset, so later payload bytes could be interpreted as DWARF opcodes. Use checked LEB128 reads for expression operands. For `DW_OP_constu` and `DW_OP_consts`, consume a complete well-formed arbitrary-width encoding and retain the low address-sized bits required by the DWARF generic type. Other oversized or malformed LEB operands now put the operation in an operand decoding error state instead of producing a partial operation. LLDB reports the failed opcode and expression offset, while the LLVM compact printer reports a decoding error without changing the existing fallback behavior for unknown vendor operations. Add LLVM and LLDB unit tests covering oversized unsigned and signed constants, full operand consumption, oversized non-generic ULEB128 and SLEB128 operands, unterminated input, iterator synchronization, and compact printing. Fixes #<!-- -->202293 --- Full diff: https://github.com/llvm/llvm-project/pull/219149.diff 6 Files Affected: - (modified) lldb/source/Expression/DWARFExpression.cpp (+5) - (modified) lldb/unittests/Expression/DWARFExpressionTest.cpp (+72) - (modified) llvm/include/llvm/DebugInfo/DWARF/LowLevel/DWARFExpression.h (+5) - (modified) llvm/lib/DebugInfo/DWARF/DWARFExpressionPrinter.cpp (+5) - (modified) llvm/lib/DebugInfo/DWARF/LowLevel/DWARFExpression.cpp (+75-8) - (modified) llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp (+51) ``````````diff diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp index e62b6945dc3ed..cf8e2967cb4f7 100644 --- a/lldb/source/Expression/DWARFExpression.cpp +++ b/lldb/source/Expression/DWARFExpression.cpp @@ -1447,6 +1447,11 @@ llvm::Expected<Value> DWARFExpression::Evaluate( DW_OP_value_to_name(opcode)); } + if (op->isOperandError()) + return llvm::createStringError( + "unable to decode operands for %s at offset 0x%" PRIx64, + DW_OP_value_to_name(opcode), op_offset); + if (std::optional<unsigned> arity = OperationArity(opcode)) { if (stack.size() < *arity) return llvm::createStringError( diff --git a/lldb/unittests/Expression/DWARFExpressionTest.cpp b/lldb/unittests/Expression/DWARFExpressionTest.cpp index 76d2d4efcf557..02fd560372a3c 100644 --- a/lldb/unittests/Expression/DWARFExpressionTest.cpp +++ b/lldb/unittests/Expression/DWARFExpressionTest.cpp @@ -733,6 +733,78 @@ TEST(DWARFExpression, GenericBinaryOpsAllowDifferentSignedness) { ExpectScalar(4)); } +TEST(DWARFExpression, OversizedLEB128Constants) { + auto evaluate = [](uint8_t opcode, uint8_t low_byte, + uint8_t expected_literal) { + std::vector<uint8_t> expr = { + opcode, + // These operands encode positive 2^64 plus the low seven bits. + low_byte, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x02, + static_cast<uint8_t>(DW_OP_lit0 + expected_literal), DW_OP_eq, + DW_OP_stack_value}; + DataExtractor extractor(expr.data(), expr.size(), lldb::eByteOrderLittle, + /*addr_size=*/8); + return DWARFExpression::Evaluate( + /*exe_ctx=*/nullptr, /*reg_ctx=*/nullptr, /*module_sp=*/{}, extractor, + /*unit=*/nullptr, lldb::eRegisterKindLLDB, + /*initial_value_ptr=*/nullptr, /*object_address_ptr=*/nullptr); + }; + + // Generic constants are address-sized, so the high 2^64 bit is discarded. + // The comparisons also verify that the complete LEB128 operands are consumed. + EXPECT_THAT_EXPECTED(evaluate(DW_OP_constu, 0x80, 0), + ExpectScalar(64, 1, false)); + EXPECT_THAT_EXPECTED(evaluate(DW_OP_consts, 0x80, 0), + ExpectScalar(64, 1, false)); + EXPECT_THAT_EXPECTED(evaluate(DW_OP_constu, 0x85, 5), + ExpectScalar(64, 1, false)); + EXPECT_THAT_EXPECTED(evaluate(DW_OP_consts, 0x85, 5), + ExpectScalar(64, 1, false)); +} + +TEST(DWARFExpression, RejectsOversizedLEB128Operand) { + auto evaluate = [](llvm::ArrayRef<uint8_t> expr) { + DataExtractor extractor(expr.data(), expr.size(), lldb::eByteOrderLittle, + /*addr_size=*/8); + return DWARFExpression::Evaluate( + /*exe_ctx=*/nullptr, /*reg_ctx=*/nullptr, /*module_sp=*/{}, extractor, + /*unit=*/nullptr, lldb::eRegisterKindLLDB, + /*initial_value_ptr=*/nullptr, /*object_address_ptr=*/nullptr); + }; + + std::vector<uint8_t> oversized_uleb = {DW_OP_lit0, + DW_OP_plus_uconst, + 0x80, + 0x80, + 0x80, + 0x80, + 0x80, + 0x80, + 0x80, + 0x80, + 0x80, + 0x02, + DW_OP_stack_value}; + EXPECT_THAT_EXPECTED( + evaluate(oversized_uleb), + llvm::FailedWithMessage( + "unable to decode operands for DW_OP_plus_uconst at offset 0x1")); + + const uint8_t oversized_sleb[] = {DW_OP_breg0, 0x80, 0x80, 0x80, + 0x80, 0x80, 0x80, 0x80, + 0x80, 0x80, 0x02, DW_OP_stack_value}; + EXPECT_THAT_EXPECTED( + evaluate(oversized_sleb), + llvm::FailedWithMessage( + "unable to decode operands for DW_OP_breg0 at offset 0x0")); + + const uint8_t unterminated_constant[] = {DW_OP_constu, 0x80}; + EXPECT_THAT_EXPECTED( + evaluate(unterminated_constant), + llvm::FailedWithMessage( + "unable to decode operands for DW_OP_constu at offset 0x0")); +} + TEST(DWARFExpression, RelationalOpsProduceGenericResult) { struct TestCase { uint8_t opcode; diff --git a/llvm/include/llvm/DebugInfo/DWARF/LowLevel/DWARFExpression.h b/llvm/include/llvm/DebugInfo/DWARF/LowLevel/DWARFExpression.h index 2bf448fb21708..724b3aee5584c 100644 --- a/llvm/include/llvm/DebugInfo/DWARF/LowLevel/DWARFExpression.h +++ b/llvm/include/llvm/DebugInfo/DWARF/LowLevel/DWARFExpression.h @@ -90,6 +90,7 @@ class DWARFExpression { uint8_t Opcode; ///< The Op Opcode, DW_OP_<something>. Description Desc; bool Error = false; + bool OperandError = false; uint64_t EndOffset; SmallVector<uint64_t> Operands; SmallVector<uint64_t> OperandEndOffsets; @@ -109,6 +110,10 @@ class DWARFExpression { } uint64_t getEndOffset() const { return EndOffset; } bool isError() const { return Error; } + /// Whether a known operation failed because an operand could not be + /// decoded. Unknown operations can be in the general error state without + /// having an operand error. + bool isOperandError() const { return Error && OperandError; } private: LLVM_ABI bool extract(DataExtractor Data, uint8_t AddressSize, diff --git a/llvm/lib/DebugInfo/DWARF/DWARFExpressionPrinter.cpp b/llvm/lib/DebugInfo/DWARF/DWARFExpressionPrinter.cpp index 7a80ae027db1e..782b150e14c42 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFExpressionPrinter.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFExpressionPrinter.cpp @@ -297,6 +297,11 @@ static bool printCompactDWARFExpr( while (I != E) { const DWARFExpression::Operation &Op = *I; + if (Op.isOperandError()) { + OS << "<decoding error>"; + return false; + } + uint8_t Opcode = Op.getCode(); switch (Opcode) { case dwarf::DW_OP_regx: { diff --git a/llvm/lib/DebugInfo/DWARF/LowLevel/DWARFExpression.cpp b/llvm/lib/DebugInfo/DWARF/LowLevel/DWARFExpression.cpp index a0b5c42fa78cb..97df3bc6d8bb7 100644 --- a/llvm/lib/DebugInfo/DWARF/LowLevel/DWARFExpression.cpp +++ b/llvm/lib/DebugInfo/DWARF/LowLevel/DWARFExpression.cpp @@ -7,8 +7,10 @@ //===----------------------------------------------------------------------===// #include "llvm/DebugInfo/DWARF/LowLevel/DWARFExpression.h" +#include "llvm/Support/Error.h" #include <cassert> #include <cstdint> +#include <optional> #include <vector> using namespace llvm; @@ -158,9 +160,54 @@ static Desc getSubOpDesc(unsigned Opcode, unsigned SubOpcode) { return getDescImpl(Descriptions, SubOpcode); } +static std::optional<uint64_t> extractLEB128(const DataExtractor &Data, + uint64_t &Offset, bool Signed) { + DataExtractor::Cursor Cursor(Offset); + uint64_t Value = Signed ? static_cast<uint64_t>(Data.getSLEB128(Cursor)) + : Data.getULEB128(Cursor); + if (!Cursor) { + consumeError(Cursor.takeError()); + return std::nullopt; + } + Offset = Cursor.tell(); + return Value; +} + +static std::optional<uint64_t> extractGenericConstant(const DataExtractor &Data, + uint64_t &Offset, + uint8_t AddressSize, + bool Signed) { + if (std::optional<uint64_t> Value = extractLEB128(Data, Offset, Signed)) + return Value; + + // Generic constants are truncated to the target's address-sized generic + // type. If the mathematical value does not fit the decoder's 64-bit return + // type, scan the complete operand while retaining only those low bits. + StringRef Bytes = Data.getData(); + const unsigned BitSize = AddressSize * 8; + unsigned Shift = 0; + uint64_t Value = 0; + while (Offset < Bytes.size()) { + uint8_t Byte = static_cast<uint8_t>(Bytes[Offset++]); + if (Shift < BitSize) { + unsigned RemainingBits = BitSize - Shift; + uint64_t Slice = Byte & 0x7f; + if (RemainingBits < 7) + Slice &= (uint64_t(1) << RemainingBits) - 1; + Value |= Slice << Shift; + } + if ((Byte & 0x80) == 0) + return Value; + if (Shift < BitSize) + Shift += 7; + } + return std::nullopt; +} + bool DWARFExpression::Operation::extract(DataExtractor Data, uint8_t AddressSize, uint64_t Offset, std::optional<DwarfFormat> Format) { + OperandError = false; EndOffset = Offset; Opcode = Data.getU8(&Offset); @@ -173,11 +220,21 @@ bool DWARFExpression::Operation::extract(DataExtractor Data, for (unsigned Operand = 0; Operand < Desc.Op.size(); ++Operand) { unsigned Size = Desc.Op[Operand]; unsigned Signed = Size & Operation::SignBit; + auto ExtractLEBOperand = [&](bool IsSigned) { + std::optional<uint64_t> Value = extractLEB128(Data, Offset, IsSigned); + if (!Value) { + OperandError = true; + return false; + } + Operands[Operand] = *Value; + return true; + }; switch (Size & ~Operation::SignBit) { case Operation::SizeSubOpLEB: assert(Operand == 0 && "SubOp operand must be the first operand"); - Operands[Operand] = Data.getULEB128(&Offset); + if (!ExtractLEBOperand(/*IsSigned=*/false)) + return false; Desc = getSubOpDesc(Opcode, Operands[Operand]); if (Desc.Version == Operation::DwarfNA) return false; @@ -214,17 +271,26 @@ bool DWARFExpression::Operation::extract(DataExtractor Data, Data.getUnsigned(&Offset, dwarf::getDwarfOffsetByteSize(*Format)); break; case Operation::SizeLEB: - if (Signed) - Operands[Operand] = Data.getSLEB128(&Offset); - else - Operands[Operand] = Data.getULEB128(&Offset); + if (Opcode == DW_OP_constu || Opcode == DW_OP_consts) { + if (std::optional<uint64_t> Value = extractGenericConstant( + Data, Offset, AddressSize, /*Signed=*/Signed)) + Operands[Operand] = *Value; + else { + OperandError = true; + return false; + } + } else if (!ExtractLEBOperand(Signed)) { + return false; + } break; case Operation::BaseTypeRef: - Operands[Operand] = Data.getULEB128(&Offset); + if (!ExtractLEBOperand(/*IsSigned=*/false)) + return false; break; case Operation::NvidiaMuxArg: assert(Operand == 1); - Operands[Operand] = Data.getULEB128(&Offset); + if (!ExtractLEBOperand(/*IsSigned=*/false)) + return false; // The selector names an NVIDIA specific operation, and the number and // type of the operands that follow it are implied by that operation. // No NVIDIA operation is known here, so where this operation ends is @@ -239,7 +305,8 @@ bool DWARFExpression::Operation::extract(DataExtractor Data, case 1: case 2: case 4: - Operands[Operand] = Data.getULEB128(&Offset); + if (!ExtractLEBOperand(/*IsSigned=*/false)) + return false; break; case 3: // global as uint32 Operands[Operand] = Data.getU32(&Offset); diff --git a/llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp index ff2f45334dbfc..48978cfb5d5c4 100644 --- a/llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp +++ b/llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp @@ -290,6 +290,57 @@ TEST(NVIDIAMux, Full_DW_OP_LLVM_NVIDIA_mux_MissingSelector) { EXPECT_EQ(OS.str(), "<decoding error> e9 0d"); } +TEST(LEB128Operands, OversizedGenericConstants) { + constexpr uint8_t Opcodes[] = {DW_OP_constu, DW_OP_consts}; + for (uint8_t Opcode : Opcodes) { + const uint8_t Enc[] = {Opcode, + // Positive 2^64 + 5 as either ULEB128 or SLEB128. + 0x85, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, + 0x02, DW_OP_stack_value}; + DataExtractor DE(Enc, true); + DWARFExpression Expr(DE, 8); + + auto It = Expr.begin(); + ASSERT_FALSE(It->isError()); + EXPECT_FALSE(It->isOperandError()); + EXPECT_EQ(It->getRawOperand(0), 5u); + EXPECT_EQ(It->getEndOffset(), 11u); + ++It; + ASSERT_FALSE(It->isError()); + EXPECT_EQ(It->getCode(), DW_OP_stack_value); + } +} + +TEST(LEB128Operands, OversizedNonGenericOperand) { + const uint8_t Enc[] = {DW_OP_plus_uconst, + 0x80, + 0x80, + 0x80, + 0x80, + 0x80, + 0x80, + 0x80, + 0x80, + 0x80, + 0x02, + DW_OP_stack_value}; + DataExtractor DE(Enc, true); + DWARFExpression Expr(DE, 8); + + auto It = Expr.begin(); + EXPECT_TRUE(It->isError()); + EXPECT_TRUE(It->isOperandError()); + ++It; + EXPECT_EQ(It, Expr.end()); +} + +TEST_F(DWARFExpressionCompactPrinterTest, + OversizedNonGenericOperandFailsCompactPrinting) { + TestExprPrinterFailure( + {DW_OP_breg0, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x02}, + "<decoding error>"); +} + // NVPTX packs virtual register names into DWARF register numbers, so compact // printing without a callback must recover the name and return true. TEST(NVPTXPackedRegister, Compact_DW_OP_regx_NoMRI) { `````````` </details> https://github.com/llvm/llvm-project/pull/219149 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
