Author: Charles Zablit Date: 2026-07-13T12:53:40+01:00 New Revision: 3081049a69f85dd4079a959c4e50c6ccf9f691d0
URL: https://github.com/llvm/llvm-project/commit/3081049a69f85dd4079a959c4e50c6ccf9f691d0 DIFF: https://github.com/llvm/llvm-project/commit/3081049a69f85dd4079a959c4e50c6ccf9f691d0.diff LOG: [lldb] Don't build an opcode from a failed disassembly (#208279) lldb does not check if the instruction was decoded correctly. This causes the following assert to hit: https://github.com/llvm/llvm-project/blob/06499c927e5012166f4553c58f8af593521ea14d/lldb/include/lldb/Core/Opcode.h#L214 due to a buffer overflow. This happens when starting `lldb-dap` on Windows with `lldb-server`. This patch clamps the instruction size to the maximum possible instruction size possible to avoid the assert to hit. It also adds a regression test. Related: - https://github.com/llvm/llvm-project/pull/208277. Added: lldb/unittests/Disassembler/x86/TestGetOpcodeOversized.cpp Modified: lldb/include/lldb/Core/Opcode.h lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp lldb/unittests/Disassembler/CMakeLists.txt Removed: ################################################################################ diff --git a/lldb/include/lldb/Core/Opcode.h b/lldb/include/lldb/Core/Opcode.h index 7e756d3f15d22..2738e71a207da 100644 --- a/lldb/include/lldb/Core/Opcode.h +++ b/lldb/include/lldb/Core/Opcode.h @@ -43,6 +43,11 @@ class Opcode { Opcode() = default; + /// The size in bytes of the fixed buffer used to store opcode bytes. It must + /// be big enough to hold the longest opcode of any supported target (x86 + /// caps instructions at 15 bytes). + static constexpr size_t kMaxByteSize = 16; + Opcode(uint8_t inst, lldb::ByteOrder order) : m_byte_order(order), m_type(eType8) { m_data.inst8 = inst; @@ -294,8 +299,8 @@ class Opcode { uint32_t inst32; uint64_t inst64; struct { - uint8_t bytes[16]; // This must be big enough to handle any opcode for any - // supported target. + uint8_t bytes[kMaxByteSize]; // This must be big enough to handle any + // opcode for any supported target. uint8_t length; } inst; } m_data; diff --git a/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp b/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp index a18c95bdbde98..6129967ce65bc 100644 --- a/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp +++ b/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp @@ -45,6 +45,8 @@ #include "lldb/Utility/Log.h" #include "lldb/Utility/RegularExpression.h" #include "lldb/Utility/Stream.h" + +#include <algorithm> #include <optional> using namespace lldb; @@ -530,6 +532,7 @@ class InstructionLLVMC : public lldb_private::Instruction { m_is_valid = mc_disasm_ptr->GetMCInst(opcode_data, opcode_data_len, pc, inst, inst_size); m_opcode.Clear(); + inst_size = std::min<uint64_t>(inst_size, Opcode::kMaxByteSize); if (inst_size != 0) { if (arch.GetTriple().isRISCV()) m_opcode.SetOpcode16_32TupleBytes(opcode_data, inst_size, diff --git a/lldb/unittests/Disassembler/CMakeLists.txt b/lldb/unittests/Disassembler/CMakeLists.txt index 0f6d21178933c..8894bcfa8f081 100644 --- a/lldb/unittests/Disassembler/CMakeLists.txt +++ b/lldb/unittests/Disassembler/CMakeLists.txt @@ -10,6 +10,7 @@ endif() if("X86" IN_LIST LLVM_TARGETS_TO_BUILD) list(APPEND disas_srcs x86/TestGetControlFlowKindx86.cpp + x86/TestGetOpcodeOversized.cpp ) endif() diff --git a/lldb/unittests/Disassembler/x86/TestGetOpcodeOversized.cpp b/lldb/unittests/Disassembler/x86/TestGetOpcodeOversized.cpp new file mode 100644 index 0000000000000..793d221634958 --- /dev/null +++ b/lldb/unittests/Disassembler/x86/TestGetOpcodeOversized.cpp @@ -0,0 +1,90 @@ +//===----------------------------------------------------------------------===// +// +// 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 "llvm/Support/TargetSelect.h" +#include "gtest/gtest.h" + +#include "lldb/Core/Address.h" +#include "lldb/Core/Disassembler.h" +#include "lldb/Core/Opcode.h" +#include "lldb/Utility/ArchSpec.h" + +#include "Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h" + +using namespace lldb; +using namespace lldb_private; + +namespace { +class TestGetOpcodeOversized : public testing::Test { +public: + static void SetUpTestCase(); + static void TearDownTestCase(); +}; + +void TestGetOpcodeOversized::SetUpTestCase() { + llvm::InitializeAllTargets(); + llvm::InitializeAllAsmPrinters(); + llvm::InitializeAllTargetMCs(); + llvm::InitializeAllDisassemblers(); + DisassemblerLLVMC::Initialize(); +} + +void TestGetOpcodeOversized::TearDownTestCase() { + DisassemblerLLVMC::Terminate(); +} +} // namespace + +// A long enough run of redundant prefixes makes the x86 disassembler report an +// instruction whose length is greater than Opcode's fixed-size byte buffer. +TEST_F(TestGetOpcodeOversized, OversizedInstructionIsTruncated) { + ArchSpec arch("x86_64-*-linux"); + + // 20 operand-size (0x66) prefixes followed by a NOP: decodes successfully as + // a 21-byte instruction, overlowing the 15-byte x86 limit and the 16-byte + // opcode buffer. + uint8_t data[21]; + memset(data, 0x66, sizeof(data)); + data[sizeof(data) - 1] = 0x90; // nop + static_assert(sizeof(data) > Opcode::kMaxByteSize, + "test input must exceed the opcode buffer"); + + Address start_addr(0x100); + DisassemblerSP disass_sp = Disassembler::DisassembleBytes( + arch, nullptr, nullptr, nullptr, nullptr, start_addr, &data, sizeof(data), + /*num_instructions=*/1, false); + + if (!disass_sp) + return; + + const InstructionList &inst_list = disass_sp->GetInstructionList(); + ASSERT_EQ(1u, inst_list.GetSize()); + + // The instruction is still produced, but its recorded opcode is clamped to + // the buffer capacity rather than overflowing it. + InstructionSP inst_sp = inst_list.GetInstructionAtIndex(0); + EXPECT_LE(inst_sp->GetOpcode().GetByteSize(), Opcode::kMaxByteSize); +} + +// A normal instruction is unaffected by the truncation logic. +TEST_F(TestGetOpcodeOversized, NormalInstructionIsUnchanged) { + ArchSpec arch("x86_64-*-linux"); + + uint8_t data[] = {0x48, 0x83, 0xec, 0x18}; // subq $0x18, %rsp + + Address start_addr(0x100); + DisassemblerSP disass_sp = Disassembler::DisassembleBytes( + arch, nullptr, nullptr, nullptr, nullptr, start_addr, &data, sizeof(data), + /*num_instructions=*/1, false); + + if (!disass_sp) + return; + + const InstructionList &inst_list = disass_sp->GetInstructionList(); + ASSERT_EQ(1u, inst_list.GetSize()); + EXPECT_EQ(4u, inst_list.GetInstructionAtIndex(0)->GetOpcode().GetByteSize()); +} _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
