Commit fc329ce2 authored by Jakob Kummerow's avatar Jakob Kummerow Committed by Commit Bot

[ubsan] Fix various cases of undefined behavior

Mostly signed integer overflows, and a few cases of double
division by zero (which is defined by IEEE-754 to return
Infinity (or NaN for 0/0) but is UB in C++).
In base/ieee754.cc, use constants for NaN and Infinity instead
of computing these values.
In spaces-unittest.cc, ensure that a large enough allocation
is used.

Bug: v8:3770
Change-Id: I50d9a77dc860ef9993b7b269a5f8c117b0f62f9d
Reviewed-on: https://chromium-review.googlesource.com/c/1403454
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: 's avatarAdam Klein <adamk@chromium.org>
Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58701}
parent 6733e948
......@@ -61,7 +61,7 @@ int32_t SignedMulHighAndAdd32(int32_t lhs, int32_t rhs, int32_t acc) {
int32_t SignedDiv32(int32_t lhs, int32_t rhs) {
if (rhs == 0) return 0;
if (rhs == -1) return -lhs;
if (rhs == -1) return lhs == std::numeric_limits<int32_t>::min() ? lhs : -lhs;
return lhs / rhs;
}
......
......@@ -20,6 +20,7 @@
#include "src/base/build_config.h"
#include "src/base/macros.h"
#include "src/base/overflowing-math.h"
namespace v8 {
namespace base {
......@@ -945,7 +946,7 @@ double acos(double x) {
else
return pi + 2.0 * pio2_lo; /* acos(-1)= pi */
}
return (x - x) / (x - x); /* acos(|x|>1) is NaN */
return std::numeric_limits<double>::signaling_NaN(); // acos(|x|>1) is NaN
}
if (ix < 0x3FE00000) { /* |x| < 0.5 */
if (ix <= 0x3C600000) return pio2_hi + pio2_lo; /*if|x|<2**-57*/
......@@ -998,7 +999,7 @@ double acosh(double x) {
uint32_t lx;
EXTRACT_WORDS(hx, lx, x);
if (hx < 0x3FF00000) { /* x < 1 */
return divide(x - x, x - x);
return std::numeric_limits<double>::signaling_NaN();
} else if (hx >= 0x41B00000) { /* x > 2**28 */
if (hx >= 0x7FF00000) { /* x is inf of NaN */
return x + x;
......@@ -1072,9 +1073,10 @@ double asin(double x) {
if (ix >= 0x3FF00000) { /* |x|>= 1 */
uint32_t lx;
GET_LOW_WORD(lx, x);
if (((ix - 0x3FF00000) | lx) == 0) /* asin(1)=+-pi/2 with inexact */
if (((ix - 0x3FF00000) | lx) == 0) { /* asin(1)=+-pi/2 with inexact */
return x * pio2_hi + x * pio2_lo;
return (x - x) / (x - x); /* asin(|x|>1) is NaN */
}
return std::numeric_limits<double>::signaling_NaN(); // asin(|x|>1) is NaN
} else if (ix < 0x3FE00000) { /* |x|<0.5 */
if (ix < 0x3E400000) { /* if |x| < 2**-27 */
if (huge + x > one) return x; /* return x with inexact if x!=0*/
......@@ -1298,11 +1300,13 @@ double atan2(double y, double x) {
ix = hx & 0x7FFFFFFF;
EXTRACT_WORDS(hy, ly, y);
iy = hy & 0x7FFFFFFF;
if (((ix | ((lx | -static_cast<int32_t>(lx)) >> 31)) > 0x7FF00000) ||
((iy | ((ly | -static_cast<int32_t>(ly)) >> 31)) > 0x7FF00000)) {
if (((ix | ((lx | NegateWithWraparound<int32_t>(lx)) >> 31)) > 0x7FF00000) ||
((iy | ((ly | NegateWithWraparound<int32_t>(ly)) >> 31)) > 0x7FF00000)) {
return x + y; /* x or y is NaN */
}
if (((hx - 0x3FF00000) | lx) == 0) return atan(y); /* x=1.0 */
if ((SubWithWraparound(hx, 0x3FF00000) | lx) == 0) {
return atan(y); /* x=1.0 */
}
m = ((hy >> 31) & 1) | ((hx >> 30) & 2); /* 2*sign(x)+sign(y) */
/* when y = 0 */
......@@ -1431,18 +1435,6 @@ double cos(double x) {
}
}
/* div(x, y)
* Returns the quotient x/y, avoiding C++ undefined behavior if y == 0.
*/
double divide(double x, double y) {
if (y != 0) return x / y;
if (x == 0) return std::numeric_limits<double>::quiet_NaN();
if ((x > 0) == (bit_cast<uint64_t>(y) == 0)) {
return std::numeric_limits<double>::infinity();
}
return -std::numeric_limits<double>::infinity();
}
/* exp(x)
* Returns the exponential of x.
*
......@@ -1621,9 +1613,14 @@ double atanh(double x) {
uint32_t lx;
EXTRACT_WORDS(hx, lx, x);
ix = hx & 0x7FFFFFFF;
if ((ix | ((lx | -static_cast<int32_t>(lx)) >> 31)) > 0x3FF00000) /* |x|>1 */
return (x - x) / (x - x);
if (ix == 0x3FF00000) return x / zero;
if ((ix | ((lx | NegateWithWraparound<int32_t>(lx)) >> 31)) > 0x3FF00000) {
/* |x|>1 */
return std::numeric_limits<double>::signaling_NaN();
}
if (ix == 0x3FF00000) {
return x > 0 ? std::numeric_limits<double>::infinity()
: -std::numeric_limits<double>::infinity();
}
if (ix < 0x3E300000 && (huge + x) > zero) return x; /* x<2**-28 */
SET_HIGH_WORD(x, ix);
if (ix < 0x3FE00000) { /* x < 0.5 */
......@@ -1702,7 +1699,6 @@ double log(double x) {
Lg7 = 1.479819860511658591e-01; /* 3FC2F112 DF3E5244 */
static const double zero = 0.0;
static volatile double vzero = 0.0;
double hfsq, f, s, z, R, w, t1, t2, dk;
int32_t k, hx, i, j;
......@@ -1712,9 +1708,12 @@ double log(double x) {
k = 0;
if (hx < 0x00100000) { /* x < 2**-1022 */
if (((hx & 0x7FFFFFFF) | lx) == 0)
return -two54 / vzero; /* log(+-0)=-inf */
if (hx < 0) return (x - x) / zero; /* log(-#) = NaN */
if (((hx & 0x7FFFFFFF) | lx) == 0) {
return -std::numeric_limits<double>::infinity(); /* log(+-0)=-inf */
}
if (hx < 0) {
return std::numeric_limits<double>::signaling_NaN(); /* log(-#) = NaN */
}
k -= 54;
x *= two54; /* subnormal number, scale up x */
GET_HIGH_WORD(hx, x);
......@@ -1845,7 +1844,6 @@ double log1p(double x) {
Lp7 = 1.479819860511658591e-01; /* 3FC2F112 DF3E5244 */
static const double zero = 0.0;
static volatile double vzero = 0.0;
double hfsq, f, c, s, z, R, u;
int32_t k, hx, hu, ax;
......@@ -1857,9 +1855,9 @@ double log1p(double x) {
if (hx < 0x3FDA827A) { /* 1+x < sqrt(2)+ */
if (ax >= 0x3FF00000) { /* x <= -1.0 */
if (x == -1.0)
return -two54 / vzero; /* log1p(-1)=+inf */
return -std::numeric_limits<double>::infinity(); /* log1p(-1)=+inf */
else
return (x - x) / (x - x); /* log1p(x<-1)=NaN */
return std::numeric_limits<double>::signaling_NaN(); // log1p(x<-1)=NaN
}
if (ax < 0x3E200000) { /* |x| < 2**-29 */
if (two54 + x > zero /* raise inexact */
......@@ -2028,9 +2026,6 @@ double log2(double x) {
ivln2hi = 1.44269504072144627571e+00, /* 0x3FF71547, 0x65200000 */
ivln2lo = 1.67517131648865118353e-10; /* 0x3DE705FC, 0x2EEFA200 */
static const double zero = 0.0;
static volatile double vzero = 0.0;
double f, hfsq, hi, lo, r, val_hi, val_lo, w, y;
int32_t i, k, hx;
uint32_t lx;
......@@ -2039,15 +2034,18 @@ double log2(double x) {
k = 0;
if (hx < 0x00100000) { /* x < 2**-1022 */
if (((hx & 0x7FFFFFFF) | lx) == 0)
return -two54 / vzero; /* log(+-0)=-inf */
if (hx < 0) return (x - x) / zero; /* log(-#) = NaN */
if (((hx & 0x7FFFFFFF) | lx) == 0) {
return -std::numeric_limits<double>::infinity(); /* log(+-0)=-inf */
}
if (hx < 0) {
return std::numeric_limits<double>::signaling_NaN(); /* log(-#) = NaN */
}
k -= 54;
x *= two54; /* subnormal number, scale up x */
GET_HIGH_WORD(hx, x);
}
if (hx >= 0x7FF00000) return x + x;
if (hx == 0x3FF00000 && lx == 0) return zero; /* log(1) = +0 */
if (hx == 0x3FF00000 && lx == 0) return 0.0; /* log(1) = +0 */
k += (hx >> 20) - 1023;
hx &= 0x000FFFFF;
i = (hx + 0x95F64) & 0x100000;
......@@ -2135,9 +2133,6 @@ double log10(double x) {
log10_2hi = 3.01029995663611771306e-01, /* 0x3FD34413, 0x509F6000 */
log10_2lo = 3.69423907715893078616e-13; /* 0x3D59FEF3, 0x11F12B36 */
static const double zero = 0.0;
static volatile double vzero = 0.0;
double y;
int32_t i, k, hx;
uint32_t lx;
......@@ -2146,16 +2141,19 @@ double log10(double x) {
k = 0;
if (hx < 0x00100000) { /* x < 2**-1022 */
if (((hx & 0x7FFFFFFF) | lx) == 0)
return -two54 / vzero; /* log(+-0)=-inf */
if (hx < 0) return (x - x) / zero; /* log(-#) = NaN */
if (((hx & 0x7FFFFFFF) | lx) == 0) {
return -std::numeric_limits<double>::infinity(); /* log(+-0)=-inf */
}
if (hx < 0) {
return std::numeric_limits<double>::quiet_NaN(); /* log(-#) = NaN */
}
k -= 54;
x *= two54; /* subnormal number, scale up x */
GET_HIGH_WORD(hx, x);
GET_LOW_WORD(lx, x);
}
if (hx >= 0x7FF00000) return x + x;
if (hx == 0x3FF00000 && lx == 0) return zero; /* log(1) = +0 */
if (hx == 0x3FF00000 && lx == 0) return 0.0; /* log(1) = +0 */
k += (hx >> 20) - 1023;
i = (k & 0x80000000) >> 31;
......
......@@ -36,9 +36,6 @@ V8_BASE_EXPORT double atan2(double y, double x);
// Returns the cosine of |x|, where |x| is given in radians.
V8_BASE_EXPORT double cos(double x);
// Returns the quotient of x and y, avoiding C++ undefined behavior if y == 0.
V8_BASE_EXPORT double divide(double x, double y);
// Returns the base-e exponential of |x|.
V8_BASE_EXPORT double exp(double x);
......
......@@ -91,7 +91,7 @@ int RandomNumberGenerator::NextInt(int max) {
while (true) {
int rnd = Next(31);
int val = rnd % max;
if (rnd - val + (max - 1) >= 0) {
if (std::numeric_limits<int>::max() - (rnd - val) >= (max - 1)) {
return val;
}
}
......
......@@ -72,7 +72,7 @@ inline double DoubleToInteger(double x) {
return (x >= 0) ? std::floor(x) : std::ceil(x);
}
// Implements most of https://tc39.github.io/ecma262/#sec-toint32.
int32_t DoubleToInt32(double x) {
if ((std::isfinite(x)) && (x <= INT_MAX) && (x >= INT_MIN)) {
int32_t i = static_cast<int32_t>(x);
......@@ -80,13 +80,18 @@ int32_t DoubleToInt32(double x) {
}
Double d(x);
int exponent = d.Exponent();
uint64_t bits;
if (exponent < 0) {
if (exponent <= -Double::kSignificandSize) return 0;
return d.Sign() * static_cast<int32_t>(d.Significand() >> -exponent);
bits = d.Significand() >> -exponent;
} else {
if (exponent > 31) return 0;
return d.Sign() * static_cast<int32_t>(d.Significand() << exponent);
// Masking to a 32-bit value ensures that the result of the
// static_cast<int64_t> below is not the minimal int64_t value,
// which would overflow on multiplication with d.Sign().
bits = (d.Significand() << exponent) & 0xFFFFFFFFul;
}
return static_cast<int32_t>(d.Sign() * static_cast<int64_t>(bits));
}
bool DoubleToSmiInteger(double value, int* smi_int_value) {
......
......@@ -4,6 +4,7 @@
#include "src/date.h"
#include "src/base/overflowing-math.h"
#include "src/conversions.h"
#include "src/objects-inl.h"
#ifdef V8_INTL_SUPPORT
......@@ -283,7 +284,8 @@ int DateCache::GetLocalOffsetFromOS(int64_t time_ms, bool is_utc) {
void DateCache::ExtendTheAfterSegment(int time_sec, int offset_ms) {
if (after_->offset_ms == offset_ms &&
after_->start_sec <= time_sec + kDefaultDSTDeltaInSec &&
after_->start_sec <=
base::AddWithWraparound(time_sec, kDefaultDSTDeltaInSec) &&
time_sec <= after_->end_sec) {
// Extend the after_ segment.
after_->start_sec = time_sec;
......
......@@ -240,7 +240,7 @@ static void FillFractionals(uint64_t fractionals, int exponent,
fractionals -= static_cast<uint64_t>(digit) << point;
}
// If the first bit after the point is set we have to round up.
if (((fractionals >> (point - 1)) & 1) == 1) {
if (point > 0 && ((fractionals >> (point - 1)) & 1) == 1) {
DtoaRoundUp(buffer, length, decimal_point);
}
} else { // We need 128 bits.
......
......@@ -636,17 +636,20 @@ Handle<Object> JsonParser<seq_one_byte>::ParseJsonNumber() {
// a decimal point or exponent.
if (IsDecimalDigit(c0_)) return ReportUnexpectedCharacter();
} else {
int i = 0;
uint32_t i = 0;
int digits = 0;
if (c0_ < '1' || c0_ > '9') return ReportUnexpectedCharacter();
do {
// This can overflow. That's OK, the "digits < 10" check below
// will discard overflown results.
i = i * 10 + c0_ - '0';
digits++;
Advance();
} while (IsDecimalDigit(c0_));
if (c0_ != '.' && c0_ != 'e' && c0_ != 'E' && digits < 10) {
SkipWhitespace();
return Handle<Smi>(Smi::FromInt((negative ? -i : i)), isolate());
return Handle<Smi>(Smi::FromInt((negative ? -static_cast<int>(i) : i)),
isolate());
}
}
if (c0_ == '.') {
......
......@@ -12,7 +12,7 @@
#include "src/ast/ast.h"
#include "src/ast/source-range-ast-visitor.h"
#include "src/bailout-reason.h"
#include "src/base/ieee754.h"
#include "src/base/overflowing-math.h"
#include "src/base/platform/platform.h"
#include "src/char-predicates-inl.h"
#include "src/compiler-dispatcher/compiler-dispatcher.h"
......@@ -160,8 +160,7 @@ bool Parser::ShortcutNumericLiteralBinaryExpression(Expression** x,
*x = factory()->NewNumberLiteral(x_val * y_val, pos);
return true;
case Token::DIV:
*x = factory()->NewNumberLiteral(base::ieee754::divide(x_val, y_val),
pos);
*x = factory()->NewNumberLiteral(base::Divide(x_val, y_val), pos);
return true;
case Token::BIT_OR: {
int value = DoubleToInt32(x_val) | DoubleToInt32(y_val);
......
......@@ -29,8 +29,6 @@ int RegExpMacroAssembler::CaseInsensitiveCompareUC16(Address byte_offset1,
Address byte_offset2,
size_t byte_length,
Isolate* isolate) {
unibrow::Mapping<unibrow::Ecma262Canonicalize>* canonicalize =
isolate->regexp_macro_assembler_canonicalize();
// This function is not allowed to cause a garbage collection.
// A GC might move the calling generated code and invalidate the
// return address on the stack.
......@@ -67,6 +65,8 @@ int RegExpMacroAssembler::CaseInsensitiveCompareUC16(Address byte_offset1,
}
#endif // V8_INTL_SUPPORT
DCHECK_NOT_NULL(isolate);
unibrow::Mapping<unibrow::Ecma262Canonicalize>* canonicalize =
isolate->regexp_macro_assembler_canonicalize();
for (size_t i = 0; i < length; i++) {
unibrow::uchar c1 = substring1[i];
unibrow::uchar c2 = substring2[i];
......
......@@ -40,6 +40,7 @@
#include "include/v8-util.h"
#include "src/api-inl.h"
#include "src/arguments.h"
#include "src/base/overflowing-math.h"
#include "src/base/platform/platform.h"
#include "src/compilation-cache.h"
#include "src/debug/debug.h"
......@@ -1262,7 +1263,7 @@ THREADED_PROFILED_TEST(FastReturnValues) {
};
for (size_t i = 0; i < arraysize(int_values); i++) {
for (int modifier = -1; modifier <= 1; modifier++) {
int int_value = int_values[i] + modifier;
int int_value = v8::base::AddWithWraparound(int_values[i], modifier);
// check int32_t
fast_return_value_int32 = int_value;
value = TestFastReturnValues<int32_t>();
......
......@@ -27,6 +27,7 @@
#include <stdlib.h>
#include "src/base/overflowing-math.h"
#include "src/v8.h"
#include "test/cctest/cctest.h"
......@@ -133,7 +134,7 @@ void TestSet(IntKeyHash hash, int size) {
for (uint32_t i = 0; i < n; i++) {
CHECK_EQ(i, static_cast<double>(set.occupancy()));
set.Insert(x);
x = x * factor + offset;
x = base::AddWithWraparound(base::MulWithWraparound(x, factor), offset);
}
CHECK_EQ(n, static_cast<double>(set.occupancy()));
......@@ -141,7 +142,7 @@ void TestSet(IntKeyHash hash, int size) {
x = start;
for (uint32_t i = 0; i < n; i++) {
CHECK(set.Present(x));
x = x * factor + offset;
x = base::AddWithWraparound(base::MulWithWraparound(x, factor), offset);
}
CHECK_EQ(n, static_cast<double>(set.occupancy()));
......@@ -152,7 +153,7 @@ void TestSet(IntKeyHash hash, int size) {
CHECK(set.Present(x));
set.Remove(x);
CHECK(!set.Present(x));
x = x * factor + offset;
x = base::AddWithWraparound(base::MulWithWraparound(x, factor), offset);
// Verify the the expected values are still there.
int y = start;
......@@ -162,7 +163,7 @@ void TestSet(IntKeyHash hash, int size) {
} else {
CHECK(set.Present(y));
}
y = y * factor + offset;
y = base::AddWithWraparound(base::MulWithWraparound(y, factor), offset);
}
}
CHECK_EQ(0u, set.occupancy());
......
......@@ -9,6 +9,7 @@
#include "src/accessors.h"
#include "src/api-inl.h"
#include "src/base/overflowing-math.h"
#include "src/compilation-cache.h"
#include "src/execution.h"
#include "src/field-type.h"
......@@ -440,7 +441,7 @@ static void TestLayoutDescriptorQueriesSlow(int max_sequence_length) {
int cur = 0;
for (int i = 0; i < kMaxNumberOfDescriptors; i++) {
bit_flip_positions[i] = cur;
cur = (cur + 1) * 2;
cur = base::MulWithWraparound((cur + 1), 2);
}
CHECK_LT(cur, 10000);
bit_flip_positions[kMaxNumberOfDescriptors] = 10000;
......@@ -453,7 +454,7 @@ static void TestLayoutDescriptorQueriesSlow(int max_sequence_length) {
int cur = 3;
for (int i = 0; i < kMaxNumberOfDescriptors; i++) {
bit_flip_positions[i] = cur;
cur = (cur + 1) * 2;
cur = base::MulWithWraparound((cur + 1), 2);
}
CHECK_LT(cur, 10000);
bit_flip_positions[kMaxNumberOfDescriptors] = 10000;
......
......@@ -6,6 +6,7 @@
#include "src/base/ieee754.h"
#include "src/base/macros.h"
#include "src/base/overflowing-math.h"
#include "testing/gmock-support.h"
#include "testing/gtest-support.h"
......@@ -314,8 +315,8 @@ TEST(Ieee754, Sin) {
EXPECT_THAT(sin(-kInfinity), IsNaN());
// Tests for sin for |x| < pi/4
EXPECT_EQ(-kInfinity, 1 / sin(-0.0));
EXPECT_EQ(kInfinity, 1 / sin(0.0));
EXPECT_EQ(-kInfinity, Divide(1.0, sin(-0.0)));
EXPECT_EQ(kInfinity, Divide(1.0, sin(0.0)));
// sin(x) = x for x < 2^-27
EXPECT_EQ(2.3283064365386963e-10, sin(2.3283064365386963e-10));
EXPECT_EQ(-2.3283064365386963e-10, sin(-2.3283064365386963e-10));
......@@ -361,8 +362,8 @@ TEST(Ieee754, Tan) {
EXPECT_THAT(tan(-kInfinity), IsNaN());
// Tests for tan for |x| < pi/4
EXPECT_EQ(kInfinity, 1 / tan(0.0));
EXPECT_EQ(-kInfinity, 1 / tan(-0.0));
EXPECT_EQ(kInfinity, Divide(1.0, tan(0.0)));
EXPECT_EQ(-kInfinity, Divide(1.0, tan(-0.0)));
// tan(x) = x for |x| < 2^-28
EXPECT_EQ(2.3283064365386963e-10, tan(2.3283064365386963e-10));
EXPECT_EQ(-2.3283064365386963e-10, tan(-2.3283064365386963e-10));
......
......@@ -68,8 +68,9 @@ TEST_F(SpacesTest, WriteBarrierFromHeapObject) {
}
TEST_F(SpacesTest, WriteBarrierIsMarking) {
char memory[256];
memset(&memory, 0, sizeof(memory));
const size_t kSizeOfMemoryChunk = sizeof(MemoryChunk);
char memory[kSizeOfMemoryChunk];
memset(&memory, 0, kSizeOfMemoryChunk);
MemoryChunk* chunk = reinterpret_cast<MemoryChunk*>(&memory);
heap_internals::MemoryChunk* slim_chunk =
reinterpret_cast<heap_internals::MemoryChunk*>(&memory);
......@@ -84,8 +85,9 @@ TEST_F(SpacesTest, WriteBarrierIsMarking) {
}
TEST_F(SpacesTest, WriteBarrierInNewSpaceToSpace) {
char memory[256];
memset(&memory, 0, sizeof(memory));
const size_t kSizeOfMemoryChunk = sizeof(MemoryChunk);
char memory[kSizeOfMemoryChunk];
memset(&memory, 0, kSizeOfMemoryChunk);
MemoryChunk* chunk = reinterpret_cast<MemoryChunk*>(&memory);
heap_internals::MemoryChunk* slim_chunk =
reinterpret_cast<heap_internals::MemoryChunk*>(&memory);
......@@ -100,8 +102,9 @@ TEST_F(SpacesTest, WriteBarrierInNewSpaceToSpace) {
}
TEST_F(SpacesTest, WriteBarrierInNewSpaceFromSpace) {
char memory[256];
memset(&memory, 0, sizeof(memory));
const size_t kSizeOfMemoryChunk = sizeof(MemoryChunk);
char memory[kSizeOfMemoryChunk];
memset(&memory, 0, kSizeOfMemoryChunk);
MemoryChunk* chunk = reinterpret_cast<MemoryChunk*>(&memory);
heap_internals::MemoryChunk* slim_chunk =
reinterpret_cast<heap_internals::MemoryChunk*>(&memory);
......
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