Skip to content

Commit

Permalink
deps: V8: cherry-pick 254c7945eea2
Browse files Browse the repository at this point in the history
Original commit message:

    Merged: [deoptimizer] Fix bug in OptimizedFrame::Summarize

    Revision: 3353a7d0b017146d543434be4036a81aaf7d25ae

    BUG=chromium:1182647
    NOTRY=true
    NOPRESUBMIT=true
    NOTREECHECKS=true
    R=​bmeurer@chromium.org

    (cherry picked from commit c0c96b768a7d3463b11403874549e6496529740d)

    Change-Id: I86abd6a3f34169be5f99aa9f54bb7bb3706fa85a
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2780300
    Reviewed-by: Georg Neis <neis@chromium.org>
    Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
    Commit-Queue: Georg Neis <neis@chromium.org>
    Cr-Original-Commit-Position: refs/branch-heads/8.9@{#49}
    Cr-Original-Branched-From: 16b9bbbd581c25391981aa03180b76aa60463a3e-refs/heads/8.9.255@{#1}
    Cr-Original-Branched-From: d16a2a688498bd1c3e6a49edb25d8c4ca56232dc-refs/heads/master@{#72039}
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2794427
    Reviewed-by: Victor-Gabriel Savu <vsavu@google.com>
    Commit-Queue: Artem Sumaneev <asumaneev@google.com>
    Cr-Commit-Position: refs/branch-heads/8.6@{#72}
    Cr-Branched-From: a64aed2333abf49e494d2a5ce24bbd14fff19f60-refs/heads/8.6.395@{#1}
    Cr-Branched-From: a626bc036236c9bf92ac7b87dc40c9e538b087e3-refs/heads/master@{#69472}

Refs: v8/v8@254c794

PR-URL: #38275
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
  • Loading branch information
targos committed Apr 30, 2021
1 parent 77b7a3b commit 5dc8246
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 11 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Expand Up @@ -36,7 +36,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.48',
'v8_embedder_string': '-node.49',

##### V8 defaults for Node.js #####

Expand Down
58 changes: 49 additions & 9 deletions deps/v8/src/deoptimizer/deoptimizer.cc
Expand Up @@ -3266,7 +3266,8 @@ Address TranslatedState::DecompressIfNeeded(intptr_t value) {
}
}

TranslatedState::TranslatedState(const JavaScriptFrame* frame) {
TranslatedState::TranslatedState(const JavaScriptFrame* frame)
: purpose_(kFrameInspection) {
int deopt_index = Safepoint::kNoDeoptimizationIndex;
DeoptimizationData data =
static_cast<const OptimizedFrame*>(frame)->GetDeoptimizationData(
Expand Down Expand Up @@ -3641,25 +3642,63 @@ void TranslatedState::EnsureCapturedObjectAllocatedAt(
}

default:
CHECK(map->IsJSObjectMap());
EnsureJSObjectAllocated(slot, map);
TranslatedValue* properties_slot = &(frame->values_[value_index]);
value_index++;
int remaining_children_count = slot->GetChildrenCount() - 1;

TranslatedValue* properties_slot = frame->ValueAt(value_index);
value_index++, remaining_children_count--;
if (properties_slot->kind() == TranslatedValue::kCapturedObject) {
// If we are materializing the property array, make sure we put
// the mutable heap numbers at the right places.
// We are materializing the property array, so make sure we put the
// mutable heap numbers at the right places.
EnsurePropertiesAllocatedAndMarked(properties_slot, map);
EnsureChildrenAllocated(properties_slot->GetChildrenCount(), frame,
&value_index, worklist);
} else {
CHECK_EQ(properties_slot->kind(), TranslatedValue::kTagged);
}
// Make sure all the remaining children (after the map and properties) are
// allocated.
return EnsureChildrenAllocated(slot->GetChildrenCount() - 2, frame,

TranslatedValue* elements_slot = frame->ValueAt(value_index);
value_index++, remaining_children_count--;
if (elements_slot->kind() == TranslatedValue::kCapturedObject ||
!map->IsJSArrayMap()) {
// Handle this case with the other remaining children below.
value_index--, remaining_children_count++;
} else {
CHECK_EQ(elements_slot->kind(), TranslatedValue::kTagged);
elements_slot->GetValue();
if (purpose_ == kFrameInspection) {
// We are materializing a JSArray for the purpose of frame inspection.
// If we were to construct it with the above elements value then an
// actual deopt later on might create another JSArray instance with
// the same elements store. That would violate the key assumption
// behind left-trimming.
elements_slot->ReplaceElementsArrayWithCopy();
}
}

// Make sure all the remaining children (after the map, properties store,
// and possibly elements store) are allocated.
return EnsureChildrenAllocated(remaining_children_count, frame,
&value_index, worklist);
}
UNREACHABLE();
}

void TranslatedValue::ReplaceElementsArrayWithCopy() {
DCHECK_EQ(kind(), TranslatedValue::kTagged);
DCHECK_EQ(materialization_state(), TranslatedValue::kFinished);
auto elements = Handle<FixedArrayBase>::cast(GetValue());
DCHECK(elements->IsFixedArray() || elements->IsFixedDoubleArray());
if (elements->IsFixedDoubleArray()) {
DCHECK(!elements->IsCowArray());
set_storage(isolate()->factory()->CopyFixedDoubleArray(
Handle<FixedDoubleArray>::cast(elements)));
} else if (!elements->IsCowArray()) {
set_storage(isolate()->factory()->CopyFixedArray(
Handle<FixedArray>::cast(elements)));
}
}

void TranslatedState::EnsureChildrenAllocated(int count, TranslatedFrame* frame,
int* value_index,
std::stack<int>* worklist) {
Expand Down Expand Up @@ -3724,6 +3763,7 @@ Handle<ByteArray> TranslatedState::AllocateStorageFor(TranslatedValue* slot) {

void TranslatedState::EnsureJSObjectAllocated(TranslatedValue* slot,
Handle<Map> map) {
CHECK(map->IsJSObjectMap());
CHECK_EQ(map->instance_size(), slot->GetChildrenCount() * kTaggedSize);

Handle<ByteArray> object_storage = AllocateStorageFor(slot);
Expand Down
19 changes: 18 additions & 1 deletion deps/v8/src/deoptimizer/deoptimizer.h
Expand Up @@ -117,6 +117,8 @@ class TranslatedValue {
return storage_;
}

void ReplaceElementsArrayWithCopy();

Kind kind_;
MaterializationState materialization_state_ = kUninitialized;
TranslatedState* container_; // This is only needed for materialization of
Expand Down Expand Up @@ -313,7 +315,15 @@ class TranslatedFrame {

class TranslatedState {
public:
TranslatedState() = default;
// There are two constructors, each for a different purpose:

// The default constructor is for the purpose of deoptimizing an optimized
// frame (replacing it with one or several unoptimized frames). It is used by
// the Deoptimizer.
TranslatedState() : purpose_(kDeoptimization) {}

// This constructor is for the purpose of merely inspecting an optimized
// frame. It is used by stack trace generation and various debugging features.
explicit TranslatedState(const JavaScriptFrame* frame);

void Prepare(Address stack_frame_pointer);
Expand Down Expand Up @@ -347,6 +357,12 @@ class TranslatedState {
private:
friend TranslatedValue;

// See the description of the constructors for an explanation of the two
// purposes. The only actual difference is that in the kFrameInspection case
// extra work is needed to not violate assumptions made by left-trimming. For
// details, see the code around ReplaceElementsArrayWithCopy.
enum Purpose { kDeoptimization, kFrameInspection };

TranslatedFrame CreateNextTranslatedFrame(TranslationIterator* iterator,
FixedArray literal_array,
Address fp, FILE* trace_file);
Expand Down Expand Up @@ -404,6 +420,7 @@ class TranslatedState {
static Float32 GetFloatSlot(Address fp, int slot_index);
static Float64 GetDoubleSlot(Address fp, int slot_index);

Purpose const purpose_;
std::vector<TranslatedFrame> frames_;
Isolate* isolate_ = nullptr;
Address stack_frame_pointer_ = kNullAddress;
Expand Down
25 changes: 25 additions & 0 deletions deps/v8/test/mjsunit/compiler/regress-1182647.js
@@ -0,0 +1,25 @@
// Copyright 2021 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// Flags: --allow-natives-syntax --verify-heap

function foo() {
const arr = Array(1000);

function bar() {
try { ({a: p4nda, b: arr.length}); } catch(e) {}
}

for (var i = 0; i < 25; i++) bar();

/p4nda/.test({}); // Deopt here.

arr.shift();
}

%PrepareFunctionForOptimization(foo);
foo();
foo();
%OptimizeFunctionOnNextCall(foo);
foo();

0 comments on commit 5dc8246

Please sign in to comment.