Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(367)

Issue 1286343004: Filter out slot buffer slots, that point to SMIs in dead objects. (Closed)

Created:
5 years, 4 months ago by Hannes Payer (out of office)
Modified:
5 years, 4 months ago
Reviewers:
Jakob Kummerow
CC:
v8-dev
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Filter out slot buffer slots, that point to SMIs in dead objects. The following situation may happen which reproduces this bug: (1) We allocate JSObject A on an evacuation candidate. (2) We allocate JSObject B on a non-evacuation candidate. (3) Incremental marking starts and marks object A and B. (4) We create a reference from B.field = A; which records the slot B.field since A is on an evacuation candidate. (5) After that we write a SMI into B.field. (6) After that B goes into dictionary mode and shrinks its original size. B.field is now outside of the JSObject, i.e B.field is in memory that will be freed by the sweeper threads. (7) GC is triggered. (8) BUG: Slots buffer filtering walks over the slots buffer, SMIs are not filtered out because we assumed that SMIs are just ignored when the slots get updated later. However, recorded SMI slots of dead objects may be overwritten by double values at evacuation time. (9) During evacuation, a heap number that looks like a valid pointer is moved over B.field. (10) The slots buffer is scanned for updates, follows B.field since it looks like a pointer (the double value looks like a pointer), and crashes. BUG=chromium:519577, chromium:454297 LOG=y Committed: https://crrev.com/8606664b37f4dc4b42106563984c19e4f72d9d3a Cr-Commit-Position: refs/heads/master@{#30200}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -6 lines) Patch
M src/heap/mark-compact.cc View 1 chunk +4 lines, -6 lines 0 comments Download

Messages

Total messages: 11 (4 generated)
Hannes Payer (out of office)
5 years, 4 months ago (2015-08-17 14:57:36 UTC) #2
Jakob Kummerow
lgtm
5 years, 4 months ago (2015-08-17 14:58:55 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1286343004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1286343004/1
5 years, 4 months ago (2015-08-17 14:59:28 UTC) #5
commit-bot: I haz the power
Try jobs failed on following builders: v8_presubmit on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/4962)
5 years, 4 months ago (2015-08-17 15:03:07 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1286343004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1286343004/1
5 years, 4 months ago (2015-08-17 15:06:56 UTC) #9
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 4 months ago (2015-08-17 15:24:18 UTC) #10
commit-bot: I haz the power
5 years, 4 months ago (2015-08-17 15:24:36 UTC) #11
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/8606664b37f4dc4b42106563984c19e4f72d9d3a
Cr-Commit-Position: refs/heads/master@{#30200}

Powered by Google App Engine
This is Rietveld 408576698