New issue
Advanced search Search tips

Issue 888867 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 21
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 866610



Sign in to add a comment

v8::Isolate::GetIncumbentContext doesn't work with ASAN

Project Member Reported by yukishiino@chromium.org, Sep 25

Issue description

The implementation of v8::Isolate::GetIncumbentContext heavily depends on the machine stack, and it doesn't work well with ASAN now (probably because ASAN uses their own stack structure).

It causes that the incumbent-related layout tests don't work as expected when ASAN is enabled.

 
Blocking: 866610
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 21

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/f379bb117f8ca0cd5903ecdc7f89fc3b6d513d68

commit f379bb117f8ca0cd5903ecdc7f89fc3b6d513d68
Author: Yuki Shiino <yukishiino@chromium.org>
Date: Wed Nov 21 05:40:08 2018

Make Isolate::GetIncumbentContext() work fine with ASAN

When ASAN is enabled, the previous implementation of
Isolate::GetIncumbentContext didn't work well due to mixture of fake
and real stack frames.

This patch converts an address in the fake stack frame to an address
in the real stack frame so that we can compare two addresses.

Bug:  chromium:888867 ,  chromium:866610 
Change-Id: Iccf570b8555f2fbdc737b12894a2784ffdb31602
Reviewed-on: https://chromium-review.googlesource.com/c/1343709
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57662}
[modify] https://crrev.com/f379bb117f8ca0cd5903ecdc7f89fc3b6d513d68/src/isolate.cc
[modify] https://crrev.com/f379bb117f8ca0cd5903ecdc7f89fc3b6d513d68/test/unittests/api/isolate-unittest.cc

Cc: timothygu@chromium.org
Status: Fixed (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 7

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/134c67ef8abb2374488c71240347de58dadfe9e0

commit 134c67ef8abb2374488c71240347de58dadfe9e0
Author: Yuki Shiino <yukishiino@chromium.org>
Date: Fri Dec 07 14:52:25 2018

Make Isolate::GetIncumbentContext() work well with MSan

https://crrev.com/c/1343709 fixed GetIncumbentContext to work
with ASan, however, GetIncumbentContext didn't work well with
MSan because MSan uses a simulator which supports yet another
separate stack frame.

This patch fixes GetIncumbentContext so that it works well
with not only ASan but also MSan simply following the same way
as v8::TryCatch does.

i::GetCurrentStackPosition() solves the issue of ASan and
SafeStack (native but separate stack frame), and
i::SimulatorStack solves the issue of MSan (simulator stack
frame).

Bug:  chromium:888867 ,  chromium:866610 
Change-Id: Id803cbfd17fb1b1d9b8ee34c4802768f3a2f8e79
Reviewed-on: https://chromium-review.googlesource.com/c/1356691
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58096}
[modify] https://crrev.com/134c67ef8abb2374488c71240347de58dadfe9e0/include/v8.h
[modify] https://crrev.com/134c67ef8abb2374488c71240347de58dadfe9e0/src/api.cc
[modify] https://crrev.com/134c67ef8abb2374488c71240347de58dadfe9e0/src/isolate.cc
[modify] https://crrev.com/134c67ef8abb2374488c71240347de58dadfe9e0/src/simulator.h

Sign in to add a comment