From f829660c71a1925fbd502f91a4e041683ab28466 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 5 Sep 2016 15:56:27 +0200 Subject: [PATCH] deps: cherry-pick 8ed65b97 from V8's upstream Original commit message: 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} Fixes: https://github.com/nodejs/node/issues/8310 PR-URL: https://github.com/nodejs/node/pull/8411 Reviewed-By: Ben Noordhuis Reviewed-By: Franziska Hinkelmann --- deps/v8/src/field-type.cc | 4 +++- deps/v8/test/cctest/test-field-type-tracking.cc | 11 +++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/deps/v8/src/field-type.cc b/deps/v8/src/field-type.cc index 76d694c132..57d00d0463 100644 --- a/deps/v8/src/field-type.cc +++ b/deps/v8/src/field-type.cc @@ -13,7 +13,9 @@ namespace internal { // static FieldType* FieldType::None() { - return reinterpret_cast(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(Smi::FromInt(2)); } // static diff --git a/deps/v8/test/cctest/test-field-type-tracking.cc b/deps/v8/test/cctest/test-field-type-tracking.cc index c7c6f84423..9d928e84a8 100644 --- a/deps/v8/test/cctest/test-field-type-tracking.cc +++ b/deps/v8/test/cctest/test-field-type-tracking.cc @@ -16,6 +16,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; @@ -2473,6 +2474,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::Any()); + CHECK_EQ(FieldType::None()->Convert(&zone), Type::None()); +} // TODO(ishell): add this test once IS_ACCESSOR_FIELD_SUPPORTED is supported. // TEST(TransitionAccessorConstantToAnotherAccessorConstant)