Use new ASAN APIs for scanning ASAN fake stacks during garbage collection.

Use new ASAN APIs for scanning ASAN fake stacks during garbage collection.

BUG=339813
NOTRY=true

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

[kcc1, 2014-02-06 13:44:06]

LGTM with nits

https://codereview.chromium.org/141713008/diff/20001/Source/heap/AddressSanit...
File Source/heap/AddressSanitizer.h (right):

https://codereview.chromium.org/141713008/diff/20001/Source/heap/AddressSanit...
Source/heap/AddressSanitizer.h:37: #if defined(ADDRESS_SANITIZER) && !OS(WIN)
remember that this part depends on 
https://code.google.com/p/chromium/issues/detail?id=340196
https://code.google.com/p/chromium/issues/detail?id=341352

https://codereview.chromium.org/141713008/diff/20001/Source/heap/ThreadState.cpp
File Source/heap/ThreadState.cpp (right):

https://codereview.chromium.org/141713008/diff/20001/Source/heap/ThreadState....
Source/heap/ThreadState.cpp:314: NO_SANITIZE_ADDRESS
I'd put #ifdef inside the function body, this will get rid of most other ifdefs.
But that's up to you

https://codereview.chromium.org/141713008/diff/20001/Source/heap/ThreadState.h
File Source/heap/ThreadState.h (right):

https://codereview.chromium.org/141713008/diff/20001/Source/heap/ThreadState....
Source/heap/ThreadState.h:496: #if defined(ADDRESS_SANITIZER) && !OS(WIN)
Why !OS(WIN)?
asan will be available on Windows eventually

[Vyacheslav Egorov (Chromium), 2014-02-06 13:45:56]

lgtm

[Mads Ager (chromium), 2014-02-06 14:15:17]

Thanks for the reviews guys. As Kostya notes this is blocked by an update of
clang for all platforms on which we run asan. I'll land when the updates have
gone through.

https://codereview.chromium.org/141713008/diff/20001/Source/heap/AddressSanit...
File Source/heap/AddressSanitizer.h (right):

https://codereview.chromium.org/141713008/diff/20001/Source/heap/AddressSanit...
Source/heap/AddressSanitizer.h:37: #if defined(ADDRESS_SANITIZER) && !OS(WIN)
On 2014/02/06 13:44:06, kcc1 wrote:
> remember that this part depends on 
> https://code.google.com/p/chromium/issues/detail?id=340196
> https://code.google.com/p/chromium/issues/detail?id=341352

Yes, I will. I'll land the change to use this header separately as soon as I
can. Then I will land this change on top when all the compilers have been
updated to the version containing the new APIs.

https://codereview.chromium.org/141713008/diff/20001/Source/heap/ThreadState.cpp
File Source/heap/ThreadState.cpp (right):

https://codereview.chromium.org/141713008/diff/20001/Source/heap/ThreadState....
Source/heap/ThreadState.cpp:314: NO_SANITIZE_ADDRESS
On 2014/02/06 13:44:06, kcc1 wrote:
> I'd put #ifdef inside the function body, this will get rid of most other
ifdefs.
> But that's up to you

Good idea. Done!

https://codereview.chromium.org/141713008/diff/20001/Source/heap/ThreadState.h
File Source/heap/ThreadState.h (right):

https://codereview.chromium.org/141713008/diff/20001/Source/heap/ThreadState....
Source/heap/ThreadState.h:496: #if defined(ADDRESS_SANITIZER) && !OS(WIN)
On 2014/02/06 13:44:06, kcc1 wrote:
> Why !OS(WIN)?
> asan will be available on Windows eventually

SyZyAsan uses the same define and they do not have this API (and they don't have
a fake stack). So, we shouldn't attempt to do this on Windows.

[wibling-chromium, 2014-02-07 10:13:25]

lgtm

[zerny-chromium, 2014-02-07 12:07:15]

lgtm2

[haraken, 2014-02-10 05:35:31]

LGTM, cq+ed.

[haraken, 2014-02-10 05:35:34]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-02-10 05:35:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ager@chromium.org/141713008/100001

[commit-bot: I haz the power, 2014-02-10 05:35:57]

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

[commit-bot: I haz the power, 2014-02-10 05:35:57]

Failed to apply patch for Source/heap/ThreadState.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/heap/ThreadState.cpp
  Hunk #1 FAILED at 240.
  Hunk #2 succeeded at 333 (offset 25 lines).
  1 out of 2 hunks FAILED -- saving rejects to file
Source/heap/ThreadState.cpp.rej

Patch:       Source/heap/ThreadState.cpp
Index: Source/heap/ThreadState.cpp
diff --git a/Source/heap/ThreadState.cpp b/Source/heap/ThreadState.cpp
index
e955142c39f0b452b580234323b5db17d523d097..55d827a1796ed19d1c6fb91165519aeaee5f0a11
100644
--- a/Source/heap/ThreadState.cpp
+++ b/Source/heap/ThreadState.cpp
@@ -240,6 +240,9 @@ ThreadState::ThreadState()
     , m_noAllocationCount(0)
     , m_inGC(false)
     , m_heapContainsCache(new HeapContainsCache())
+#if defined(ADDRESS_SANITIZER) && !OS(WIN)
+    , m_asanFakeStack(__asan_get_current_fake_stack())
+#endif
 {
     ASSERT(!**s_threadSpecific);
     **s_threadSpecific = this;
@@ -308,15 +311,46 @@ void ThreadState::visitRoots(Visitor* visitor)
 }
 
 NO_SANITIZE_ADDRESS
+void ThreadState::visitAsanFakeStackForPointer(Visitor* visitor, Address ptr)
+{
+#if defined(ADDRESS_SANITIZER) && !OS(WIN)
+    Address* start = reinterpret_cast<Address*>(m_startOfStack);
+    Address* end = reinterpret_cast<Address*>(m_endOfStack);
+    Address* fakeFrameStart = 0;
+    Address* fakeFrameEnd = 0;
+    Address* maybeFakeFrame = reinterpret_cast<Address*>(ptr);
+    Address* realFrameForFakeFrame =
+        reinterpret_cast<Address*>(
+            __asan_addr_is_in_fake_stack(
+                m_asanFakeStack, maybeFakeFrame,
+                reinterpret_cast<void**>(&fakeFrameStart),
+                reinterpret_cast<void**>(&fakeFrameEnd)));
+    if (realFrameForFakeFrame) {
+        // This is a fake frame from the asan fake stack.
+        if (realFrameForFakeFrame > end && start > realFrameForFakeFrame) {
+            // The real stack address for the asan fake frame is
+            // within the stack range that we need to scan so we need
+            // to visit the values in the fake frame.
+            for (Address* p = fakeFrameStart; p < fakeFrameEnd; p++)
+                Heap::checkAndMarkPointer(visitor, *p);
+        }
+    }
+#endif
+}
+
+NO_SANITIZE_ADDRESS
 void ThreadState::visitStack(Visitor* visitor)
 {
     Address* end = reinterpret_cast<Address*>(m_startOfStack);
     for (Address* current = reinterpret_cast<Address*>(m_endOfStack); current <
end; ++current) {
         Heap::checkAndMarkPointer(visitor, *current);
+        visitAsanFakeStackForPointer(visitor, *current);
     }
 
-    for (Vector<Address>::iterator it = m_safePointStackCopy.begin(); it !=
m_safePointStackCopy.end(); ++it)
+    for (Vector<Address>::iterator it = m_safePointStackCopy.begin(); it !=
m_safePointStackCopy.end(); ++it) {
         Heap::checkAndMarkPointer(visitor, *it);
+        visitAsanFakeStackForPointer(visitor, *it);
+    }
 }
 
 void ThreadState::visitPersistents(Visitor* visitor)

[Mads Ager (chromium), 2014-04-02 09:59:36]

Yunlian, this is the change that is currently blocked by a clang update for
chromeos. This has been blocked for almost two months now so it would be great
if I can get it in soon. How far are you with the roll? Would it be acceptable
for us to break the asan build on chromeos until you roll?

[Nico, 2014-04-03 00:15:25]

On 2014/04/02 09:59:36, Mads Ager (chromium) wrote:
> Yunlian, this is the change that is currently blocked by a clang update for
> chromeos. This has been blocked for almost two months now so it would be great
> if I can get it in soon. How far are you with the roll? Would it be acceptable
> for us to break the asan build on chromeos until you roll?

The cros clang update is in the cq and should land once the cros tree opens:
https://chromium-review.googlesource.com/#/c/189842/

[Mads Ager (chromium), 2014-04-04 09:10:25]

clang has rolled everywhere. Landing!

[haraken, 2014-04-04 09:13:08]

On 2014/04/04 09:10:25, Mads Ager (chromium) wrote:
> clang has rolled everywhere. Landing!

That's awesome!

[Mads Ager (chromium), 2014-04-04 09:14:57]

The CQ bit was checked by ager@chromium.org

[commit-bot: I haz the power, 2014-04-04 09:15:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ager@chromium.org/141713008/260001

[wibling-chromium, 2014-04-04 09:15:18]

lgtm

[commit-bot: I haz the power, 2014-04-04 10:28:56]

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

[commit-bot: I haz the power, 2014-04-04 10:29:00]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_dbg

[Mads Ager (chromium), 2014-04-04 10:29:49]

The CQ bit was checked by ager@chromium.org

[commit-bot: I haz the power, 2014-04-04 10:29:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ager@chromium.org/141713008/260001

[commit-bot: I haz the power, 2014-04-04 11:52:14]

Change committed as 170856