Merge pull request #1305 from adrian-prantl/60926115

Fix UB in EmulateInstructionARM64.cpp

Author: fredriss

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)
@@ -83,23 +85,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()
 // ========================
 
@@ -588,6 +573,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);

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;

lldb/unittests/CMakeLists.txt

@@ -67,6 +67,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)

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()

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);
+}