[llvm] r334634 - Change checked arithmetic functions API to return Optional

George Karpenkov via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 13 11:31:43 PDT 2018


Author: george.karpenkov
Date: Wed Jun 13 11:31:43 2018
New Revision: 334634

URL: http://llvm.org/viewvc/llvm-project?rev=334634&view=rev
Log:
Change checked arithmetic functions API to return Optional

Returning optional is much safer.
The previous API had potential to cause use of undefined variables, if
the value passed by pointer was accidentally read afterwards.

Differential Revision: https://reviews.llvm.org/D48137

Modified:
    llvm/trunk/include/llvm/Support/CheckedArithmetic.h
    llvm/trunk/unittests/Support/CheckedArithmeticTest.cpp

Modified: llvm/trunk/include/llvm/Support/CheckedArithmetic.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/CheckedArithmetic.h?rev=334634&r1=334633&r2=334634&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/CheckedArithmetic.h (original)
+++ llvm/trunk/include/llvm/Support/CheckedArithmetic.h Wed Jun 13 11:31:43 2018
@@ -16,66 +16,61 @@
 #define LLVM_SUPPORT_CHECKEDARITHMETIC_H
 
 #include "llvm/ADT/APInt.h"
+#include "llvm/ADT/Optional.h"
 
 #include <type_traits>
 
 namespace {
 
 /// Utility function to apply a given method of \c APInt \p F to \p LHS and
-/// \p RHS, and write the output into \p Res.
-/// \return Whether the operation overflows.
+/// \p RHS.
+/// \return Empty optional if the operation overflows, or result otherwise.
 template <typename T, typename F>
 typename std::enable_if<std::is_integral<T>::value && sizeof(T) * 8 <= 64,
-                        bool>::type
-checkedOp(T LHS, T RHS, F Op, T *Res = nullptr, bool Signed = true) {
+                        llvm::Optional<T>>::type
+checkedOp(T LHS, T RHS, F Op, bool Signed = true) {
   llvm::APInt ALHS(/*BitSize=*/sizeof(T) * 8, LHS, Signed);
   llvm::APInt ARHS(/*BitSize=*/sizeof(T) * 8, RHS, Signed);
   bool Overflow;
   llvm::APInt Out = (ALHS.*Op)(ARHS, Overflow);
-  if (Res)
-    *Res = Signed ? Out.getSExtValue() : Out.getZExtValue();
-  return Overflow;
+  if (Overflow)
+    return llvm::None;
+  return Signed ? Out.getSExtValue() : Out.getZExtValue();
 }
 }
 
 namespace llvm {
 
-/// Add two signed integers \p LHS and \p RHS, write into \p Res if non-null.
-/// Does not guarantee saturating arithmetic.
-/// \return Whether the result overflows.
+/// Add two signed integers \p LHS and \p RHS, return wrapped result if
+/// available.
 template <typename T>
-typename std::enable_if<std::is_signed<T>::value, bool>::type
-checkedAdd(T LHS, T RHS, T *Res = nullptr) {
-  return checkedOp(LHS, RHS, &llvm::APInt::sadd_ov, Res);
+typename std::enable_if<std::is_signed<T>::value, llvm::Optional<T>>::type
+checkedAdd(T LHS, T RHS) {
+  return checkedOp(LHS, RHS, &llvm::APInt::sadd_ov);
 }
 
-/// Multiply two signed integers \p LHS and \p RHS, write into \p Res if
-/// non-null.
-/// Does not guarantee saturating arithmetic.
-/// \return Whether the result overflows.
+/// Multiply two signed integers \p LHS and \p RHS, return wrapped result
+/// if available.
 template <typename T>
-typename std::enable_if<std::is_signed<T>::value, bool>::type
-checkedMul(T LHS, T RHS, T *Res = nullptr) {
-  return checkedOp(LHS, RHS, &llvm::APInt::smul_ov, Res);
+typename std::enable_if<std::is_signed<T>::value, llvm::Optional<T>>::type
+checkedMul(T LHS, T RHS) {
+  return checkedOp(LHS, RHS, &llvm::APInt::smul_ov);
 }
 
-/// Add two unsigned integers \p LHS and \p RHS, write into \p Res if non-null.
-/// Does not guarantee saturating arithmetic.
-/// \return Whether the result overflows.
+/// Add two unsigned integers \p LHS and \p RHS, return wrapped result
+/// if available.
 template <typename T>
-typename std::enable_if<std::is_unsigned<T>::value, bool>::type
-checkedAddUnsigned(T LHS, T RHS, T *Res = nullptr) {
-  return checkedOp(LHS, RHS, &llvm::APInt::uadd_ov, Res, /*Signed=*/false);
+typename std::enable_if<std::is_unsigned<T>::value, llvm::Optional<T>>::type
+checkedAddUnsigned(T LHS, T RHS) {
+  return checkedOp(LHS, RHS, &llvm::APInt::uadd_ov, /*Signed=*/false);
 }
 
-/// Multiply two unsigned integers \p LHS and \p RHS, write into \p Res if
-/// non-null.
-/// Does not guarantee saturating arithmetic.
-/// \return Whether the result overflows.
+/// Multiply two unsigned integers \p LHS and \p RHS, return wrapped result
+/// if available.
 template <typename T>
-typename std::enable_if<std::is_unsigned<T>::value, bool>::type
-checkedMulUnsigned(T LHS, T RHS, T *Res = nullptr) {
-  return checkedOp(LHS, RHS, &llvm::APInt::umul_ov, Res, /*Signed=*/false);
+typename std::enable_if<std::is_unsigned<T>::value, llvm::Optional<T>>::type
+checkedMulUnsigned(T LHS, T RHS) {
+  return checkedOp(LHS, RHS, &llvm::APInt::umul_ov, /*Signed=*/false);
 }
 
 } // End llvm namespace

Modified: llvm/trunk/unittests/Support/CheckedArithmeticTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/CheckedArithmeticTest.cpp?rev=334634&r1=334633&r2=334634&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/CheckedArithmeticTest.cpp (original)
+++ llvm/trunk/unittests/Support/CheckedArithmeticTest.cpp Wed Jun 13 11:31:43 2018
@@ -6,65 +6,53 @@ using namespace llvm;
 namespace {
 
 TEST(CheckedArithmetic, CheckedAdd) {
-  int64_t Out;
   const int64_t Max = std::numeric_limits<int64_t>::max();
   const int64_t Min = std::numeric_limits<int64_t>::min();
-  EXPECT_EQ(checkedAdd<int64_t>(Max, Max, &Out), true);
-  EXPECT_EQ(checkedAdd<int64_t>(Min, -1, &Out), true);
-  EXPECT_EQ(checkedAdd<int64_t>(Max, 1, &Out), true);
-  EXPECT_EQ(checkedAdd<int64_t>(10, 1, &Out), false);
-  EXPECT_EQ(Out, 11);
+  EXPECT_EQ(checkedAdd<int64_t>(Max, Max), None);
+  EXPECT_EQ(checkedAdd<int64_t>(Min, -1), None);
+  EXPECT_EQ(checkedAdd<int64_t>(Max, 1), None);
+  EXPECT_EQ(checkedAdd<int64_t>(10, 1), Optional<int64_t>(11));
 }
 
 TEST(CheckedArithmetic, CheckedAddSmall) {
-  int16_t Out;
   const int16_t Max = std::numeric_limits<int16_t>::max();
   const int16_t Min = std::numeric_limits<int16_t>::min();
-  EXPECT_EQ(checkedAdd<int16_t>(Max, Max, &Out), true);
-  EXPECT_EQ(checkedAdd<int16_t>(Min, -1, &Out), true);
-  EXPECT_EQ(checkedAdd<int16_t>(Max, 1, &Out), true);
-  EXPECT_EQ(checkedAdd<int16_t>(10, 1, &Out), false);
-  EXPECT_EQ(Out, 11);
+  EXPECT_EQ(checkedAdd<int16_t>(Max, Max), None);
+  EXPECT_EQ(checkedAdd<int16_t>(Min, -1), None);
+  EXPECT_EQ(checkedAdd<int16_t>(Max, 1), None);
+  EXPECT_EQ(checkedAdd<int16_t>(10, 1), Optional<int64_t>(11));
 }
 
 TEST(CheckedArithmetic, CheckedMul) {
-  int64_t Out;
   const int64_t Max = std::numeric_limits<int64_t>::max();
   const int64_t Min = std::numeric_limits<int64_t>::min();
-  EXPECT_EQ(checkedMul<int64_t>(Max, 2, &Out), true);
-  EXPECT_EQ(checkedMul<int64_t>(Max, Max, &Out), true);
-  EXPECT_EQ(checkedMul<int64_t>(Min, 2, &Out), true);
-  EXPECT_EQ(checkedMul<int64_t>(10, 2, &Out), false);
-  EXPECT_EQ(Out, 20);
+  EXPECT_EQ(checkedMul<int64_t>(Max, 2), None);
+  EXPECT_EQ(checkedMul<int64_t>(Max, Max), None);
+  EXPECT_EQ(checkedMul<int64_t>(Min, 2), None);
+  EXPECT_EQ(checkedMul<int64_t>(10, 2), Optional<int64_t>(20));
 }
 
 TEST(CheckedArithmetic, CheckedMulSmall) {
-  int16_t Out;
   const int16_t Max = std::numeric_limits<int16_t>::max();
   const int16_t Min = std::numeric_limits<int16_t>::min();
-  EXPECT_EQ(checkedMul<int16_t>(Max, 2, &Out), true);
-  EXPECT_EQ(checkedMul<int16_t>(Max, Max, &Out), true);
-  EXPECT_EQ(checkedMul<int16_t>(Min, 2, &Out), true);
-  EXPECT_EQ(checkedMul<int16_t>(10, 2, &Out), false);
-  EXPECT_EQ(Out, 20);
+  EXPECT_EQ(checkedMul<int16_t>(Max, 2), None);
+  EXPECT_EQ(checkedMul<int16_t>(Max, Max), None);
+  EXPECT_EQ(checkedMul<int16_t>(Min, 2), None);
+  EXPECT_EQ(checkedMul<int16_t>(10, 2), Optional<int16_t>(20));
 }
 
 TEST(CheckedArithmetic, CheckedAddUnsigned) {
-  uint64_t Out;
   const uint64_t Max = std::numeric_limits<uint64_t>::max();
-  EXPECT_EQ(checkedAddUnsigned<uint64_t>(Max, Max, &Out), true);
-  EXPECT_EQ(checkedAddUnsigned<uint64_t>(Max, 1, &Out), true);
-  EXPECT_EQ(checkedAddUnsigned<uint64_t>(10, 1, &Out), false);
-  EXPECT_EQ(Out, uint64_t(11));
+  EXPECT_EQ(checkedAddUnsigned<uint64_t>(Max, Max), None);
+  EXPECT_EQ(checkedAddUnsigned<uint64_t>(Max, 1), None);
+  EXPECT_EQ(checkedAddUnsigned<uint64_t>(10, 1), Optional<uint64_t>(11));
 }
 
 TEST(CheckedArithmetic, CheckedMulUnsigned) {
-  uint64_t Out;
   const uint64_t Max = std::numeric_limits<uint64_t>::max();
-  EXPECT_EQ(checkedMulUnsigned<uint64_t>(Max, 2, &Out), true);
-  EXPECT_EQ(checkedMulUnsigned<uint64_t>(Max, Max, &Out), true);
-  EXPECT_EQ(checkedMulUnsigned<uint64_t>(10, 2, &Out), false);
-  EXPECT_EQ(Out, uint64_t(20));
+  EXPECT_EQ(checkedMulUnsigned<uint64_t>(Max, 2), llvm::None);
+  EXPECT_EQ(checkedMulUnsigned<uint64_t>(Max, Max), llvm::None);
+  EXPECT_EQ(checkedMulUnsigned<uint64_t>(10, 2), Optional<uint64_t>(20));
 }
 
 




More information about the llvm-commits mailing list