Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2416563002:
[heap] Fix forwarding pointer race between sweeper threads and pointer updating threads. (Closed)
|
Created:
4 years, 2 months ago by Hannes Payer (out of office) Modified:
4 years, 2 months ago CC:
v8-reviews_googlegroups.com, Hannes Payer (out of office), ulan Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[heap] Fix forwarding pointer race between sweeper threads and pointer updating threads.
The race can happen if:
1) Fixed array A is right before object B in new space
2) A slot in object C located in old space points to object B (old to new remembered set entry is created)
3) Object C becomes unreachable which held the only reference to object B which also becomes unreachable
4) Fixed array A gets right trimmed
5) The sweeper will sweep the last word of object A and object B. It will write the free space map into the last word of object A and a size field in the first word of object B.
6) Pointer updating may observe the size field now because the recored slot points to the start of object B and will confuse it with a forwarding pointer.
Note a similar race may happen with left trimming. Array A points to its backing store, the backing store gets left trimmed by 1 element, and array A dies.
BUG=
Committed: https://crrev.com/13488842de6a672dffe102a4a6eb414dd1fb25e0
Cr-Commit-Position: refs/heads/master@{#40218}
Patch Set 1 #
Total comments: 3
Patch Set 2 : explict bailout #Messages
Total messages: 21 (13 generated)
The CQ bit was checked by hpayer@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [heap] Fix forwarding pointer race betweenwq sweeper threads and pointer updating threads. BUG= ========== to ========== [heap] Fix forwarding pointer race between sweeper threads and pointer updating threads. The race can happen if: 1) Fixed array A is right before object B in new space 2) A slot in object C located in old space points to object B (old to new remembered set entry is created) 3) Object C becomes unreachable which held the only reference to object B which also becomes unreachable 4) Fixed array A gets right trimmed 5) The sweeper will sweep the last word of object A and object B. It will write the free space map into the last word of object A and a size field in the first word of object B. 6) Pointer updating may observe the size field now because the recored slot points to the start of object B and will confuse it with a forwarding pointer. BUG= ==========
hpayer@chromium.org changed reviewers: + mlippautz@chromium.org, ulan@chromium.org
Description was changed from ========== [heap] Fix forwarding pointer race between sweeper threads and pointer updating threads. The race can happen if: 1) Fixed array A is right before object B in new space 2) A slot in object C located in old space points to object B (old to new remembered set entry is created) 3) Object C becomes unreachable which held the only reference to object B which also becomes unreachable 4) Fixed array A gets right trimmed 5) The sweeper will sweep the last word of object A and object B. It will write the free space map into the last word of object A and a size field in the first word of object B. 6) Pointer updating may observe the size field now because the recored slot points to the start of object B and will confuse it with a forwarding pointer. BUG= ========== to ========== [heap] Fix forwarding pointer race between sweeper threads and pointer updating threads. The race can happen if: 1) Fixed array A is right before object B in new space 2) A slot in object C located in old space points to object B (old to new remembered set entry is created) 3) Object C becomes unreachable which held the only reference to object B which also becomes unreachable 4) Fixed array A gets right trimmed 5) The sweeper will sweep the last word of object A and object B. It will write the free space map into the last word of object A and a size field in the first word of object B. 6) Pointer updating may observe the size field now because the recored slot points to the start of object B and will confuse it with a forwarding pointer. Note that a similar scenario can be created with left trimming. BUG= ==========
Description was changed from ========== [heap] Fix forwarding pointer race between sweeper threads and pointer updating threads. The race can happen if: 1) Fixed array A is right before object B in new space 2) A slot in object C located in old space points to object B (old to new remembered set entry is created) 3) Object C becomes unreachable which held the only reference to object B which also becomes unreachable 4) Fixed array A gets right trimmed 5) The sweeper will sweep the last word of object A and object B. It will write the free space map into the last word of object A and a size field in the first word of object B. 6) Pointer updating may observe the size field now because the recored slot points to the start of object B and will confuse it with a forwarding pointer. Note that a similar scenario can be created with left trimming. BUG= ========== to ========== [heap] Fix forwarding pointer race between sweeper threads and pointer updating threads. The race can happen if: 1) Fixed array A is right before object B in new space 2) A slot in object C located in old space points to object B (old to new remembered set entry is created) 3) Object C becomes unreachable which held the only reference to object B which also becomes unreachable 4) Fixed array A gets right trimmed 5) The sweeper will sweep the last word of object A and object B. It will write the free space map into the last word of object A and a size field in the first word of object B. 6) Pointer updating may observe the size field now because the recored slot points to the start of object B and will confuse it with a forwarding pointer. BUG= ==========
https://codereview.chromium.org/2416563002/diff/1/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/2416563002/diff/1/src/heap/mark-compact.cc#ne... src/heap/mark-compact.cc:3645: map_word.ToRawValue() >= Page::kPageSize) { I think we need to return REMOVE_SLOT if map_word.ToRawValue() < Page::kPageSize otherwise we will crash below when checking for InToSpace.
Description was changed from ========== [heap] Fix forwarding pointer race between sweeper threads and pointer updating threads. The race can happen if: 1) Fixed array A is right before object B in new space 2) A slot in object C located in old space points to object B (old to new remembered set entry is created) 3) Object C becomes unreachable which held the only reference to object B which also becomes unreachable 4) Fixed array A gets right trimmed 5) The sweeper will sweep the last word of object A and object B. It will write the free space map into the last word of object A and a size field in the first word of object B. 6) Pointer updating may observe the size field now because the recored slot points to the start of object B and will confuse it with a forwarding pointer. BUG= ========== to ========== [heap] Fix forwarding pointer race between sweeper threads and pointer updating threads. The race can happen if: 1) Fixed array A is right before object B in new space 2) A slot in object C located in old space points to object B (old to new remembered set entry is created) 3) Object C becomes unreachable which held the only reference to object B which also becomes unreachable 4) Fixed array A gets right trimmed 5) The sweeper will sweep the last word of object A and object B. It will write the free space map into the last word of object A and a size field in the first word of object B. 6) Pointer updating may observe the size field now because the recored slot points to the start of object B and will confuse it with a forwarding pointer. Note a similar race may happen with left trimming. Array A points to its backing store, the backing store gets left trimmed by 1 element, and array A dies. BUG= ==========
lgtm
lgtm https://codereview.chromium.org/2416563002/diff/1/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/2416563002/diff/1/src/heap/mark-compact.cc#ne... src/heap/mark-compact.cc:3645: map_word.ToRawValue() >= Page::kPageSize) { On 2016/10/12 12:18:10, ulan wrote: > I think we need to return REMOVE_SLOT if map_word.ToRawValue() < Page::kPageSize > > otherwise we will crash below when checking for InToSpace. Nevermind, this works because InToSpace checks for Smi.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2416563002/diff/1/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/2416563002/diff/1/src/heap/mark-compact.cc#ne... src/heap/mark-compact.cc:3645: map_word.ToRawValue() >= Page::kPageSize) { On 2016/10/12 12:21:10, ulan wrote: > On 2016/10/12 12:18:10, ulan wrote: > > I think we need to return REMOVE_SLOT if map_word.ToRawValue() < > Page::kPageSize > > > > otherwise we will crash below when checking for InToSpace. > > Nevermind, this works because InToSpace checks for Smi. Yes, however I changed it to early bailout. The code is easier to read then.
The CQ bit was checked by hpayer@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mlippautz@chromium.org, ulan@chromium.org Link to the patchset: https://codereview.chromium.org/2416563002/#ps20001 (title: "explict bailout")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [heap] Fix forwarding pointer race between sweeper threads and pointer updating threads. The race can happen if: 1) Fixed array A is right before object B in new space 2) A slot in object C located in old space points to object B (old to new remembered set entry is created) 3) Object C becomes unreachable which held the only reference to object B which also becomes unreachable 4) Fixed array A gets right trimmed 5) The sweeper will sweep the last word of object A and object B. It will write the free space map into the last word of object A and a size field in the first word of object B. 6) Pointer updating may observe the size field now because the recored slot points to the start of object B and will confuse it with a forwarding pointer. Note a similar race may happen with left trimming. Array A points to its backing store, the backing store gets left trimmed by 1 element, and array A dies. BUG= ========== to ========== [heap] Fix forwarding pointer race between sweeper threads and pointer updating threads. The race can happen if: 1) Fixed array A is right before object B in new space 2) A slot in object C located in old space points to object B (old to new remembered set entry is created) 3) Object C becomes unreachable which held the only reference to object B which also becomes unreachable 4) Fixed array A gets right trimmed 5) The sweeper will sweep the last word of object A and object B. It will write the free space map into the last word of object A and a size field in the first word of object B. 6) Pointer updating may observe the size field now because the recored slot points to the start of object B and will confuse it with a forwarding pointer. Note a similar race may happen with left trimming. Array A points to its backing store, the backing store gets left trimmed by 1 element, and array A dies. BUG= ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [heap] Fix forwarding pointer race between sweeper threads and pointer updating threads. The race can happen if: 1) Fixed array A is right before object B in new space 2) A slot in object C located in old space points to object B (old to new remembered set entry is created) 3) Object C becomes unreachable which held the only reference to object B which also becomes unreachable 4) Fixed array A gets right trimmed 5) The sweeper will sweep the last word of object A and object B. It will write the free space map into the last word of object A and a size field in the first word of object B. 6) Pointer updating may observe the size field now because the recored slot points to the start of object B and will confuse it with a forwarding pointer. Note a similar race may happen with left trimming. Array A points to its backing store, the backing store gets left trimmed by 1 element, and array A dies. BUG= ========== to ========== [heap] Fix forwarding pointer race between sweeper threads and pointer updating threads. The race can happen if: 1) Fixed array A is right before object B in new space 2) A slot in object C located in old space points to object B (old to new remembered set entry is created) 3) Object C becomes unreachable which held the only reference to object B which also becomes unreachable 4) Fixed array A gets right trimmed 5) The sweeper will sweep the last word of object A and object B. It will write the free space map into the last word of object A and a size field in the first word of object B. 6) Pointer updating may observe the size field now because the recored slot points to the start of object B and will confuse it with a forwarding pointer. Note a similar race may happen with left trimming. Array A points to its backing store, the backing store gets left trimmed by 1 element, and array A dies. BUG= Committed: https://crrev.com/13488842de6a672dffe102a4a6eb414dd1fb25e0 Cr-Commit-Position: refs/heads/master@{#40218} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/13488842de6a672dffe102a4a6eb414dd1fb25e0 Cr-Commit-Position: refs/heads/master@{#40218} |
