[Lldb-commits] [lldb] a0b674f - Fix UB in EmulateInstructionARM64.cpp
Adrian Prantl via lldb-commits
lldb-commits at lists.llvm.org
Mon Jun 1 18:14:16 PDT 2020
Author: Adrian Prantl
Date: 2020-06-01T18:11:50-07:00
New Revision: a0b674fd7f06b86241cf19387313b508248a3868
URL: https://github.com/llvm/llvm-project/commit/a0b674fd7f06b86241cf19387313b508248a3868
DIFF: https://github.com/llvm/llvm-project/commit/a0b674fd7f06b86241cf19387313b508248a3868.diff
LOG: Fix UB in EmulateInstructionARM64.cpp
This fixes an unhandled signed integer overflow in AddWithCarry() by
using the llvm::checkedAdd() function. Thats to Vedant Kumar for the
suggestion!
<rdar://problem/60926115>
Differential Revision: https://reviews.llvm.org/D80955
Added:
lldb/unittests/Instruction/CMakeLists.txt
lldb/unittests/Instruction/TestAArch64Emulator.cpp
Modified:
lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h
lldb/unittests/CMakeLists.txt
Removed:
################################################################################
diff --git a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
index dcf41444e48f..35fb51759ca8 100644
--- a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
+++ b/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 @@ static inline uint64_t LSL(uint64_t x, integer shift) {
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 @@ bool EmulateInstructionARM64::ConditionHolds(const uint32_t cond) {
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);
diff --git a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h
index 4841e813d89e..11ad8a99b0fc 100644
--- a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h
+++ b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h
@@ -152,6 +152,9 @@ class EmulateInstructionARM64 : public lldb_private::EmulateInstruction {
} 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;
diff --git a/lldb/unittests/CMakeLists.txt b/lldb/unittests/CMakeLists.txt
index 42aa2c2d7567..33818b5888c0 100644
--- a/lldb/unittests/CMakeLists.txt
+++ b/lldb/unittests/CMakeLists.txt
@@ -71,6 +71,7 @@ add_subdirectory(Editline)
add_subdirectory(Expression)
add_subdirectory(Host)
add_subdirectory(Interpreter)
+add_subdirectory(Instruction)
add_subdirectory(Language)
add_subdirectory(ObjectFile)
add_subdirectory(Platform)
diff --git a/lldb/unittests/Instruction/CMakeLists.txt b/lldb/unittests/Instruction/CMakeLists.txt
new file mode 100644
index 000000000000..63d829831023
--- /dev/null
+++ b/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()
diff --git a/lldb/unittests/Instruction/TestAArch64Emulator.cpp b/lldb/unittests/Instruction/TestAArch64Emulator.cpp
new file mode 100644
index 000000000000..4506c200dee3
--- /dev/null
+++ b/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);
+}
More information about the lldb-commits
mailing list