Skip to content

Commit

Permalink
deps: cherry-pick 8ed65b97 from V8's upstream
Browse files Browse the repository at this point in the history
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=#8310

    Review-Url: https://codereview.chromium.org/2292953002
    Cr-Commit-Position: refs/heads/master@{#39023}

Fixes: #8310
PR-URL: #8411
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
  • Loading branch information
addaleax committed Sep 8, 2016
1 parent 180867d commit f829660
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
4 changes: 3 additions & 1 deletion deps/v8/src/field-type.cc
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions deps/v8/test/cctest/test-field-type-tracking.cc
Expand Up @@ -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;

Expand Down Expand Up @@ -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)

0 comments on commit f829660

Please sign in to comment.