Fix operand access being wrong about conditional access

zyantific · Dec 10, 2024 · 07c0bdb · 07c0bdb
1 parent e7d80fb
commit 07c0bdb
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 4 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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