New issue
Advanced search Search tips

Issue 598761 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Feature

Blocking:
issue 82385
issue 498033



Sign in to add a comment

gn should support the Windows ASan config

Project Member Reported by r...@chromium.org, Mar 29 2016

Issue description

Specifically, the arguments for this configuration are:

is_clang = true
is_asan = true
target_cpu = "x86" # WinASan is currently 32-bit only
is_debug = false

I have some patches for this, but I figure it will be an ongoing effort so I'm filing this bug to track it.
 

Comment 1 by thakis@chromium.org, Mar 29 2016

Blocking: 82385
Blocking: 354261
Labels: Proj-GN-Migration
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 29 2016

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

commit 9065ac8a2b45154c8e5dbef97bddd01829680f88
Author: rnk <rnk@chromium.org>
Date: Tue Mar 29 20:42:53 2016

Build crash_service_win64 with clang-cl when is_clang=true

Otherwise this gn assertion fails:
  ERROR at //build/config/sanitizers/sanitizers.gni:85:1: Assertion failed.
  assert(!using_sanitizer || is_clang,
  ^-----
  Sanitizers (is_*san) require setting is_clang = true in 'gn args'

R=dpranke@chromium.org,scottmg@chromium.org
BUG= 598761 

Review URL: https://codereview.chromium.org/1838953003

Cr-Commit-Position: refs/heads/master@{#383808}

[modify] https://crrev.com/9065ac8a2b45154c8e5dbef97bddd01829680f88/chrome/tools/crash_service/BUILD.gn

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 30 2016

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

commit 3dbbe337ce92c737b5c5c497c5ceb7fd4c81f85f
Author: rnk <rnk@chromium.org>
Date: Wed Mar 30 16:08:25 2016

Fix sandbox ASan test to cope with forward slashes in __FILE__

gn invokes the compiler with forward slash paths while gyp does not.
This test failed when I ported WinASan to gn.

This CL doesn't use base::FilePath::BaseName because then I'd need to
get a wide string __FILE__, which is tricky.

R=wfh@chromium.org
BUG= 598761 

Review URL: https://codereview.chromium.org/1837403002

Cr-Commit-Position: refs/heads/master@{#383990}

[modify] https://crrev.com/3dbbe337ce92c737b5c5c497c5ceb7fd4c81f85f/sandbox/win/src/address_sanitizer_test.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 30 2016

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

commit 3dbbe337ce92c737b5c5c497c5ceb7fd4c81f85f
Author: rnk <rnk@chromium.org>
Date: Wed Mar 30 16:08:25 2016

Fix sandbox ASan test to cope with forward slashes in __FILE__

gn invokes the compiler with forward slash paths while gyp does not.
This test failed when I ported WinASan to gn.

This CL doesn't use base::FilePath::BaseName because then I'd need to
get a wide string __FILE__, which is tricky.

R=wfh@chromium.org
BUG= 598761 

Review URL: https://codereview.chromium.org/1837403002

Cr-Commit-Position: refs/heads/master@{#383990}

[modify] https://crrev.com/3dbbe337ce92c737b5c5c497c5ceb7fd4c81f85f/sandbox/win/src/address_sanitizer_test.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 31 2016

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

commit dce97597a49d6238e2dc041e2c80d89c3558dd1f
Author: rnk <rnk@chromium.org>
Date: Thu Mar 31 19:41:10 2016

Add support for the Windows ASan configuration to the gn build

This is mostly just removing a check for "if (is_posix)" and some
minor cflags fixups.

R=dpranke@chromium.org,eugenis@chromium.org
BUG= 598761 

Review URL: https://codereview.chromium.org/1839763006

Cr-Commit-Position: refs/heads/master@{#384360}

[modify] https://crrev.com/dce97597a49d6238e2dc041e2c80d89c3558dd1f/build/config/sanitizers/BUILD.gn
[modify] https://crrev.com/dce97597a49d6238e2dc041e2c80d89c3558dd1f/build/config/win/BUILD.gn

Comment 7 by thakis@chromium.org, Apr 12 2016

Blocking: 498033

Comment 8 by thakis@chromium.org, Apr 12 2016

Is this done?

Comment 9 by r...@chromium.org, Apr 12 2016

I think we still need to deal with this:
https://codereview.chromium.org/1839763006/diff/1/build/config/sanitizers/BUILD.gn#newcode203

I *think* Chrome builds with /MT when is_component_build=false, and chrome_child.dll and chrome.dll need to link clang_rt.asan_dll_thunk-i386.lib.
Components: Build
Blocking: -354261
As suggested by laforge@ and a conversation w/ the monorail folks, I'm going to try tracking GN-Migration related issues by *just* using the Proj-GN-Migration label, and not using blocking/rollup bugs, so that we can use blocking for just tasks that truly need to be completed before other tasks can make progress.

Comment 12 by r...@chromium.org, Jul 26 2016

Cc: etienneb@chromium.org
Owner: ----
I haven't been working on this, but I forgot to update the bug.
Cc: h...@chromium.org mbarbe...@chromium.org
Labels: -Pri-3 Pri-1
The Win ASAN builders are some of the very last builders to still require GYP. We would like to stop GYP support ASAP, i.e., a couple days from now.

Can someone please prioritize looking at this? If not, we're likely to just break you.
I'm currently looking to this.
I have a few patches to land to fix.
Tests are on-going...
Courgette build was causing trouble when generating the build files.

There is still some issues with the not-shared build and some DLL that needs to be copied.

Comment 16 by r...@chromium.org, Jul 28 2016

P1 seems reasonable, ClusterFuzz uses these builds.
Here is the first patch:
  https://codereview.chromium.org/2192833002/

I'm gonna continue to chase why the static build is not working.
And, there is still a missing copy action for the runtime.
With clang 4.0.0 and chromium ToT, for a 32-bit build
There is a security check failing. I'm gonna dig this, but I launch a build with the current clang version provided with chromium.

stay tuned.

----------------------
D:\src\chromium\src>ninja -C out\asan chrome
ninja: Entering directory `out\asan'
[1/8290] ACTION //v8:run_mksnapshot(//build/toolchain/win:clang_x86)
FAILED: gen/v8/snapshot.cc snapshot_blob.bin
D:/src/depot_tools/python276_bin/python.exe ../../v8/tools/run.py ./mksnapshot --startup_src gen/v8/snapshot.cc --random
-seed 314159265 --startup_blob snapshot_blob.bin
ninja: build stopped: subcommand failed.

I think there are some fixes needed to turn off asan in the host toolchain builds (that I've made locally). I'm actually a bit puzzled that you'd be able to do much w/o them. I'll post them later today when I'm in the office. I did also see the missing copy.
As I get it, even if the host toolchain is instrumenting the code, it should work anyway. That's only mean we are checking our own tools against invalid memory accesses.

FYI, for a 64-bit, I do not need to apply any other fixes.
(With clang 4.0.0 ToT + my local patch).
Ah, unless there is issue to target x86 code on a x64 host.
This may explained.
I changed:
  if (current_cpu == "x64") {
for:
  if (target_cpu == "x64") {

And, the genstrings.exe seems to work.
I'm starting a fresh build to see if it's fine. (+/- 45min)
Owner: etienneb@chromium.org
Okay, as noted on 

https://codereview.chromium.org/2192833002/

some combination of that CL plus a change to copy the asan runtime into the root_build_dir plus possibly my other two CLs to mess w/ toolchain args produces a working release component build. I think that I might be able to just tweak things a bit more to get a working static build, but haven't tried it yet.

I CQ'ed etienneb's change, so we can probably try flipping the builds to at least a shared-release build and see what happens over the weekend, and fix whatever issues arise on/by Monday, so we'll have working (shared) win asan at some point on monday, with minimal gaps in coverage.
Project Member

Comment 24 by bugdroid1@chromium.org, Jul 30 2016

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

commit 364deb713883f28d2647c5ff160c1a6f7b7ef342
Author: etienneb <etienneb@chromium.org>
Date: Sat Jul 30 03:43:42 2016

Fix GN generation for WinASAN build

This is a step toward a working Asan build with GN.
There are still a few missing pieces to land.

This patch is fixing an assert triggering due to courgette using an
harcoded toolchains.

----------------------
 ERROR at //build/config/sanitizers/sanitizers.gni:12:1: Assertion failed.
  assert(!using_sanitizer || is_clang,
  ^-----
  Sanitizers (is_*san) require setting is_clang = true in 'gn args'
----------------------

This patch is also fixing the compiler-rt runtime paths for a x64 bit
build. The port of WinASAN-64 is almost done and should part of the next
clang roll-deps.

R=thakis@chromium.org, dpranke@chromium.org
TBR=brettw@chromium.org
BUG= 598761 

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

[modify] https://crrev.com/364deb713883f28d2647c5ff160c1a6f7b7ef342/BUILD.gn
[modify] https://crrev.com/364deb713883f28d2647c5ff160c1a6f7b7ef342/base/BUILD.gn
[modify] https://crrev.com/364deb713883f28d2647c5ff160c1a6f7b7ef342/base/allocator/BUILD.gn
[modify] https://crrev.com/364deb713883f28d2647c5ff160c1a6f7b7ef342/build/config/BUILDCONFIG.gn
[modify] https://crrev.com/364deb713883f28d2647c5ff160c1a6f7b7ef342/build/config/sanitizers/BUILD.gn
[modify] https://crrev.com/364deb713883f28d2647c5ff160c1a6f7b7ef342/courgette/BUILD.gn

Clang ASAN build is currently broken after GN migration. Are you planning a fix for this.

https://build.chromium.org/p/chromium.lkgr/builders/Win%20ASan%20Release/builds/2677/steps/generate_build_files/logs/stdio
Yes, etienneb and I have been working on it; the latest patch is https://codereview.chromium.org/2198333002/ but I don't think this works for static builds yet, just shared. 

Hopefully we'll have this working tomorrow.
With my previous patch, I've got this error:
[https://codereview.chromium.org/2196403002]


[17733/30495] ACTION //v8:run_mksnapshot(//build/toolchain/win:clang_x86)
FAILED: gen/v8/snapshot.cc snapshot_blob.bin
D:/src/depot_tools/python276_bin/python.exe ../../v8/tools/run.py ./mksnapshot --startup_src gen/v8/snapshot.cc --random
-seed 314159265 --startup_blob snapshot_blob.bin
=================================================================
==21356==ERROR: AddressSanitizer: use-after-poison on address 0x0085e160 at pc 0x012bea5b bp 0xdeadbeef sp 0x0044cc18
WRITE of size 4 at 0x0085e160 thread T0
AddressSanitizer: nested bug in the same thread, aborting.

ninja: build stopped: subcommand failed.
Yup, that's the same error I get.
I have both executable failing (v8_shell and mksnapshot) and strangely they are failing at the same place.

D:\src\chromium\src\v8\src\builtins\builtins.cc
  void Builtins::SetUp(Isolate* isolate, bool create_heap_objects) {
     [....]

It's failing when initializing V8 builtins.
I can't tell exactly where I do not have debug symbols and there is tons of macro.

I'm still digging.
A more detailed stack-frame:

=================================================================
==25700==ERROR: AddressSanitizer: use-after-poison on address 0x0741b558 at pc 0x0fa3e5ae bp 0xdeadbeef sp 0x004533e8
WRITE of size 368 at 0x0741b558 thread T0
    #0 0xfa3e5c4  (D:\src\chromium\src\out\asan\clang_rt.asan_dynamic-i386.dll+0x1001e5c4)
    #1 0x19accef in v8::internal::compiler::InstructionSelector::InstructionSelector(class v8::internal::Zone *,unsigned
 int,class v8::internal::compiler::Linkage *,class v8::internal::compiler::InstructionSequence *,class v8::internal::com
piler::Schedule *,class v8::internal::compiler::SourcePositionTable *,class v8::internal::compiler::Frame *,enum v8::int
ernal::compiler::InstructionSelector::SourcePositionMode,class v8::internal::compiler::InstructionSelector::Features) D:
\src\chromium\src\v8\src\compiler\instruction-selector.cc:37:7
    #2 0x1bcd91f in v8::internal::compiler::InstructionSelectionPhase::Run(class v8::internal::compiler::PipelineData *,
class v8::internal::Zone *,class v8::internal::compiler::Linkage *) D:\src\chromium\src\v8\src\compiler\pipeline.cc:1192
:25
    #3 0x1bab5e1 in v8::internal::compiler::PipelineImpl::Run<struct v8::internal::compiler::InstructionSelectionPhase,c
lass v8::internal::compiler::Linkage *>(class v8::internal::compiler::Linkage *) D:\src\chromium\src\v8\src\compiler\pip
eline.cc:714:9
    #4 0x1b9f3e9 in v8::internal::compiler::PipelineImpl::ScheduleAndSelectInstructions(class v8::internal::compiler::Li
nkage *) D:\src\chromium\src\v8\src\compiler\pipeline.cc:1713:3
    #5 0x1ba39d2 in v8::internal::compiler::Pipeline::GenerateCodeForCodeStub(class v8::internal::Isolate *,class v8::in
ternal::compiler::CallDescriptor *,class v8::internal::compiler::Graph *,class v8::internal::compiler::Schedule *,unsign
ed int,char const *) D:\src\chromium\src\v8\src\compiler\pipeline.cc:1590
    #6 0x18c0bbd in v8::internal::compiler::CodeAssembler::GenerateCode(void) D:\src\chromium\src\v8\src\compiler\code-a
ssembler.cc:71:23
    #7 0x1762f20 in v8::internal::TurboFanCodeStub::GenerateCode(void) D:\src\chromium\src\v8\src\code-stubs.cc:419:20
    #8 0x1758e59 in v8::internal::CodeStub::GetCode(void) D:\src\chromium\src\v8\src\code-stubs.cc:155:31
    #9 0x34055b0 in v8::internal::MacroAssembler::TailCallStub(class v8::internal::CodeStub *) D:\src\chromium\src\v8\sr
c\ia32\macro-assembler-ia32.cc:2143:13
    #10 0x33b8062 in v8::internal::CreateArrayDispatchOneArgument D:\src\chromium\src\v8\src\ia32\code-stubs-ia32.cc:407
4:10
    #11 0x33b7398 in v8::internal::ArrayConstructorStub::GenerateDispatchToArrayStub(class v8::internal::MacroAssembler
*,enum v8::internal::AllocationSiteOverrideMode) D:\src\chromium\src\v8\src\ia32\code-stubs-ia32.cc:4132:5
    #12 0x33b8889 in v8::internal::ArrayConstructorStub::Generate(class v8::internal::MacroAssembler *) D:\src\chromium\
src\v8\src\ia32\code-stubs-ia32.cc:4194:3
    #13 0x1759fb7 in v8::internal::PlatformCodeStub::GenerateCode(void) D:\src\chromium\src\v8\src\code-stubs.cc:129:5
    #14 0x1758e59 in v8::internal::CodeStub::GetCode(void) D:\src\chromium\src\v8\src\code-stubs.cc:155:31
    #15 0x34055b0 in v8::internal::MacroAssembler::TailCallStub(class v8::internal::CodeStub *) D:\src\chromium\src\v8\s
rc\ia32\macro-assembler-ia32.cc:2143:13
    #16 0x323e90c in v8::internal::Builtins::Generate_ArrayCode(class v8::internal::MacroAssembler *) D:\src\chromium\sr
c\v8\src\builtins\ia32\builtins-ia32.cc:1566:6
    #17 0x16879f1 in v8::internal::`anonymous namespace'::BuildWithMacroAssembler D:\src\chromium\src\v8\src\builtins\bu
iltins.cc:55:3
    #18 0x166da25 in v8::internal::Builtins::SetUp(class v8::internal::Isolate *,bool) D:\src\chromium\src\v8\src\builti
ns\builtins.cc:151:5
    #19 0x25a6b41 in v8::internal::Isolate::Init(class v8::internal::Deserializer *) D:\src\chromium\src\v8\src\isolate.
cc:2385:13
    #20 0x1332571 in v8::V8::CreateSnapshotDataBlob(char const *) D:\src\chromium\src\v8\src\api.cc:551
    #21 0x12f178b in main D:\src\chromium\src\v8\src\snapshot\mksnapshot.cc:164:24
    #22 0x3822b58 in _scrt_common_main_seh f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:255
    #23 0x75fd3389  (C:\Windows\syswow64\kernel32.dll+0x7dd73389)
    #24 0x77719901  (C:\Windows\SysWOW64\ntdll.dll+0x7dea9901)
    #25 0x777198d4  (C:\Windows\SysWOW64\ntdll.dll+0x7dea98d4)
I'll poke at this some.
This may not be related, but the interception was incorrect when I was using the clang ToT. A breakpoint was triggered when using the tool mksnapshot.

Patch: https://reviews.llvm.org/D23081

I was trying to build the base_unittests with is_asan on my win7 computer.

===========================
is_debug = false
enable_nacl = false
is_clang = true
is_asan = true
clang_base_path = "d:\src\llvm\ninja64"
clang_use_chrome_plugins = false
fwiw in gyp we only enable asan and friends in target binaries (https://chromiumcodereview.appspot.com/10392105) (to my surprise, i thought it's on everywhere)
I spent time debugging why the mksnapshot is crashing with an Asan error.

The code is crashing here:

Code* BuildWithCodeStubAssemblerCS(Isolate* isolate,
                                   CodeAssemblerGenerator generator,
                                   CallDescriptors::Key interface_descriptor,
                                   Code::Flags flags, const char* name) {
  HandleScope scope(isolate);
  Zone zone(isolate->allocator());   // ALLOCATION
  [...]
  CodeStubAssembler assembler(isolate, &zone, descriptor, flags, name);  // CRASH
}


I decided to disable the V8 Asan interaction by commenting this line:
#define V8_USE_ADDRESS_SANITIZER 1

https://cs.chromium.org/chromium/src/v8/src/base/macros.h?q=V8_USE_ADDRESS_SANITIZER&sq=package:chromium&dr=CSs&l=146


And, the error is gone.
I don't know enough on V8 <-> ASAN but it seems to me there is something wrong here.
As mentioned in comment 33, I think we should only add the -fsanitize flags if current_toolchain == target_toolchain for now.
(which will work around the mksnapshot issue)
Nevermind; we didn't set GYP_CROSSCOMPILE on windows with gyp, so we did build mksnapshot with asan there.
put differently, target_toolchain == v8_snapshot_toolchain (we don't build v8 twice).

I don't know how this worked in GYP, but it seems like there must've been some other explanation.
Project Member

Comment 39 by bugdroid1@chromium.org, Aug 2 2016

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

commit cb4bc9e68fdefc6ebd44e20b558a465a98cf0a55
Author: rnk <rnk@chromium.org>
Date: Tue Aug 02 23:14:14 2016

Copy ASan runtime to buildroot on Windows as well as Mac

Otherwise executables with ASan instrumentation fail to start.

R=etienneb@chromium.org,thakis@chromium.org,dpranke@chromium.org
BUG= 598761 

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

[modify] https://crrev.com/cb4bc9e68fdefc6ebd44e20b558a465a98cf0a55/build/config/sanitizers/BUILD.gn

Project Member

Comment 40 by bugdroid1@chromium.org, Aug 3 2016

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

commit 4a42af48e74062a70c97ae54438218fbb23ecd53
Author: rnk <rnk@chromium.org>
Date: Wed Aug 03 01:19:48 2016

Move clang_version setting back into declare_args

This allows the user to indicate that their clang version is something
other than the default. Right now we guess 3.9.0 for the clang version,
but TOT LLVM is on version 4.0.0.

I use this to test changes to the ASan runtime in Chrome.

R=hans@chromium.org,etienneb@chromium.org,dpranke@chromium.org
BUG= 598761 

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

[modify] https://crrev.com/4a42af48e74062a70c97ae54438218fbb23ecd53/build/toolchain/toolchain.gni

The mini_installer is also causing trouble because it's removing the lib we are trying to add (dpranke@ patch).

executable(target_name) {
    output_name = "mini_installer"

    [...]

    configs -= [
      "//build/config/compiler:default_optimization",
      "//build/config:executable_config",     //// HERE !!!!
      "//build/config/win:console",
    ]
With the dpranke@ patch, the V8 duct-tape, I was able to compile the same targets than the build bot:

angle_unittests base_unittests browser_tests cacheinvalidation_unittests capture_unittests chrome_app_unittests chrome_elf_unittests components_unittests content_browsertests content_unittests courgette_unittests crypto_unittests device_unittests extensions_unittests gcm_unit_tests google_apis_unittests gpu_ipc_service_unittests gpu_unittests installer_util_unittests ipc_tests jingle_unittests media_blink_unittests media_unittests midi_unittests net_unittests ppapi_unittests printing_unittests remoting_unittests sbox_integration_tests sbox_unittests sbox_validation_tests setup_unittests skia_unittests sql_unittests sync_unit_tests ui_base_unittests unit_tests url_unittests views_unittests

My current config is:
target_cpu = "x86"
is_debug = false
is_asan = true
is_clang = true
enable_nacl = false
I think I've found (part of) the problem with V8.

https://cs.chromium.org/chromium/src/v8/src/zone.cc?q=zone.cc&sq=package:chromium&dr&l=147

If you lift out the line before the ifdef, it's working.
See line 165.

But, I can't explain why the call to DeleteSegment is not unpoisoning the memory chunk.

  DeleteSegment(current, size);
    allocator_->Free(segment, size);

I single step into the allocator.
Which bring us here:
  https://cs.chromium.org/chromium/src/v8/src/base/accounting-allocator.cc?q=AccountingAllocator::Allocate&sq=package:chromium&dr=CSs&l=17

The malloc wasn't intercepted. Which explain why my fix is working.
The malloc/free are not touching the shadow memory, but V8 is.
So, cleaning the shadow hide this invalid hooking.

With https://codereview.chromium.org/2208093003/ things are looking fairly good for me locally.
Project Member

Comment 46 by bugdroid1@chromium.org, Aug 3 2016

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

commit 640fe86764317ab19f855b06da72ac80c39296d6
Author: thakis <thakis@chromium.org>
Date: Wed Aug 03 23:26:20 2016

Only bundle_data asan runtime on mac.

bundle_data is a noop on Windows, so no behavior change, but a bit nicer.
Patch from dpranke@

BUG= 598761 

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

[modify] https://crrev.com/640fe86764317ab19f855b06da72ac80c39296d6/build/config/sanitizers/BUILD.gn

Project Member

Comment 47 by bugdroid1@chromium.org, Aug 3 2016

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

commit 1519f9727ddc675a9e3e1de9b904ef03c7d5b831
Author: thakis <thakis@chromium.org>
Date: Wed Aug 03 23:34:22 2016

Make base_unittests pass in gn/win/asan builds.

Before this, ScopedHandleTest.MultiProcess would fail because
scoped_handle_test_dll.dll got linked against clang_rt.asan-i386.lib
instead of clang_rt.asan_dll_thunk-i386.lib.  This now works.

BUG= 598761 

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

[modify] https://crrev.com/1519f9727ddc675a9e3e1de9b904ef03c7d5b831/build/config/BUILD.gn
[modify] https://crrev.com/1519f9727ddc675a9e3e1de9b904ef03c7d5b831/build/config/sanitizers/BUILD.gn

On the ToT bot we're still getting errors like so:

[7939/36247] ACTION //components/resources:compressed_about_credits(//build/toolchain/win:clang_x64)
FAILED: gen/components/resources/about_credits.bro 
C:/b/depot_tools/python276_bin/python.exe ../../build/gn_run_binary.py bro.exe --force --input gen/components/resources/about_credits.html --output gen/components/resources/about_credits.bro
==4176==AddressSanitizer CHECK failed: C:\b\c\b\CrWinAsan\src\third_party\llvm\projects\compiler-rt\lib\sanitizer_common\sanitizer_win.cc:250 "((VirtualQuery((void *)range_start, &mbi, sizeof(mbi)))) != (0)" (0x0, 0x0)
==4176==AddressSanitizer CHECK failed: C:\b\c\b\CrWinAsan\src\third_party\llvm\projects\compiler-rt\lib\asan\asan_win.cc:166 "((tsd_key_inited)) != (0)" (0x0, 0x0)


I had run that build step with pinned clang locally and it worked there, so maybe that's actually due to an asan runtime regression? (But the failing line wasn't touched in a while.)
I tried the chrome ToT without any patches.
(with the recent patch nico landed: https://codereview.chromium.org/2208093003/)

I confirm: the unittests used by the bot are compiling.

Comment 51 by r...@chromium.org, Aug 4 2016

We're getting errors like that because these bots are building for Win64, which WinASan doesn't support yet!

Gyp defaulted to 32-bit and gn defaults to x64, and we forgot to set 'target_cpu = "x86"'. Incoming tools/build CL.
D'oh, nice find!

Etienne asked me to write https://codereview.chromium.org/2214783002/ to catch something like that, but due to me not having a windows box today and him testing other things on his box today, we haven't landed it yet :-/
The issue with the mini_installer is still present:

D:\src\chromium\src>ninja -j1 -C out\asan
ninja: Entering directory `out\asan'
[1/4657] LINK mini_installer.exe mini_installer.exe.pdb

[...]

mini_installer_constants.obj : error LNK2001: unresolved external symbol _
mini_string.obj : error LNK2001: unresolved external symbol ___asan_init
pe_resource.obj : error LNK2001: unresolved external symbol ___asan_init
The build bot doesn't have the flag is_clang = true

  Generating files...
  ERROR at //build/config/sanitizers/sanitizers.gni:128:1: Assertion failed.
  assert(!using_sanitizer || is_clang,
  ^-----
  Sanitizers (is_*san) require setting is_clang = true in 'gn args'

I was think is_asan implies is_clang, but no.

  enable_ipc_fuzzer = true
  goma_dir = "C:\\b\\c\\cipd\\goma"
  is_asan = true
  is_component_build = false
  is_debug = false
  target_cpu = "x86"
  use_goma = true
  v8_enable_verify_heap = true

Yes all three Win ASAN bots are broken on LKGR, see https://build.chromium.org/p/chromium.lkgr/console. Weird is_asan is not implying is_clang on windows, but for other platform bots it is working fine./
Well, clang is used by default on other platforms :-)

In gyp, setting asan=1 used to enable clang=1; it should probably work the same way in gn (?) That makes it easier for devs to turn on asan at least.
Project Member

Comment 59 by bugdroid1@chromium.org, Aug 4 2016

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

commit 2c84540d35380ef8227bceaff92012cacb27aa6f
Author: thakis <thakis@chromium.org>
Date: Thu Aug 04 19:05:40 2016

win/asan 64-bit is in development; make sure we don't accidentally use it yet

BUG= 598761 

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

[modify] https://crrev.com/2c84540d35380ef8227bceaff92012cacb27aa6f/build/config/sanitizers/BUILD.gn

Project Member

Comment 60 by bugdroid1@chromium.org, Aug 4 2016

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

commit 9c263a45e736e6b44b04a4bf90edd5944a1df432
Author: rnk <rnk@chromium.org>
Date: Thu Aug 04 19:16:21 2016

Make the WinASan bots 32-bit again

This got lost in the gyp-to-gn flip, because gn is x64 by default.

R=thakis@chromium.org
NOTRY=true
BUG= 598761 

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

[modify] https://crrev.com/9c263a45e736e6b44b04a4bf90edd5944a1df432/tools/mb/mb_config.pyl

Project Member

Comment 61 by bugdroid1@chromium.org, Aug 4 2016

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

commit b2efa2c67908541d5a1a302283b1a675bf81902b
Author: Nico Weber <thakis@chromium.org>
Date: Thu Aug 04 19:29:40 2016

Explicitly set is_clang=true on the lkgr win asan bots

In the gyp build, asan=1 implicitly enabled clang.  This should probably happen
in the gn build as well, but currently it doesn't.  Until it does, explicitly
set it on these bots.

Thanks to etienneb@ for spotting this.

BUG= 598761 
R=etienneb@chromium.org, inferno@chromium.org

Review URL: https://codereview.chromium.org/2217653002 .

Cr-Commit-Position: refs/heads/master@{#409866}

[modify] https://crrev.com/b2efa2c67908541d5a1a302283b1a675bf81902b/tools/mb/mb_config.pyl

Project Member

Comment 62 by bugdroid1@chromium.org, Aug 5 2016

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

commit 3148c7f3469d5f4b2d653fd4bc8c990ac43f759b
Author: thakis <thakis@chromium.org>
Date: Fri Aug 05 00:08:32 2016

gn: Make mini_installer link in static asan builds.

The target removes the default executable_config, so it doesn't receive the
benefit of https://codereview.chromium.org/2208093003 in static-library
builds automatically.  Explicitly make it depend on sanitizers:link_executable.

BUG= 598761 

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

[modify] https://crrev.com/3148c7f3469d5f4b2d653fd4bc8c990ac43f759b/build/config/sanitizers/BUILD.gn
[modify] https://crrev.com/3148c7f3469d5f4b2d653fd4bc8c990ac43f759b/chrome/installer/mini_installer/BUILD.gn

Now:

[39475/42710] LINK(DLL) clang_nacl_win64/libGLESv2.dll clang_nacl_win64/libGLESv2.dll.lib clang_nacl_win64/libGLESv2.dll.pdb
FAILED: clang_nacl_win64/libGLESv2.dll clang_nacl_win64/libGLESv2.dll.lib clang_nacl_win64/libGLESv2.dll.pdb 
C:/b/depot_tools/python276_bin/python.exe gyp-win-tool link-wrapper environment.x64 False link.exe /nologo /IMPLIB:clang_nacl_win64/libGLESv2.dll.lib /DLL /OUT:clang_nacl_win64/libGLESv2.dll /PDB:clang_nacl_win64/libGLESv2.dll.pdb @clang_nacl_win64/libGLESv2.dll.rsp
preprocessor.lib(Macro.obj) : error LNK2019: unresolved external symbol __asan_option_detect_stack_use_after_return referenced in function "public: virtual __cdecl std::basic_streambuf<char,struct std::char_traits<char> >::~basic_streambuf<char,struct std::char_traits<char> >(void)" (??1?$basic_streambuf@DU?$char_traits@D@std@@@std@@UEAA@XZ)

preprocessor.lib(Input.obj) : error LNK2001: unresolved external symbol __asan_option_detect_stack_use_after_return

preprocessor.lib(Token.obj) : error LNK2001: unresolved external symbol __asan_option_detect_stack_use_after_return

(from https://build.chromium.org/p/chromium.fyi/builders/CrWinAsan/builds/3935)

I'm surprised the assert we added doesn't fire. This probably needs https://codereview.chromium.org/2198333002/diff/40001/build/toolchain/win/BUILD.gn to work around.
Ah, that's because we check current_cpu, not target_cpu.
https://codereview.chromium.org/2216183002/ seems like a pretty good fix for that.
Project Member

Comment 66 by bugdroid1@chromium.org, Aug 5 2016

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

commit 146d122ae8ed166f294231fbe6ca340ceffc356b
Author: Nico Weber <thakis@chromium.org>
Date: Fri Aug 05 01:45:03 2016

Only use sanitizers with default toolchain on Windows as well.

There's currently code to disable sanitizers for non-default toolchains
in gcc_toolchain.gni.  Move it to sanitizers.gni, then it's closer to
where all these args are declared and it works on Windows as well.

No intended behavior change on non-Windows.

BUG= 598761 
TBR=dpranke

Review URL: https://codereview.chromium.org/2216183002 .

Cr-Commit-Position: refs/heads/master@{#409963}

[modify] https://crrev.com/146d122ae8ed166f294231fbe6ca340ceffc356b/build/config/sanitizers/sanitizers.gni
[modify] https://crrev.com/146d122ae8ed166f294231fbe6ca340ceffc356b/build/toolchain/gcc_toolchain.gni

Status: Fixed (was: Assigned)
lkgr bots all cycle green:
https://build.chromium.org/p/chromium.lkgr/builders/Win%20ASan%20Release
https://build.chromium.org/p/chromium.lkgr/builders/Win%20ASan%20Release%20Coverage
https://build.chromium.org/p/chromium.lkgr/builders/Win%20ASan%20Release%20Media

The FYI bots haven't cycled yet. But CrWinASan is very far along: https://build.chromium.org/p/chromium.fyi/builders/CrWinAsan/builds/3937

So I think this is done.

Internet high-five to rnk and etienneb for helping with fixing this.

Comment 68 by aarya@google.com, Aug 5 2016

High-five to all of you for so much help here. Thanks a lot for getting Win ASAN gn friendly.
Nice.

Thx Nico for the rush this last two days.

I'm gonna check is_asan with clang-ToT.
I would like to see a WinAsan 64-bit build bot soon.
progress!
On my computer, the mini_installer issue is not solved.

Addressed here:  https://codereview.chromium.org/2215693003

D:\src\chromium\src>ninja -j1 -C out\asan
ninja: Entering directory `out\asan'
[1/1] Regenerating ninja files
[1/10198] LINK mini_installer.exe mini_installer.exe.pdb
FAILED: mini_installer.exe mini_installer.exe.pdb
D:/src/depot_tools/python276_bin/python.exe gyp-win-tool link-wrapper environment.x86 False link.exe /nologo /OUT:./mini
_installer.exe /PDB:./mini_installer.exe.pdb @./mini_installer.exe.rsp
clang_rt.asan-i386.lib(asan_interceptors.cc.obj) : error LNK2019: unresolved external symbol _strlen referenced in funct
ion "void __cdecl InitializeCommonInterceptors(void)" (?InitializeCommonInterceptors@@YAXXZ)
clang_rt.asan-i386.lib(asan_interceptors.cc.obj) : error LNK2019: unresolved external symbol _strnlen referenced in func
tion "void __cdecl InitializeCommonInterceptors(void)" (?InitializeCommonInterceptors@@YAXXZ)
clang_rt.asan-i386.lib(asan_interceptors.cc.obj) : error LNK2019: unresolved external symbol _strcmp referenced in funct
ion "void __cdecl InitializeCommonInterceptors(void)" (?InitializeCommonInterceptors@@YAXXZ)
clang_rt.asan-i386.lib(asan_interceptors.cc.obj) : error LNK2019: unresolved external symbol _strncmp referenced in func
tion "void __cdecl InitializeCommonInterceptors(void)" (?InitializeCommonInterceptors@@YAXXZ)
clang_rt.asan-i386.lib(asan_interceptors.cc.obj) : error LNK2019: unresolved external symbol _strstr referenced in funct
ion "void __cdecl InitializeCommonInterceptors(void)" (?InitializeCommonInterceptors@@YAXXZ)
clang_rt.asan-i386.lib(asan_interceptors.cc.obj) : error LNK2019: unresolved external symbol _strchr referenced in funct
ion "void __cdecl InitializeCommonInterceptors(void)" (?InitializeCommonInterceptors@@YAXXZ)
clang_rt.asan-i386.lib(asan_interceptors.cc.obj) : error LNK2019: unresolved external symbol _strrchr referenced in func
tion "void __cdecl InitializeCommonInterceptors(void)" (?InitializeCommonInterceptors@@YAXXZ)
clang_rt.asan-i386.lib(asan_interceptors.cc.obj) : error LNK2019: unresolved external symbol _strspn referenced in funct
ion "void __cdecl InitializeCommonInterceptors(void)" (?InitializeCommonInterceptors@@YAXXZ)
clang_rt.asan-i386.lib(asan_interceptors.cc.obj) : error LNK2019: unresolved external symbol _strcspn referenced in func
tion "void __cdecl InitializeCommonInterceptors(void)" (?InitializeCommonInterceptors@@YAXXZ)
clang_rt.asan-i386.lib(asan_interceptors.cc.obj) : error LNK2019: unresolved external symbol _strpbrk referenced in func
tion "void __cdecl InitializeCommonInterceptors(void)" (?InitializeCommonInterceptors@@YAXXZ)
clang_rt.asan-i386.lib(asan_interceptors.cc.obj) : error LNK2019: unresolved external symbol _memcmp referenced in funct
ion "void __cdecl InitializeCommonInterceptors(void)" (?InitializeCommonInterceptors@@YAXXZ)
clang_rt.asan-i386.lib(asan_interceptors.cc.obj) : error LNK2019: unresolved external symbol _memchr referenced in funct
ion "void __cdecl InitializeCommonInterceptors(void)" (?InitializeCommonInterceptors@@YAXXZ)
clang_rt.asan-i386.lib(asan_interceptors.cc.obj) : error LNK2019: unresolved external symbol _frexp referenced in functi
on "void __cdecl InitializeCommonInterceptors(void)" (?InitializeCommonInterceptors@@YAXXZ)
clang_rt.asan-i386.lib(asan_interceptors.cc.obj) : error LNK2019: unresolved external symbol _longjmp referenced in func
The bots are happy, so you're probably holding something wrong locally. What's your args.gn? How does it compare to the board's?
I'm gonna try a fresh checkout later this week.
If the bot are happy, this can wait a little bit.

I just wanted to keep track of this.

---
target_cpu = "x86"
is_debug = false
is_asan = true
is_clang = true
---

My pending changes...
D:\src\chromium\src>git diff master
diff --git a/build/config/sanitizers/BUILD.gn b/build/config/sanitizers/BUILD.gn
index cf51896..71b0c10 100644
--- a/build/config/sanitizers/BUILD.gn
+++ b/build/config/sanitizers/BUILD.gn
@@ -281,7 +281,7 @@ 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")
@@ -294,7 +294,7 @@ 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")

IIRC, bots do not compile every targets, but only unittests.
That's true for most chromium bots, but not for the clang tot bots -- they all build 'all'.
Ok, I'll check with a fresh chromium checkout later.
If the problem is still present, I'll open a new ticket.

I can't find something different on my setup, and I'm busy on something else.
I won't chase it now.
Or they're supposed to -- it might be broken for the asan bots :-( https://build.chromium.org/p/chromium.fyi/builders/CrWinAsan/builds/3948/steps/compile/logs/stdio
move mini_installer ticker here:
https://bugs.chromium.org/p/chromium/issues/detail?id=636398
moving the build bot issue to this ticket:
https://bugs.chromium.org/p/chromium/issues/detail?id=636399

Sign in to add a comment