Skip to content

Commit ee09828

Browse files
cristiancavalliofrobots
authored andcommitted
deps: backport 2bd7464 from upstream V8
Original commit message: 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. [email protected] BUG=chromium:659475 Review-Url: https://codereview.chromium.org/2444233004 Cr-Commit-Position: refs/heads/master@{#40592} PR-URL: #10169 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]> Reviewed-By: ofrobots - Ali Ijaz Sheikh <[email protected]>
1 parent 168241a commit ee09828

File tree

7 files changed

+87
-10
lines changed

7 files changed

+87
-10
lines changed

deps/v8/include/v8-version.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
#define V8_MAJOR_VERSION 5
1212
#define V8_MINOR_VERSION 1
1313
#define V8_BUILD_NUMBER 281
14-
#define V8_PATCH_LEVEL 88
14+
#define V8_PATCH_LEVEL 89
1515

1616
// Use 1 for candidates and 0 otherwise.
1717
// (Boolean macro values are not supported by all preprocessors.)

deps/v8/src/bailout-reason.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,7 @@ namespace internal {
257257
V(kUnexpectedReturnFromThrow, "Unexpectedly returned from a throw") \
258258
V(kUnsupportedSwitchStatement, "Unsupported switch statement") \
259259
V(kUnsupportedTaggedImmediate, "Unsupported tagged immediate") \
260+
V(kUnstableConstantTypeHeapObject, "Unstable constant-type heap object") \
260261
V(kVariableResolvedToWithContext, "Variable resolved to with context") \
261262
V(kWeShouldNotHaveAnEmptyLexicalContext, \
262263
"We should not have an empty lexical context") \

deps/v8/src/compiler/js-global-object-specialization.cc

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,13 @@ Reduction JSGlobalObjectSpecialization::ReduceJSStoreGlobal(Node* node) {
182182
Node* check = graph()->NewNode(simplified()->ObjectIsSmi(), value);
183183
Type* property_cell_value_type = Type::TaggedSigned();
184184
if (property_cell_value->IsHeapObject()) {
185+
// We cannot do anything if the {property_cell_value}s map is no
186+
// longer stable.
187+
Handle<Map> property_cell_value_map(
188+
Handle<HeapObject>::cast(property_cell_value)->map(), isolate());
189+
if (!property_cell_value_map->is_stable()) return NoChange();
190+
dependencies()->AssumeMapStable(property_cell_value_map);
191+
185192
// Deoptimize if the {value} is a Smi.
186193
control = graph()->NewNode(common()->DeoptimizeIf(), check, frame_state,
187194
effect, control);
@@ -190,8 +197,6 @@ Reduction JSGlobalObjectSpecialization::ReduceJSStoreGlobal(Node* node) {
190197
Node* value_map = effect =
191198
graph()->NewNode(simplified()->LoadField(AccessBuilder::ForMap()),
192199
value, effect, control);
193-
Handle<Map> property_cell_value_map(
194-
Handle<HeapObject>::cast(property_cell_value)->map(), isolate());
195200
check = graph()->NewNode(
196201
simplified()->ReferenceEqual(Type::Any()), value_map,
197202
jsgraph()->HeapConstant(property_cell_value_map));

deps/v8/src/crankshaft/hydrogen.cc

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6908,11 +6908,19 @@ void HOptimizedGraphBuilder::HandleGlobalVariableAssignment(
69086908
access = access.WithRepresentation(Representation::Smi());
69096909
break;
69106910
case PropertyCellConstantType::kStableMap: {
6911-
// The map may no longer be stable, deopt if it's ever different from
6912-
// what is currently there, which will allow for restablization.
6913-
Handle<Map> map(HeapObject::cast(cell->value())->map());
6911+
// First check that the previous value of the {cell} still has the
6912+
// map that we are about to check the new {value} for. If not, then
6913+
// the stable map assumption was invalidated and we cannot continue
6914+
// with the optimized code.
6915+
Handle<HeapObject> cell_value(HeapObject::cast(cell->value()));
6916+
Handle<Map> cell_value_map(cell_value->map());
6917+
if (!cell_value_map->is_stable()) {
6918+
return Bailout(kUnstableConstantTypeHeapObject);
6919+
}
6920+
top_info()->dependencies()->AssumeMapStable(cell_value_map);
6921+
// Now check that the new {value} is a HeapObject with the same map.
69146922
Add<HCheckHeapObject>(value);
6915-
value = Add<HCheckMaps>(value, map);
6923+
value = Add<HCheckMaps>(value, cell_value_map);
69166924
access = access.WithRepresentation(Representation::HeapObject());
69176925
break;
69186926
}

deps/v8/src/runtime/runtime-utils.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,11 @@ namespace internal {
115115
// Assert that the given argument has a valid value for a LanguageMode
116116
// and store it in a LanguageMode variable with the given name.
117117
#define CONVERT_LANGUAGE_MODE_ARG_CHECKED(name, index) \
118-
RUNTIME_ASSERT(args[index]->IsSmi()); \
119-
RUNTIME_ASSERT(is_valid_language_mode(args.smi_at(index))); \
120-
LanguageMode name = static_cast<LanguageMode>(args.smi_at(index));
118+
RUNTIME_ASSERT(args[index]->IsNumber()); \
119+
int32_t __tmp_##name = 0; \
120+
RUNTIME_ASSERT(args[index]->ToInt32(&__tmp_##name)); \
121+
RUNTIME_ASSERT(is_valid_language_mode(__tmp_##name)); \
122+
LanguageMode name = static_cast<LanguageMode>(__tmp_##name);
121123

122124

123125
// Assert that the given argument is a number within the Int32 range
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Copyright 2016 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
// Flags: --allow-natives-syntax
6+
7+
var n;
8+
9+
function Ctor() {
10+
n = new Set();
11+
}
12+
13+
function Check() {
14+
n.xyz = 0x826852f4;
15+
}
16+
17+
Ctor();
18+
Ctor();
19+
%OptimizeFunctionOnNextCall(Ctor);
20+
Ctor();
21+
22+
Check();
23+
Check();
24+
%OptimizeFunctionOnNextCall(Check);
25+
Check();
26+
27+
Ctor();
28+
Check();
29+
30+
parseInt('AAAAAAAA');
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Copyright 2016 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
// Flags: --allow-natives-syntax
6+
7+
var n;
8+
9+
function Ctor() {
10+
try { } catch (e) {}
11+
n = new Set();
12+
}
13+
14+
function Check() {
15+
n.xyz = 0x826852f4;
16+
}
17+
18+
Ctor();
19+
Ctor();
20+
%OptimizeFunctionOnNextCall(Ctor);
21+
Ctor();
22+
23+
Check();
24+
Check();
25+
%OptimizeFunctionOnNextCall(Check);
26+
Check();
27+
28+
Ctor();
29+
Check();
30+
31+
parseInt('AAAAAAAA');

0 commit comments

Comments
 (0)