New issue
Advanced search Search tips

Issue 875242 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Aug 18
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug

Blocking:
issue 869973



Sign in to add a comment

sbox_integration_tests AddressSanitizerTests.TestAddressSanitizer failing on CrWinASan

Project Member Reported by thakis@chromium.org, Aug 17

Issue description

e.g. https://ci.chromium.org/buildbot/chromium.clang/CrWinAsan/1145

[ RUN      ] AddressSanitizerTests.TestAddressSanitizer
../../sandbox/win/src/address_sanitizer_test.cc(104): error: Value of: strstr(data.c_str(), &source_file_basename[last_slash])
  Actual: false
Expected: true
The stack trace doesn't have a correct filename:
=================================================================
==1476==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x11f8397001d8 at pc 0x7ff6781e122a bp 0x00a69150f380 sp 0x00a69150f388
WRITE of size 4 at 0x11f8397001d8 thread T0
    #0 0x7ff6781e1229 in AddressSanitizerTests_Report+0x229 (C:\b\s\w\ir\out\Release\sbox_integration_tests.exe+0x1229)
    #1 0x7ff6782a1b6f in sandbox::DispatchCall+0x851 (C:\b\s\w\ir\out\Release\sbox_integration_tests.exe+0xc1b6f)
    #2 0x7ff6782a45b0 in wmain+0x285 (C:\b\s\w\ir\out\Release\sbox_integration_tests.exe+0xc45b0)
    #3 0x7ff6786ea2bf in __scrt_common_main_seh f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283
    #4 0x7fff65fd2773 in BaseThreadInitThunk+0x13 (C:\Windows\System32\KERNEL32.DLL+0x12773)
    #5 0x7fff660f0d50 in RtlUserThreadStart+0x20 (C:\Windows\SYSTEM32\ntdll.dll+0x70d50)
0x11f8397001d8 is located 0 bytes to the right of 168-byte region [0x11f839700130,0x11f8397001d8)
allocated by thread T0 here:
    #0 0x7ff6786ce691 in malloc c:\b\c\b\crwinasan\src\third_party\llvm\projects\compiler-rt\lib\asan\asan_malloc_win.cc:60
    #1 0x7ff6786e73c2 in operator new f:\dd\vctools\crt\vcstartup\src\heap\new_scalar.cpp:35
    #2 0x7ff6781e10f1 in AddressSanitizerTests_Report+0xf1 (C:\b\s\w\ir\out\Release\sbox_integration_tests.exe+0x10f1)
    #3 0x7ff6782a1b6f in sandbox::DispatchCall+0x851 (C:\b\s\w\ir\out\Release\sbox_integration_tests.exe+0xc1b6f)
    #4 0x7ff6782a45b0 in wmain+0x285 (C:\b\s\w\ir\out\Release\sbox_integration_tests.exe+0xc45b0)
    #5 0x7ff6786ea2bf in __scrt_common_main_seh f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283
    #6 0x7fff65fd2773 in BaseThreadInitThunk+0x13 (C:\Windows\System32\KERNEL32.DLL+0x12773)
    #7 0x7fff660f0d50 in RtlUserThreadStart+0x20 (C:\Windows\SYSTEM32\ntdll.dll+0x70d50)
SUMMARY: AddressSanitizer: heap-buffer-overflow (C:\b\s\w\ir\out\Release\sbox_integration_tests.exe+0x1229) in AddressSanitizerTests_Report+0x229
Shadow bytes around the buggy address:
  0x0419409dffe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0419409dfff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0419409e0000: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0419409e0010: 00 00 00 00 00 00 00 00 00 00 00 00 00 fa fa fa
  0x0419409e0020: fa fa fa fa fa fa 00 00 00 00 00 00 00 00 00 00
=>0x0419409e0030: 00 00 00 00 00 00 00 00 00 00 00[fa]fa fa fa fa
  0x0419409e0040: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x0419409e0050: fd fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa
  0x0419409e0060: fa fa fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0419409e0070: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
  0x0419409e0080: 00 00 00 00 00 00 00 00 00 00 00 00 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
  Shadow gap:              cc
==1476==ABORTING
Stack trace:
Backtrace:
	testing::internal::UnitTestImpl::CurrentOsStackTraceExceptTop [0x00007FF6782DBC02+214]
	testing::internal::AssertHelper::operator= [0x00007FF6782DAB83+265]
	sandbox::AddressSanitizerTests_TestAddressSanitizer_Test::TestBody [0x00007FF6781E2A89+6199]
[  FAILED  ] AddressSanitizerTests.TestAddressSanitizer (700 ms)



First happened in https://ci.chromium.org/buildbot/chromium.clang/CrWinAsan/1092 It contains https://chromium-review.googlesource.com/1164532 which tweaked the symbol_level, which is probably the cause for this.

Do the win/asan bots need to use symbol_level=2?
 
This test, at least, requires some debug info.

Back when Timur added the test, we used gyp, and adding asan to GYP_DEFINES implied adding -gline-tables-only, which meant you always got enough debug info for stack traces like this.

I didn't find any mentions of gline-tables-only or gmlt in chromium's gn, but I thought we already had logic to add them to sanitizer builds.

Do you think it would be helpful to developers to go back to doing that, so that adding is_asan implicitly gets you stack traces? The tool is kind of useless without them.
Didn't something make -g1 and -gline-tables-only identical at some point? (Not sure.)
is_asan already implies symbol_level=1, see https://cs.chromium.org/chromium/src/build/config/compiler/compiler.gni?l=218

I think the fix is to change the code here: https://cs.chromium.org/chromium/src/build/config/compiler/BUILD.gn?l=2233
to pass -g1 if clang-cl is being used.
That's likely going to slow down all win cq bots down quite a bit, right?

(It's weird to me that we do something different on win and non-win for symbol_level=1, but I always figured we'd eventually do the windows thing everywhere, not the other way round.)
Oh and -g1 is equivalent to -gline-tables-only.

http://llvm-cs.pcc.me.uk/tools/clang/include/clang/Driver/Options.td#1751
(Which is still pretty heavy, see thread "rfc: Adding a mode to -gline-tables-only to include parameter names, namespaces" on cfe-dev)
Okay, so maybe pass -gline-tables-only at BUILD.gn:2233 only if using_sanitizer?
Yeah, that's probably the easiest thing to do for now. Want me to make a CL, or are you on it?
Can you do it please?
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 18

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

commit c728915f143bc2fc8209961a8a6b9b8f0b0fdb29
Author: Nico Weber <thakis@chromium.org>
Date: Sat Aug 18 00:01:12 2018

win/asan: Include line numbers in stack traces.

Fixes fall-out from crrev.com/581138, which in turn fixed fall-out from
crrev.com/579533 which fixed fall-out from crrev.com/579330.

This makes stacks on the win/asan bots more readable again, and also
fixes AddressSanitizerTests.TestAddressSanitizer, so that sbox_integration_tests
can run on the win-asan bot.

Bug:  875242 
Change-Id: I660f3b8d2177fca97ab71bc8be43377ed4613f92
Reviewed-on: https://chromium-review.googlesource.com/1180284
Reviewed-by: Peter Collingbourne <pcc@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584246}
[modify] https://crrev.com/c728915f143bc2fc8209961a8a6b9b8f0b0fdb29/build/config/compiler/BUILD.gn
[modify] https://crrev.com/c728915f143bc2fc8209961a8a6b9b8f0b0fdb29/testing/buildbot/chromium.memory.json
[modify] https://crrev.com/c728915f143bc2fc8209961a8a6b9b8f0b0fdb29/testing/buildbot/test_suite_exceptions.pyl

Status: Fixed (was: Untriaged)

Sign in to add a comment