https://github.com/charles-zablit updated https://github.com/llvm/llvm-project/pull/208279
>From efa86ebdbba4960f663969c4004317a5001d64ec Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Wed, 8 Jul 2026 18:53:21 +0100 Subject: [PATCH 1/2] [lldb] Don't build an opcode from a failed disassembly --- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp b/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp index a18c95bdbde98..fe59e988e3cd9 100644 --- a/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp +++ b/lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp @@ -530,7 +530,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(); - if (inst_size != 0) { + if (m_is_valid && inst_size != 0) { if (arch.GetTriple().isRISCV()) m_opcode.SetOpcode16_32TupleBytes(opcode_data, inst_size, byte_order); >From 946b826afd082c2a36b4341f4bef82c56359c9ad Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Thu, 9 Jul 2026 17:49:34 +0100 Subject: [PATCH 2/2] fixup! [lldb] Don't build an opcode from a failed disassembly --- lldb/include/lldb/Core/Opcode.h | 9 +- .../Disassembler/LLVMC/DisassemblerLLVMC.cpp | 5 +- lldb/unittests/Disassembler/CMakeLists.txt | 1 + .../x86/TestGetOpcodeOversized.cpp | 90 +++++++++++++++++++ 4 files changed, 102 insertions(+), 3 deletions(-) create mode 100644 lldb/unittests/Disassembler/x86/TestGetOpcodeOversized.cpp 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 fe59e988e3cd9..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,7 +532,8 @@ 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(); - if (m_is_valid && inst_size != 0) { + 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, byte_order); 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
