Oilpan: Validate pointers stored in Persistent

Oilpan: Validate pointers stored in Persistent

This CL adds checkPointer() to all pointer updates of Persistent.
The checkPointer() verifies that the pointer is a pointer to an object
of the ThreadHeap of the thread that is operating on the object.

BUG=475801
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196395

[haraken, 2015-05-21 11:16:20]

haraken@chromium.org changed reviewers:
+ keishi@chromium.org, oilpan-reviews@chromium.org

[haraken, 2015-05-21 11:16:21]

PTAL

[keishi, 2015-05-21 12:35:39]

lgtm

[haraken, 2015-05-21 13:15:06]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2015-05-21 13:15:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1146373002/1

[commit-bot: I haz the power, 2015-05-21 14:37:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-21 14:37:47]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/63431)

[haraken, 2015-05-21 23:30:54]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2015-05-21 23:31:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1146373002/1

[commit-bot: I haz the power, 2015-05-22 01:29:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-22 01:29:56]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/63505)

[haraken, 2015-05-22 04:50:58]

haraken@chromium.org changed reviewers:
+ tasak@google.com

[haraken, 2015-05-22 04:50:59]

+tasak

Hmm, only the win_rel_bot fails at HeapTest.WeakMembers. I don't yet figure out
what's going on there, but the result concerns me a bit. The result implies that
the conservative scanning fails at finding an on-stack pointer with this CL,
although it's not yet clear how this CL affects the shape of the stack.

[haraken, 2015-05-22 10:20:15]

On 2015/05/22 04:50:59, haraken wrote:
> +tasak
> 
> Hmm, only the win_rel_bot fails at HeapTest.WeakMembers. I don't yet figure
out
> what's going on there, but the result concerns me a bit. The result implies
that
> the conservative scanning fails at finding an on-stack pointer with this CL,
> although it's not yet clear how this CL affects the shape of the stack.

According to tasak's experiment, the issue doesn't reproduce on Win64 Debug and
Win64 Release. It seems to be a specific issue on Win32.

I need to dig into the problem using the win_blink_rel try bot...

[haraken, 2015-05-26 06:50:13]

https://codereview.chromium.org/1146373002/diff/220001/Source/platform/heap/H...
File Source/platform/heap/HeapTest.cpp (right):

https://codereview.chromium.org/1146373002/diff/220001/Source/platform/heap/H...
Source/platform/heap/HeapTest.cpp:3524: EXPECT_FALSE(h3->hasBeenFinalized());

OK, now I finally found the cause of the failure on win_rel_bot.

Without having this line, there is no user of h3 after the line 3519. So the
compiler can optimize out the h3 pointer from the stack. Then the GC can collect
the h3 and thus crash at the line 3523. Adding the line fixes the crash.

[Yuta Kitamura, 2015-05-26 09:26:28]

yutak@chromium.org changed reviewers:
+ yutak@chromium.org

[Yuta Kitamura, 2015-05-26 09:26:29]

https://codereview.chromium.org/1146373002/diff/220001/Source/platform/heap/H...
File Source/platform/heap/HeapTest.cpp (right):

https://codereview.chromium.org/1146373002/diff/220001/Source/platform/heap/H...
Source/platform/heap/HeapTest.cpp:3524: EXPECT_FALSE(h3->hasBeenFinalized());
On 2015/05/26 06:50:13, haraken wrote:
> 
> OK, now I finally found the cause of the failure on win_rel_bot.
> 
> Without having this line, there is no user of h3 after the line 3519. So the
> compiler can optimize out the h3 pointer from the stack. Then the GC can
collect
> the h3 and thus crash at the line 3523. Adding the line fixes the crash.

"volatile" can be used, maybe?

[haraken, 2015-06-03 07:40:27]

I rewrote the checkPointer with Heap::isHeapObjectAlive(). Landing.

[haraken, 2015-06-03 07:40:31]

The CQ bit was checked by haraken@chromium.org

[haraken, 2015-06-03 07:40:31]

The patchset sent to the CQ was uploaded after l-g-t-m from keishi@chromium.org
Link to the patchset: https://codereview.chromium.org/1146373002/#ps240001
(title: " ")

[commit-bot: I haz the power, 2015-06-03 07:41:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1146373002/240001

[haraken, 2015-06-03 07:45:14]

Patchset #15 (id:260001) has been deleted

[haraken, 2015-06-03 11:41:06]

The CQ bit was checked by haraken@chromium.org

[haraken, 2015-06-03 11:41:07]

The patchset sent to the CQ was uploaded after l-g-t-m from keishi@chromium.org
Link to the patchset: https://codereview.chromium.org/1146373002/#ps240001
(title: " ")

[commit-bot: I haz the power, 2015-06-03 11:41:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1146373002/240001

[commit-bot: I haz the power, 2015-06-03 11:44:54]

Committed patchset #14 (id:240001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196395