[Lldb-commits] [PATCH] D80955: Fix UB in EmulateInstructionARM64.cpp

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: jasonmolenda, vsk.
Herald added subscribers: danielkiss, kristof.beyls.

This fixes an unhandled signed integer overflow in AddWithCarry() by using the 
llvm::checkedAdd() function. Thats to @vsk for the suggestion!


https://reviews.llvm.org/D80955

Files:
  lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp


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 <stdlib.h>
+
 #define GPR_OFFSET(idx) ((idx)*8)
 #define GPR_OFFSET_NAME(reg) 0
 #define FPU_OFFSET(idx) ((idx)*16)
@@ -91,14 +93,17 @@
 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);
+  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 = SInt(result) == signed_sum;
+  proc_state.V = !overflow;
   return result;
 }
 



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 <stdlib.h>
+
 #define GPR_OFFSET(idx) ((idx)*8)
 #define GPR_OFFSET_NAME(reg) 0
 #define FPU_OFFSET(idx) ((idx)*16)
@@ -91,14 +93,17 @@
 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);
+  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 = SInt(result) == signed_sum;
+  proc_state.V = !overflow;
   return result;
 }
 

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits