[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
Review-Url: https://codereview.chromium.org/2444233004
Cr-Commit-Position: refs/heads/master@{#40592}
diff --git a/src/bailout-reason.h b/src/bailout-reason.h
index a03b6cc..17d5168 100644
--- a/src/bailout-reason.h
+++ b/src/bailout-reason.h
@@ -254,6 +254,7 @@
V(kUnexpectedReturnFromThrow, "Unexpectedly returned from a throw") \
V(kUnsupportedSwitchStatement, "Unsupported switch statement") \
V(kUnsupportedTaggedImmediate, "Unsupported tagged immediate") \
+ V(kUnstableConstantTypeHeapObject, "Unstable constant-type heap object") \
V(kVariableResolvedToWithContext, "Variable resolved to with context") \
V(kWeShouldNotHaveAnEmptyLexicalContext, \
"We should not have an empty lexical context") \
diff --git a/src/compiler/js-global-object-specialization.cc b/src/compiler/js-global-object-specialization.cc
index a06704a..b38aed4 100644
--- a/src/compiler/js-global-object-specialization.cc
+++ b/src/compiler/js-global-object-specialization.cc
@@ -196,13 +196,18 @@
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);
diff --git a/src/crankshaft/hydrogen.cc b/src/crankshaft/hydrogen.cc
index 16c3639..79e78a5 100644
--- a/src/crankshaft/hydrogen.cc
+++ b/src/crankshaft/hydrogen.cc
@@ -6518,11 +6518,19 @@
access = access.WithRepresentation(Representation::Smi());
break;
case PropertyCellConstantType::kStableMap: {
- // 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());
+ // 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());
+ if (!cell_value_map->is_stable()) {
+ return Bailout(kUnstableConstantTypeHeapObject);
+ }
+ top_info()->dependencies()->AssumeMapStable(cell_value_map);
+ // Now check that the new {value} is a HeapObject with the same map.
Add<HCheckHeapObject>(value);
- value = Add<HCheckMaps>(value, map);
+ value = Add<HCheckMaps>(value, cell_value_map);
access = access.WithRepresentation(Representation::HeapObject());
break;
}
diff --git a/src/runtime/runtime-utils.h b/src/runtime/runtime-utils.h
index 0d84354..147efed 100644
--- a/src/runtime/runtime-utils.h
+++ b/src/runtime/runtime-utils.h
@@ -69,9 +69,11 @@
// 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]->IsSmi()); \
- CHECK(is_valid_language_mode(args.smi_at(index))); \
- LanguageMode name = static_cast<LanguageMode>(args.smi_at(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);
// 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.
diff --git a/test/mjsunit/regress/regress-crbug-659475-1.js b/test/mjsunit/regress/regress-crbug-659475-1.js
new file mode 100644
index 0000000..2648203
--- /dev/null
+++ b/test/mjsunit/regress/regress-crbug-659475-1.js
@@ -0,0 +1,30 @@
+// 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');
diff --git a/test/mjsunit/regress/regress-crbug-659475-2.js b/test/mjsunit/regress/regress-crbug-659475-2.js
new file mode 100644
index 0000000..49e02fd
--- /dev/null
+++ b/test/mjsunit/regress/regress-crbug-659475-2.js
@@ -0,0 +1,31 @@
+// 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');
