New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 663829 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

ASAN error triggered with sampling profiler

Project Member Reported by etienneb@chromium.org, Nov 9 2016

Issue description

Version: ToT
OS: Windows 7/10 (64-bits)

The sampling profiler is triggering an ASAN error when instrumenting Chrome for checking memory accesses on a windows 7 and windows 10 computer.

NOTES: 
  * A patch is needed to remove the assert in the gn files to build it.
  * This is with clang ToT

D:\src\chromium\src>out\asan64dynamic\chrome --no-sandbox
=================================================================
==14512==ERROR: AddressSanitizer: stack-buffer-underflow on address 0x00000017dc20 at pc 0x00013fae1b45 bp 0x00000a70e94
0 sp 0x00000a70e958
READ of size 10816 at 0x00000017dc20 thread T2
    #0 0x13fae1b76 in __asan_memcpy d:\src\llvm\llvm\projects\compiler-rt\lib\asan\asan_interceptors.cc:413
    #1 0x7fecf9224bf in base::`anonymous namespace'::SuspendThreadAndRecordStack+0x21f (D:\src\chromium\src\out\asan64dy
namic\chrome.dll+0x1863f24bf)
    #2 0x7fecf921c8d in base::`anonymous namespace'::NativeStackSamplerWin::RecordStackSample D:\src\chromium\src\base\p
rofiler\native_stack_sampler_win.cc:457
    #3 0x7fecf87f542 in base::StackSamplingProfiler::SamplingThread::CollectProfile D:\src\chromium\src\base\profiler\st
ack_sampling_profiler.cc:187
    #4 0x7fecf87ee99 in base::StackSamplingProfiler::SamplingThread::CollectProfiles D:\src\chromium\src\base\profiler\s
tack_sampling_profiler.cc:217
    #5 0x7fecf87e776 in base::StackSamplingProfiler::SamplingThread::ThreadMain D:\src\chromium\src\base\profiler\stack_
sampling_profiler.cc:153
    #6 0x7fecf6dc3dc in base::`anonymous namespace'::ThreadFunc D:\src\chromium\src\base\threading\platform_thread_win.c
c:84
    #7 0x13faff374 in __asan::AsanThread::ThreadStart d:\src\llvm\llvm\projects\compiler-rt\lib\asan\asan_thread.cc:256
    #8 0x778c59cc in BaseThreadInitThunk+0xc (C:\Windows\system32\kernel32.dll+0x78d359cc)
    #9 0x779fa2e0 in RtlUserThreadStart+0x20 (C:\Windows\SYSTEM32\ntdll.dll+0x78e7a2e0)

Address 0x00000017dc20 is located in stack of thread T0 at offset 0 in frame
    #0 0x7fed0c9dc9b in net::EnsureWinsockInit D:\src\chromium\src\net\base\winsock_init.cc:44

  This frame has 1 object(s):
    [32, 440) 'wsa_data.i.i.i.i.i' <== Memory access at offset 0 partially underflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp, SEH and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-underflow d:\src\llvm\llvm\projects\compiler-rt\lib\asan\asan_interceptors.cc:41
3 in __asan_memcpy
Shadow bytes around the buggy address:
  0x000140b2fb30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000140b2fb40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000140b2fb50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000140b2fb60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000140b2fb70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x000140b2fb80: 00 00 00 00[f1]f1 f1 f1 00 00 00 00 00 00 00 00
  0x000140b2fb90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000140b2fba0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000140b2fbb0: 00 00 00 00 00 00 00 00 00 00 00 f3 f3 f3 f3 f3
  0x000140b2fbc0: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
  0x000140b2fbd0: f1 f1 f1 f1 00 f2 f2 f2 00 f2 f2 f2 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
Thread T2 created by T0 here:
    #0 0x13faf87c7 in __asan_wrap_CreateThread d:\src\llvm\llvm\projects\compiler-rt\lib\asan\asan_win.cc:129
    #1 0x7fecf6dbadf in base::PlatformThread::CreateWithPriority D:\src\chromium\src\base\threading\platform_thread_win.
cc:193
    #2 0x7fecf881858 in base::StackSamplingProfiler::Start D:\src\chromium\src\base\profiler\stack_sampling_profiler.cc:
282
    #3 0x7fecea1576e in ChromeBrowserMainParts::ChromeBrowserMainParts D:\src\chromium\src\chrome\browser\chrome_browser
_main.cc:663
    #4 0x7fece8edec8 in ChromeBrowserMainPartsWin::ChromeBrowserMainPartsWin D:\src\chromium\src\chrome\browser\chrome_b
rowser_main_win.cc:266
    #5 0x7fece8aaf54 in ChromeContentBrowserClient::CreateBrowserMainParts D:\src\chromium\src\chrome\browser\chrome_con
tent_browser_client.cc:874
    #6 0x7fecccac272 in content::BrowserMainLoop::Init D:\src\chromium\src\content\browser\browser_main_loop.cc:475
    #7 0x7fecccbfcb2 in content::BrowserMainRunnerImpl::Initialize D:\src\chromium\src\content\browser\browser_main_runn
er.cc:119
    #8 0x7fecccab8e3 in content::BrowserMain D:\src\chromium\src\content\browser\browser_main.cc:42
    #9 0x7fece81d9a1 in content::RunNamedProcessTypeMain D:\src\chromium\src\content\app\content_main_runner.cc:408
    #10 0x7fece81f64e in content::ContentMainRunnerImpl::Run D:\src\chromium\src\content\app\content_main_runner.cc:776
    #11 0x7fece81d451 in content::ContentMain D:\src\chromium\src\content\app\content_main.cc:20
    #12 0x7fec9531333 in ChromeMain D:\src\chromium\src\chrome\app\chrome_main.cc:97
    #13 0x13f6d9e82 in MainDllLoader::Launch D:\src\chromium\src\chrome\app\main_dll_loader_win.cc:174
    #14 0x13f6d1db7 in main D:\src\chromium\src\chrome\app\chrome_exe_main_win.cc:247
    #15 0x13fb0e4b4 in __scrt_common_main_seh f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:253
    #16 0x778c59cc in BaseThreadInitThunk+0xc (C:\Windows\system32\kernel32.dll+0x78d359cc)
    #17 0x779fa2e0 in RtlUserThreadStart+0x20 (C:\Windows\SYSTEM32\ntdll.dll+0x78e7a2e0)

==14512==ABORTING
 
The problem is with the std::memcpy in this function:

void SuspendThreadAndRecordStack(
    HANDLE thread_handle,
    const void* base_address,
    void* stack_copy_buffer,
    size_t stack_copy_buffer_size,
    std::vector<RecordedFrame>* stack,
    NativeStackSamplerTestDelegate* test_delegate) {

Adding the __attribute__(no_sanitize("address")) doesn't solve it.
Cc: chrisha@chromium.org
Replacing the std::memcpy seems to fix it.

Here is the full patch.

diff --git a/base/compiler_specific.h b/base/compiler_specific.h
index 0dbc3ae..13ff63b 100644
--- a/base/compiler_specific.h
+++ b/base/compiler_specific.h
@@ -149,6 +149,16 @@
 // If available, it would look like:
 //   __attribute__((format(wprintf, format_param, dots_param)))

+// Sanitizers annotations.
+#ifdef __has_attribute
+#if __has_attribute(no_sanitize)
+#define NO_SANITIZE(what) __attribute__((no_sanitize(what)))
+#endif
+#endif
+#ifndef NO_SANITIZE
+#define NO_SANITIZE(what)
+#endif
+
 // MemorySanitizer annotations.
 #if defined(MEMORY_SANITIZER) && !defined(OS_NACL)
 #include <sanitizer/msan_interface.h>
diff --git a/base/profiler/native_stack_sampler_win.cc b/base/profiler/native_stack_sampler_win.cc
index 063374f..6d0f331 100644
--- a/base/profiler/native_stack_sampler_win.cc
+++ b/base/profiler/native_stack_sampler_win.cc
@@ -319,7 +319,7 @@ void SuspendThreadAndRecordStack(
     void* stack_copy_buffer,
     size_t stack_copy_buffer_size,
     std::vector<RecordedFrame>* stack,
-    NativeStackSamplerTestDelegate* test_delegate) {
+    NativeStackSamplerTestDelegate* test_delegate) NO_SANITIZE("address") {
   DCHECK(stack->empty());

   CONTEXT thread_context = {0};
@@ -353,8 +353,11 @@ void SuspendThreadAndRecordStack(
     if (PointsToGuardPage(bottom))
       return;

-    std::memcpy(stack_copy_buffer, reinterpret_cast<const void*>(bottom),
-                top - bottom);
+    for (size_t pos = 0; pos < top - bottom; ++pos)
+      reinterpret_cast<char*>(stack_copy_buffer)[pos] =
+          reinterpret_cast<const char*>(bottom)[pos];
   }

   if (test_delegate)
@@ -385,7 +388,7 @@ class NativeStackSamplerWin : public NativeStackSampler {
     // reserved stack size is 1 MB and Chrome Windows threads currently always
     // use the default, but this allows for expansion if it occurs. The size
     // beyond the actual stack size consists of unallocated virtual memory pages
-    // so carries little cost (just a bit of wated address space).
+    // so carries little cost (just a bit of wasted address space).
     kStackCopyBufferSize = 2 * 1024 * 1024
   };

diff --git a/build/config/sanitizers/BUILD.gn b/build/config/sanitizers/BUILD.gn
index 36bceec..7c925bf 100644
--- a/build/config/sanitizers/BUILD.gn
+++ b/build/config/sanitizers/BUILD.gn
@@ -323,10 +323,10 @@ config("link_executable") {
   if (is_asan && is_win && !is_component_build) {
     if (target_cpu == "x64") {
       # Windows 64-bit. TODO(etienneb): Remove the assert when this is ready.
-      assert(false, "win/asan does not work in 64-bit yet")
+      # assert(false, "win/asan does not work in 64-bit yet")
       libs = [ "clang_rt.asan-x86_64.lib" ]
     } else {
-      assert(target_cpu == "x86", "WinASan unsupported architecture")
+      # assert(target_cpu == "x86", "WinASan unsupported architecture")
       libs = [ "clang_rt.asan-i386.lib" ]
     }
   }
@@ -336,10 +336,10 @@ config("link_shared_library") {
   if (is_asan && is_win && !is_component_build) {
     if (target_cpu == "x64") {
       # Windows 64-bit. TODO(etienneb): Remove the assert when this is ready.
-      assert(false, "win/asan does not work in 64-bit yet")
+      # assert(false, "win/asan does not work in 64-bit yet")
       libs = [ "clang_rt.asan_dll_thunk-x86_64.lib" ]
     } else {
-      assert(target_cpu == "x86", "WinASan unsupported architecture")
+      # assert(target_cpu == "x86", "WinASan unsupported architecture")
       libs = [ "clang_rt.asan_dll_thunk-i386.lib" ]
     }
   }

Comment 4 by r...@chromium.org, Nov 9 2016

Cc: kcc@chromium.org vitalyb...@chromium.org euge...@chromium.org
Evgeniy, do you remember the right way to do an unsanitized memcpy? I can't find the code in LLVM that I thought controlled this.
I don't think we have anything better than this.
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 18 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4e9250a7663d37a3490b2aa936a0b634af518670

commit 4e9250a7663d37a3490b2aa936a0b634af518670
Author: etienneb <etienneb@chromium.org>
Date: Fri Nov 18 18:47:53 2016

Fix false-positive ASAN detection with the stack profiler on win64

The sampling profiler is triggering an ASAN error when instrumenting Chrome for checking memory accesses on a windows 7 and windows 10 computer.

=================================================================
==14512==ERROR: AddressSanitizer: stack-buffer-underflow on address 0x00000017dc20 at pc 0x00013fae1b45 bp 0x00000a70e940 sp 0x00000a70e958
READ of size 10816 at 0x00000017dc20 thread T2
    #0 0x13fae1b76 in __asan_memcpy d:\src\llvm\llvm\projects\compiler-rt\lib\asan\asan_interceptors.cc:413
    #1 0x7fecf9224bf in base::`anonymous namespace'::SuspendThreadAndRecordStack+0x21f (D:\src\chromium\src\out\asan64dynamic\chrome.dll+0x1863f24bf)
    #2 0x7fecf921c8d in base::`anonymous namespace'::NativeStackSamplerWin::RecordStackSample D:\src\chromium\src\base\profiler\native_stack_sampler_win.cc:457

[...]

  This frame has 1 object(s):
    [32, 440) 'wsa_data.i.i.i.i.i' <== Memory access at offset 0 partially underflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp, SEH and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-underflow d:\src\llvm\llvm\projects\compiler-rt\lib\asan\asan_interceptors.cc:41

R=wittman@chromium.org, mark@chromium.org, rnk@google.com, thakis@google.com

BUG= 663829 

Review-Url: https://codereview.chromium.org/2495413002
Cr-Commit-Position: refs/heads/master@{#433256}

[modify] https://crrev.com/4e9250a7663d37a3490b2aa936a0b634af518670/base/compiler_specific.h
[modify] https://crrev.com/4e9250a7663d37a3490b2aa936a0b634af518670/base/profiler/native_stack_sampler_win.cc

Status: Fixed (was: Assigned)

Sign in to add a comment