Commit 8a35265a authored by Clemens Hammacher's avatar Clemens Hammacher Committed by Commit Bot

[base] Remove safe_math headers

We only use the safe math helpers (CheckedNumeric<T>) in very few
places. The headers are huge though, and complex. They are pulled in to
839 of our object files, increasing compilation time.

I also find the implicit checks more easy to understand than the complex
logic in CheckedNumeric.

Thus, this CL removes the safe_math headers and implements bounds
checks for the five uses explicitly.

R=jkummerow@chromium.org, mlippautz@chromium.org

Bug: v8:8834
Change-Id: I2d60f95799ee61cfa161354428605f67829cd736
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1547651Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60630}
parent ef2eb933
...@@ -3305,8 +3305,6 @@ v8_component("v8_libbase") { ...@@ -3305,8 +3305,6 @@ v8_component("v8_libbase") {
"src/base/ring-buffer.h", "src/base/ring-buffer.h",
"src/base/safe_conversions.h", "src/base/safe_conversions.h",
"src/base/safe_conversions_impl.h", "src/base/safe_conversions_impl.h",
"src/base/safe_math.h",
"src/base/safe_math_impl.h",
"src/base/small-vector.h", "src/base/small-vector.h",
"src/base/sys-info.cc", "src/base/sys-info.cc",
"src/base/sys-info.h", "src/base/sys-info.h",
......
...@@ -7,7 +7,6 @@ ...@@ -7,7 +7,6 @@
#include <limits> #include <limits>
#include "src/base/logging.h" #include "src/base/logging.h"
#include "src/base/safe_math.h"
namespace v8 { namespace v8 {
namespace base { namespace base {
...@@ -71,49 +70,30 @@ int32_t SignedMod32(int32_t lhs, int32_t rhs) { ...@@ -71,49 +70,30 @@ int32_t SignedMod32(int32_t lhs, int32_t rhs) {
return lhs % rhs; return lhs % rhs;
} }
int64_t FromCheckedNumeric(const internal::CheckedNumeric<int64_t> value) {
if (value.IsValid())
return value.ValueUnsafe();
// We could return max/min but we don't really expose what the maximum delta
// is. Instead, return max/(-max), which is something that clients can reason
// about.
// TODO(rvargas) crbug.com/332611: don't use internal values.
int64_t limit = std::numeric_limits<int64_t>::max();
if (value.validity() == internal::RANGE_UNDERFLOW)
limit = -limit;
return value.ValueOrDefault(limit);
}
int64_t SignedSaturatedAdd64(int64_t lhs, int64_t rhs) { int64_t SignedSaturatedAdd64(int64_t lhs, int64_t rhs) {
internal::CheckedNumeric<int64_t> rv(lhs); using limits = std::numeric_limits<int64_t>;
rv += rhs; // Underflow if {lhs + rhs < min}. In that case, return {min}.
return FromCheckedNumeric(rv); if (rhs < 0 && lhs < limits::min() - rhs) return limits::min();
// Overflow if {lhs + rhs > max}. In that case, return {max}.
if (rhs >= 0 && lhs > limits::max() - rhs) return limits::max();
return lhs + rhs;
} }
int64_t SignedSaturatedSub64(int64_t lhs, int64_t rhs) { int64_t SignedSaturatedSub64(int64_t lhs, int64_t rhs) {
internal::CheckedNumeric<int64_t> rv(lhs); using limits = std::numeric_limits<int64_t>;
rv -= rhs; // Underflow if {lhs - rhs < min}. In that case, return {min}.
return FromCheckedNumeric(rv); if (rhs > 0 && lhs < limits::min() + rhs) return limits::min();
// Overflow if {lhs - rhs > max}. In that case, return {max}.
if (rhs <= 0 && lhs > limits::max() + rhs) return limits::max();
return lhs - rhs;
} }
bool SignedMulOverflow32(int32_t lhs, int32_t rhs, int32_t* val) { bool SignedMulOverflow32(int32_t lhs, int32_t rhs, int32_t* val) {
internal::CheckedNumeric<int32_t> rv(lhs); // Compute the result as {int64_t}, then check for overflow.
rv *= rhs; int64_t result = int64_t{lhs} * int64_t{rhs};
int32_t limit = std::numeric_limits<int32_t>::max(); *val = static_cast<int32_t>(result);
*val = rv.ValueOrDefault(limit); using limits = std::numeric_limits<int32_t>;
return !rv.IsValid(); return result < limits::min() || result > limits::max();
}
bool SignedMulOverflow64(int64_t lhs, int64_t rhs, int64_t* val) {
internal::CheckedNumeric<int64_t> rv(lhs);
rv *= rhs;
int64_t limit = std::numeric_limits<int64_t>::max();
*val = rv.ValueOrDefault(limit);
return !rv.IsValid();
} }
} // namespace bits } // namespace bits
......
...@@ -19,12 +19,6 @@ ...@@ -19,12 +19,6 @@
namespace v8 { namespace v8 {
namespace base { namespace base {
namespace internal {
template <typename T>
class CheckedNumeric;
}
namespace bits { namespace bits {
// CountPopulation(value) returns the number of bits set in |value|. // CountPopulation(value) returns the number of bits set in |value|.
...@@ -242,11 +236,6 @@ inline bool SignedSubOverflow64(int64_t lhs, int64_t rhs, int64_t* val) { ...@@ -242,11 +236,6 @@ inline bool SignedSubOverflow64(int64_t lhs, int64_t rhs, int64_t* val) {
return ((res ^ lhs) & (res ^ ~rhs) & (1ULL << 63)) != 0; return ((res ^ lhs) & (res ^ ~rhs) & (1ULL << 63)) != 0;
} }
// SignedMulOverflow64(lhs,rhs,val) performs a signed multiplication of |lhs|
// and |rhs| and stores the result into the variable pointed to by |val| and
// returns true if the signed multiplication resulted in an overflow.
V8_BASE_EXPORT bool SignedMulOverflow64(int64_t lhs, int64_t rhs, int64_t* val);
// SignedMulHigh32(lhs, rhs) multiplies two signed 32-bit values |lhs| and // SignedMulHigh32(lhs, rhs) multiplies two signed 32-bit values |lhs| and
// |rhs|, extracts the most significant 32 bits of the result, and returns // |rhs|, extracts the most significant 32 bits of the result, and returns
// those. // those.
...@@ -295,10 +284,6 @@ inline uint32_t UnsignedMod32(uint32_t lhs, uint32_t rhs) { ...@@ -295,10 +284,6 @@ inline uint32_t UnsignedMod32(uint32_t lhs, uint32_t rhs) {
} }
// Clamp |value| on overflow and underflow conditions.
V8_BASE_EXPORT int64_t
FromCheckedNumeric(const internal::CheckedNumeric<int64_t> value);
// SignedSaturatedAdd64(lhs, rhs) adds |lhs| and |rhs|, // SignedSaturatedAdd64(lhs, rhs) adds |lhs| and |rhs|,
// checks and returns the result. // checks and returns the result.
V8_BASE_EXPORT int64_t SignedSaturatedAdd64(int64_t lhs, int64_t rhs); V8_BASE_EXPORT int64_t SignedSaturatedAdd64(int64_t lhs, int64_t rhs);
......
...@@ -39,13 +39,23 @@ int64_t ComputeThreadTicks() { ...@@ -39,13 +39,23 @@ int64_t ComputeThreadTicks() {
&thread_info_count); &thread_info_count);
CHECK_EQ(kr, KERN_SUCCESS); CHECK_EQ(kr, KERN_SUCCESS);
v8::base::CheckedNumeric<int64_t> absolute_micros( // We can add the seconds into a {int64_t} without overflow.
thread_info_data.user_time.seconds + CHECK_LE(thread_info_data.user_time.seconds,
thread_info_data.system_time.seconds); std::numeric_limits<int64_t>::max() -
absolute_micros *= v8::base::Time::kMicrosecondsPerSecond; thread_info_data.system_time.seconds);
absolute_micros += (thread_info_data.user_time.microseconds + int64_t seconds =
thread_info_data.system_time.microseconds); thread_info_data.user_time.seconds + thread_info_data.system_time.seconds;
return absolute_micros.ValueOrDie(); // Multiplying the seconds by {kMicrosecondsPerSecond}, and adding something
// in [0, 2 * kMicrosecondsPerSecond) must result in a valid {int64_t}.
static constexpr int64_t kSecondsLimit =
(std::numeric_limits<int64_t>::max() /
v8::base::Time::kMicrosecondsPerSecond) -
2;
CHECK_GT(kSecondsLimit, seconds);
int64_t micros = seconds * v8::base::Time::kMicrosecondsPerSecond;
micros += (thread_info_data.user_time.microseconds +
thread_info_data.system_time.microseconds);
return micros;
} }
#elif V8_OS_POSIX #elif V8_OS_POSIX
// Helper function to get results from clock_gettime() and convert to a // Helper function to get results from clock_gettime() and convert to a
...@@ -69,8 +79,14 @@ V8_INLINE int64_t ClockNow(clockid_t clk_id) { ...@@ -69,8 +79,14 @@ V8_INLINE int64_t ClockNow(clockid_t clk_id) {
if (clock_gettime(clk_id, &ts) != 0) { if (clock_gettime(clk_id, &ts) != 0) {
UNREACHABLE(); UNREACHABLE();
} }
v8::base::internal::CheckedNumeric<int64_t> result(ts.tv_sec); // Multiplying the seconds by {kMicrosecondsPerSecond}, and adding something
result *= v8::base::Time::kMicrosecondsPerSecond; // in [0, kMicrosecondsPerSecond) must result in a valid {int64_t}.
static constexpr int64_t kSecondsLimit =
(std::numeric_limits<int64_t>::max() /
v8::base::Time::kMicrosecondsPerSecond) -
1;
CHECK_GT(kSecondsLimit, ts.tv_sec);
int64_t result = int64_t{ts.tv_sec} * v8::base::Time::kMicrosecondsPerSecond;
#if defined(V8_OS_AIX) #if defined(V8_OS_AIX)
if (clk_id == CLOCK_THREAD_CPUTIME_ID) { if (clk_id == CLOCK_THREAD_CPUTIME_ID) {
result += (tc.stime / v8::base::Time::kNanosecondsPerMicrosecond); result += (tc.stime / v8::base::Time::kNanosecondsPerMicrosecond);
...@@ -80,7 +96,7 @@ V8_INLINE int64_t ClockNow(clockid_t clk_id) { ...@@ -80,7 +96,7 @@ V8_INLINE int64_t ClockNow(clockid_t clk_id) {
#else #else
result += (ts.tv_nsec / v8::base::Time::kNanosecondsPerMicrosecond); result += (ts.tv_nsec / v8::base::Time::kNanosecondsPerMicrosecond);
#endif #endif
return result.ValueOrDie(); return result;
#else // Monotonic clock not supported. #else // Monotonic clock not supported.
return 0; return 0;
#endif #endif
......
...@@ -14,7 +14,6 @@ ...@@ -14,7 +14,6 @@
#include "src/base/base-export.h" #include "src/base/base-export.h"
#include "src/base/bits.h" #include "src/base/bits.h"
#include "src/base/macros.h" #include "src/base/macros.h"
#include "src/base/safe_math.h"
#if V8_OS_WIN #if V8_OS_WIN
#include "src/base/win32-headers.h" #include "src/base/win32-headers.h"
#endif #endif
......
This diff is collapsed.
This diff is collapsed.
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment