From 07c0bdb8e8a356fd9bad3356c32209bbb815695a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=CE=B6eh=20Matt?= <5415177+ZehMatt@users.noreply.github.com>
Date: Tue, 10 Dec 2024 17:20:59 +0200
Subject: [PATCH] Fix operand access being wrong about conditional access
---
tests/src/tests/tests.decoder.cpp | 44 +++++++++++++++++++++
tests/src/tests/tests.packed.cpp | 47 ++++++++++++++++++++++-
zasm/include/zasm/core/packed.hpp | 4 +-
zasm/include/zasm/program/instruction.hpp | 2 +-
4 files changed, 93 insertions(+), 4 deletions(-)
diff --git a/tests/src/tests/tests.decoder.cpp b/tests/src/tests/tests.decoder.cpp
index 22cafb7..c1e22d1 100644
--- a/tests/src/tests/tests.decoder.cpp
+++ b/tests/src/tests/tests.decoder.cpp
@@ -78,4 +78,48 @@ namespace zasm::tests
}
}
+ TEST(DecoderTests, Issue_149)
+ {
+ Program program(MachineMode::AMD64);
+ Decoder decoder(MachineMode::AMD64);
+
+ const std::array<uint8_t, 8> inputBytes = {
+ 0xF0, 0x0F, 0xB1, 0x95, 0x74, 0x17, 0x01, 0x00, // lock cmpxchg dword ptr ss:[rbp+0x11774], edx
+ };
+
+ auto decoded = decoder.decode(inputBytes.data(), inputBytes.size(), 0x00400000);
+ ASSERT_TRUE(decoded);
+
+ ASSERT_EQ(decoded->getMnemonic(), x86::Mnemonic::Cmpxchg);
+
+ ASSERT_EQ(decoded->getOperandCount(), 4);
+
+ ASSERT_EQ(decoded->isOperandType<Mem>(0), true);
+ ASSERT_EQ(decoded->getOperand<Mem>(0).getBase(), x86::rbp);
+ ASSERT_EQ(decoded->getOperand<Mem>(0).getDisplacement(), 0x11774);
+ ASSERT_EQ(decoded->isOperandRead(0), true);
+ ASSERT_EQ(decoded->isOperandWrite(0), true);
+ ASSERT_EQ(decoded->isOperandCondWrite(0), true);
+
+ ASSERT_EQ(decoded->isOperandType<Reg>(1), true);
+ ASSERT_EQ(decoded->getOperand<Reg>(1), x86::edx);
+ ASSERT_EQ(decoded->isOperandRead(1), true);
+ ASSERT_EQ(decoded->isOperandCondRead(1), false);
+ ASSERT_EQ(decoded->isOperandWrite(1), false);
+ ASSERT_EQ(decoded->isOperandCondWrite(1), false);
+
+ ASSERT_EQ(decoded->isOperandType<Reg>(2), true);
+ ASSERT_EQ(decoded->getOperand<Reg>(2), x86::eax);
+ ASSERT_EQ(decoded->isOperandRead(2), true);
+ ASSERT_EQ(decoded->isOperandWrite(2), true);
+ ASSERT_EQ(decoded->isOperandCondWrite(2), true);
+
+ ASSERT_EQ(decoded->isOperandType<Reg>(3), true);
+ ASSERT_EQ(decoded->getOperand<Reg>(3), x86::rflags);
+ ASSERT_EQ(decoded->isOperandRead(3), false);
+ ASSERT_EQ(decoded->isOperandCondRead(3), false);
+ ASSERT_EQ(decoded->isOperandWrite(3), true);
+ ASSERT_EQ(decoded->isOperandCondWrite(3), false);
+ }
+
} // namespace zasm::tests
diff --git a/tests/src/tests/tests.packed.cpp b/tests/src/tests/tests.packed.cpp
index 46d4424..4c0d91f 100644
--- a/tests/src/tests/tests.packed.cpp
+++ b/tests/src/tests/tests.packed.cpp
@@ -81,4 +81,49 @@ namespace zasm::tests
ASSERT_EQ(vis.get<8>(), Operand::Visibility::Invalid);
ASSERT_EQ(vis.get<9>(), Operand::Visibility::Invalid);
}
-} // namespace zasm::tests
+
+ TEST(PackedTests, TestPackOperandAccess)
+ {
+ using PackedOperandAccess = Packed<uint64_t, Operand::Access, 4>;
+ static_assert(sizeof(PackedOperandAccess) == sizeof(uint64_t));
+
+ PackedOperandAccess access{};
+ ASSERT_EQ(access.size(), 16);
+ ASSERT_EQ(access.capacity(), 16);
+
+ ASSERT_EQ(access.get<0>(), Operand::Access::None);
+ ASSERT_EQ(access.get<1>(), Operand::Access::None);
+ ASSERT_EQ(access.get<2>(), Operand::Access::None);
+ ASSERT_EQ(access.get<3>(), Operand::Access::None);
+ ASSERT_EQ(access.get<4>(), Operand::Access::None);
+ ASSERT_EQ(access.get<5>(), Operand::Access::None);
+ ASSERT_EQ(access.get<6>(), Operand::Access::None);
+ ASSERT_EQ(access.get<7>(), Operand::Access::None);
+ ASSERT_EQ(access.get<8>(), Operand::Access::None);
+ ASSERT_EQ(access.get<9>(), Operand::Access::None);
+ ASSERT_EQ(access.get<10>(), Operand::Access::None);
+
+ access.set<0>(Operand::Access::Read);
+ ASSERT_EQ(access.get<0>(), Operand::Access::Read);
+
+ access.set<5>(Operand::Access::Write);
+ ASSERT_EQ(access.get<0>(), Operand::Access::Read);
+ ASSERT_EQ(access.get<5>(), Operand::Access::Write);
+
+ access.set<10>(Operand::Access::CondRead);
+ ASSERT_EQ(access.get<0>(), Operand::Access::Read);
+
+ access.set<15>(Operand::Access::CondWrite);
+ ASSERT_EQ(access.get<0>(), Operand::Access::Read);
+ ASSERT_EQ(access.get<5>(), Operand::Access::Write);
+ ASSERT_EQ(access.get<10>(), Operand::Access::CondRead);
+ ASSERT_EQ(access.get<15>(), Operand::Access::CondWrite);
+
+ access.set<15>(Operand::Access::None);
+ ASSERT_EQ(access.get<0>(), Operand::Access::Read);
+ ASSERT_EQ(access.get<5>(), Operand::Access::Write);
+ ASSERT_EQ(access.get<10>(), Operand::Access::CondRead);
+ ASSERT_EQ(access.get<15>(), Operand::Access::None);
+ }
+
+ } // namespace zasm::tests
diff --git a/zasm/include/zasm/core/packed.hpp b/zasm/include/zasm/core/packed.hpp
index 44f1d45..34f9a44 100644
--- a/zasm/include/zasm/core/packed.hpp
+++ b/zasm/include/zasm/core/packed.hpp
@@ -54,7 +54,7 @@ namespace zasm
template<std::size_t TIndex> constexpr void set(const TElement& val) noexcept
{
constexpr auto bitIndex = (TIndex * TElementBitSize);
- static_assert(bitIndex + TElementBitSize < kStorageBitSize);
+ static_assert(bitIndex + TElementBitSize <= kStorageBitSize);
const auto newVal = (static_cast<TUnderlying>(val) & kElementMask) << bitIndex;
@@ -81,7 +81,7 @@ namespace zasm
template<std::size_t TIndex> constexpr TElement get() const noexcept
{
constexpr auto bitIndex = (TIndex * TElementBitSize);
- static_assert(bitIndex + TElementBitSize < kStorageBitSize);
+ static_assert(bitIndex + TElementBitSize <= kStorageBitSize);
const auto res = (_data >> bitIndex) & kElementMask;
return static_cast<TElement>(res);
diff --git a/zasm/include/zasm/program/instruction.hpp b/zasm/include/zasm/program/instruction.hpp
index ed861c2..e393ca4 100644
--- a/zasm/include/zasm/program/instruction.hpp
+++ b/zasm/include/zasm/program/instruction.hpp
@@ -67,7 +67,7 @@ namespace zasm
public:
static constexpr auto kInstrType = InstructionBase::Type::Detail;
- using OperandsAccess = Packed<std::uint32_t, Operand::Access, 3>;
+ using OperandsAccess = Packed<std::uint64_t, Operand::Access, 4>;
using OperandsVisibility = Packed<std::uint32_t, Operand::Visibility, 3>;
struct CPUFlags