Commit d0a047d4 authored by bmeurer's avatar bmeurer Committed by Commit bot

Revert of [compiler] Properly validate stable map assumption for globals....

Revert of [compiler] Properly validate stable map assumption for globals. (patchset #3 id:40001 of https://codereview.chromium.org/2444233004/ )

Reason for revert:
Breaks tree: http://build.chromium.org/p/client.v8/builders/V8%20Linux64%20GC%20Stress%20-%20custom%20snapshot/builds/8789

Original issue's description:
> [compiler] Properly validate stable map assumption for globals.
>
> For global object property cells, we did not check that the map on the
> previous object is still the same for which we actually optimized. So
> the optimized code was not in sync with the actual state of the property
> cell. When loading from such a global object property cell, Crankshaft
> optimizes away any map checks (based on the stable map assumption),
> leading to arbitrary memory access in the worst case.
>
> TurboFan has the same bug for stores, but is safe on loads because we
> do appropriate map checks there. However mixing TurboFan and Crankshaft
> still exposes the bug.
>
> R=yangguo@chromium.org
> BUG=chromium:659475

TBR=yangguo@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:659475

Review-Url: https://codereview.chromium.org/2454513003
Cr-Commit-Position: refs/heads/master@{#40582}
parent a1670159
......@@ -196,18 +196,13 @@ Reduction JSGlobalObjectSpecialization::ReduceJSStoreGlobal(Node* node) {
Type* property_cell_value_type;
MachineRepresentation representation = MachineRepresentation::kTagged;
if (property_cell_value->IsHeapObject()) {
// We cannot do anything if the {property_cell_value}s map is no
// longer stable.
Handle<Map> property_cell_value_map(
Handle<HeapObject>::cast(property_cell_value)->map(), isolate());
if (!property_cell_value_map->is_stable()) return NoChange();
dependencies()->AssumeMapStable(property_cell_value_map);
// Check that the {value} is a HeapObject.
value = effect = graph()->NewNode(simplified()->CheckHeapObject(),
value, effect, control);
// Check {value} map agains the {property_cell} map.
Handle<Map> property_cell_value_map(
Handle<HeapObject>::cast(property_cell_value)->map(), isolate());
effect = graph()->NewNode(
simplified()->CheckMaps(1), value,
jsgraph()->HeapConstant(property_cell_value_map), effect, control);
......
......@@ -3092,7 +3092,6 @@ class HConstant final : public HTemplateInstruction<0> {
DECLARE_INSTRUCTION_FACTORY_P2(HConstant, int32_t, Representation);
DECLARE_INSTRUCTION_FACTORY_P1(HConstant, double);
DECLARE_INSTRUCTION_FACTORY_P1(HConstant, Handle<Object>);
DECLARE_INSTRUCTION_FACTORY_P2(HConstant, Handle<Object>, Representation);
DECLARE_INSTRUCTION_FACTORY_P1(HConstant, ExternalReference);
static HConstant* CreateAndInsertAfter(Isolate* isolate, Zone* zone,
......
......@@ -6518,19 +6518,11 @@ void HOptimizedGraphBuilder::HandleGlobalVariableAssignment(
access = access.WithRepresentation(Representation::Smi());
break;
case PropertyCellConstantType::kStableMap: {
// First check that the previous value of the {cell} still has the
// map that we are about to check the new {value} for. If not, then
// the stable map assumption was invalidated and we cannot continue
// with the optimized code.
Handle<HeapObject> cell_value(HeapObject::cast(cell->value()));
Handle<Map> cell_value_map(cell_value->map());
HCheckMaps* cell_value_check = Add<HCheckMaps>(
Add<HConstant>(cell_value, Representation::HeapObject()),
cell_value_map);
cell_value_check->ClearDependsOnFlag(kElementsKind);
// Now check that the new {value} is a HeapObject with the same map.
// The map may no longer be stable, deopt if it's ever different from
// what is currently there, which will allow for restablization.
Handle<Map> map(HeapObject::cast(cell->value())->map());
Add<HCheckHeapObject>(value);
value = Add<HCheckMaps>(value, cell_value_map);
value = Add<HCheckMaps>(value, map);
access = access.WithRepresentation(Representation::HeapObject());
break;
}
......
......@@ -69,11 +69,9 @@ namespace internal {
// Assert that the given argument has a valid value for a LanguageMode
// and store it in a LanguageMode variable with the given name.
#define CONVERT_LANGUAGE_MODE_ARG_CHECKED(name, index) \
CHECK(args[index]->IsNumber()); \
int32_t __tmp_##name = 0; \
CHECK(args[index]->ToInt32(&__tmp_##name)); \
CHECK(is_valid_language_mode(__tmp_##name)); \
LanguageMode name = static_cast<LanguageMode>(__tmp_##name);
CHECK(args[index]->IsSmi()); \
CHECK(is_valid_language_mode(args.smi_at(index))); \
LanguageMode name = static_cast<LanguageMode>(args.smi_at(index));
// Assert that the given argument is a number within the Int32 range
// and convert it to int32_t. If the argument is not an Int32 we crash safely.
......
// Copyright 2016 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
var n;
function Ctor() {
n = new Set();
}
function Check() {
n.xyz = 0x826852f4;
}
Ctor();
Ctor();
%OptimizeFunctionOnNextCall(Ctor);
Ctor();
Check();
Check();
%OptimizeFunctionOnNextCall(Check);
Check();
Ctor();
Check();
parseInt('AAAAAAAA');
// Copyright 2016 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
var n;
function Ctor() {
try { } catch (e) {}
n = new Set();
}
function Check() {
n.xyz = 0x826852f4;
}
Ctor();
Ctor();
%OptimizeFunctionOnNextCall(Ctor);
Ctor();
Check();
Check();
%OptimizeFunctionOnNextCall(Check);
Check();
Ctor();
Check();
parseInt('AAAAAAAA');
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