This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGa0b674fd7f06: Fix UB in EmulateInstructionARM64.cpp (authored by aprantl). Herald added a subscriber: mgorny. Herald added a project: LLDB.
Changed prior to commit: https://reviews.llvm.org/D80955?vs=267734&id=267776#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80955/new/ https://reviews.llvm.org/D80955 Files: lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h lldb/unittests/CMakeLists.txt lldb/unittests/Instruction/CMakeLists.txt lldb/unittests/Instruction/TestAArch64Emulator.cpp
Index: lldb/unittests/Instruction/TestAArch64Emulator.cpp =================================================================== --- /dev/null +++ lldb/unittests/Instruction/TestAArch64Emulator.cpp @@ -0,0 +1,62 @@ +//===-- TestAArch64Emulator.cpp ------------------------------------------===// + +// +// 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 "gtest/gtest.h" + +#include "lldb/Core/Address.h" +#include "lldb/Core/Disassembler.h" +#include "lldb/Target/ExecutionContext.h" +#include "lldb/Utility/ArchSpec.h" + +#include "Plugins/Instruction/ARM64/EmulateInstructionARM64.h" + +using namespace lldb; +using namespace lldb_private; + +struct Arch64EmulatorTester : public EmulateInstructionARM64 { + Arch64EmulatorTester() + : EmulateInstructionARM64(ArchSpec("arm64-apple-ios")) {} + + static uint64_t AddWithCarry(uint32_t N, uint64_t x, uint64_t y, bool carry_in, + EmulateInstructionARM64::ProcState &proc_state) { + return EmulateInstructionARM64::AddWithCarry(N, x, y, carry_in, proc_state); + } +}; + +class TestAArch64Emulator : public testing::Test { +public: + static void SetUpTestCase(); + static void TearDownTestCase(); + +protected: +}; + +void TestAArch64Emulator::SetUpTestCase() { + EmulateInstructionARM64::Initialize(); +} + +void TestAArch64Emulator::TearDownTestCase() { + EmulateInstructionARM64::Terminate(); +} + +TEST_F(TestAArch64Emulator, TestOverflow) { + EmulateInstructionARM64::ProcState pstate; + memset(&pstate, 0, sizeof(pstate)); + uint64_t ll_max = std::numeric_limits<int64_t>::max(); + Arch64EmulatorTester emu; + ASSERT_EQ(emu.AddWithCarry(64, ll_max, 0, 0, pstate), ll_max); + ASSERT_EQ(pstate.V, 0ULL); + ASSERT_EQ(pstate.C, 0ULL); + ASSERT_EQ(emu.AddWithCarry(64, ll_max, 1, 0, pstate), (uint64_t)(ll_max + 1)); + ASSERT_EQ(pstate.V, 1ULL); + ASSERT_EQ(pstate.C, 0ULL); + ASSERT_EQ(emu.AddWithCarry(64, ll_max, 0, 1, pstate), (uint64_t)(ll_max + 1)); + ASSERT_EQ(pstate.V, 1ULL); + ASSERT_EQ(pstate.C, 0ULL); +} Index: lldb/unittests/Instruction/CMakeLists.txt =================================================================== --- /dev/null +++ lldb/unittests/Instruction/CMakeLists.txt @@ -0,0 +1,12 @@ +if("ARM" IN_LIST LLVM_TARGETS_TO_BUILD) + add_lldb_unittest(EmulatorTests + TestAArch64Emulator.cpp + LINK_LIBS + lldbCore + lldbSymbol + lldbTarget + lldbPluginInstructionARM64 + LINK_COMPONENTS + Support + ${LLVM_TARGETS_TO_BUILD}) +endif() Index: lldb/unittests/CMakeLists.txt =================================================================== --- lldb/unittests/CMakeLists.txt +++ lldb/unittests/CMakeLists.txt @@ -71,6 +71,7 @@ add_subdirectory(Expression) add_subdirectory(Host) add_subdirectory(Interpreter) +add_subdirectory(Instruction) add_subdirectory(Language) add_subdirectory(ObjectFile) add_subdirectory(Platform) Index: lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h =================================================================== --- lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h +++ lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h @@ -152,6 +152,9 @@ } ProcState; protected: + static uint64_t AddWithCarry(uint32_t N, uint64_t x, uint64_t y, bool carry_in, + EmulateInstructionARM64::ProcState &proc_state); + typedef struct { uint32_t mask; uint32_t value; Index: lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp =================================================================== --- lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp +++ lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp @@ -8,8 +8,6 @@ #include "EmulateInstructionARM64.h" -#include <stdlib.h> - #include "lldb/Core/Address.h" #include "lldb/Core/PluginManager.h" #include "lldb/Symbol/UnwindPlan.h" @@ -18,10 +16,14 @@ #include "lldb/Utility/RegisterValue.h" #include "lldb/Utility/Stream.h" +#include "llvm/Support/CheckedArithmetic.h" + #include "Plugins/Process/Utility/ARMDefines.h" #include "Plugins/Process/Utility/ARMUtils.h" #include "Plugins/Process/Utility/lldb-arm64-register-enums.h" +#include <cstdlib> + #define GPR_OFFSET(idx) ((idx)*8) #define GPR_OFFSET_NAME(reg) 0 #define FPU_OFFSET(idx) ((idx)*16) @@ -85,23 +87,6 @@ return x << shift; } -// AddWithCarry() -// =============== -static inline uint64_t -AddWithCarry(uint32_t N, uint64_t x, uint64_t y, bit carry_in, - EmulateInstructionARM64::ProcState &proc_state) { - uint64_t unsigned_sum = UInt(x) + UInt(y) + UInt(carry_in); - int64_t signed_sum = SInt(x) + SInt(y) + UInt(carry_in); - uint64_t result = unsigned_sum; - if (N < 64) - result = Bits64(result, N - 1, 0); - proc_state.N = Bit64(result, N - 1); - proc_state.Z = IsZero(result); - proc_state.C = UInt(result) == unsigned_sum; - proc_state.V = SInt(result) == signed_sum; - return result; -} - // ConstrainUnpredictable() // ======================== @@ -582,6 +567,24 @@ return result; } +uint64_t EmulateInstructionARM64:: +AddWithCarry(uint32_t N, uint64_t x, uint64_t y, bit carry_in, + EmulateInstructionARM64::ProcState &proc_state) { + uint64_t unsigned_sum = UInt(x) + UInt(y) + UInt(carry_in); + llvm::Optional<int64_t> signed_sum = llvm::checkedAdd(SInt(x), SInt(y)); + bool overflow = !signed_sum; + if (!overflow) + overflow |= !llvm::checkedAdd(*signed_sum, SInt(carry_in)); + uint64_t result = unsigned_sum; + if (N < 64) + result = Bits64(result, N - 1, 0); + proc_state.N = Bit64(result, N - 1); + proc_state.Z = IsZero(result); + proc_state.C = UInt(result) != unsigned_sum; + proc_state.V = overflow; + return result; +} + bool EmulateInstructionARM64::EmulateADDSUBImm(const uint32_t opcode) { // integer d = UInt(Rd); // integer n = UInt(Rn);
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
