Commit 8ed65b97 authored by addaleax's avatar addaleax Committed by Commit bot

Make FieldType::None() non-nullptr value to avoid undefined behaviour

When FieldType::None() returns a cast Smi::FromInt(0), which translates
as nullptr, the FieldType::IsNone() check becomes equivalent to
`this == nullptr` which is not allowed by the standard and
therefore optimized away as a false constant by GCC 6.

This has lead to crashes when invoking methods on FieldType::None().

Using a different Smi constant for FieldType::None() makes the compiler
always include a comparison against that value. The choice of these
constants has no effect as they are effectively arbitrary.

BUG=https://github.com/nodejs/node/issues/8310

Review-Url: https://codereview.chromium.org/2292953002
Cr-Commit-Position: refs/heads/master@{#39023}
parent 9f747be5
......@@ -13,7 +13,9 @@ namespace internal {
// static
FieldType* FieldType::None() {
return reinterpret_cast<FieldType*>(Smi::FromInt(0));
// Do not Smi::FromInt(0) here or for Any(), as that may translate
// as `nullptr` which is not a valid value for `this`.
return reinterpret_cast<FieldType*>(Smi::FromInt(2));
}
// static
......
......@@ -17,6 +17,7 @@
#include "src/global-handles.h"
#include "src/ic/stub-cache.h"
#include "src/macro-assembler.h"
#include "src/types.h"
using namespace v8::internal;
......@@ -2410,6 +2411,16 @@ TEST(TransitionAccessorConstantToSameAccessorConstant) {
TestTransitionTo(transition_op, transition_op, checker);
}
TEST(FieldTypeConvertSimple) {
CcTest::InitializeVM();
v8::HandleScope scope(CcTest::isolate());
Isolate* isolate = CcTest::i_isolate();
Zone zone(isolate->allocator());
CHECK_EQ(FieldType::Any()->Convert(&zone), Type::NonInternal());
CHECK_EQ(FieldType::None()->Convert(&zone), Type::None());
}
// TODO(ishell): add this test once IS_ACCESSOR_FIELD_SUPPORTED is supported.
// TEST(TransitionAccessorConstantToAnotherAccessorConstant)
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