the clang plugin just whitelist /v8/ so in chromium, we're good.

Maybe our non-gn config adds some define the disables all clang plugins for standalone builds, but it's not set for win, as we didn't expect clang to run there or something

Do you have a link to a gyp bot somewhere?

So I think what happened is that standalone.gypi never had the `-Xclang -load -Xclang <(clang_lib_path)` stuff that Chromium's build/common.gypi had. But in the gn build, v8 uses Chromium's build/config/clang/BUILD.gn which does pass this.

I guess you could set clang_use_chrome_plugins=false for your bots and things should be fine?

clang_use_chrome_plugins should be true by default. So our linux bots should all have it already (nearly all on GN). Maybe the win-clang bot uses some other/more plugins?

Here an example of what changes when switching our current linux config to clang:
    In GN, but not in gyp:
      -Xclang -add-plugin
      -Xclang -load
      -Xclang -plugin-arg-find-bad-constructs
      -Xclang /mnt/data/b/build/slave/v8-linux-noi18n-debug/build/v8/third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so
      -Xclang check-ipc
      -Xclang find-bad-constructs

I'll try to set it so false for the win-clang bot.

The -load isn't on your Windows bot, clang-cl can't load .so files on Windows. You're saying that's what your Linux bot has and it's happy? If I agree so we should figure out why Win isn't happy too. Link to Linux gn bot?

Most our linux bots switched to gn like:
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20builder

Or just build V8:
fetch v8
cd v8
tools/dev/v8gen.py x64.release
ninja -C out.gn/x64.release -v

Example output:
clang++ [...] -Xclang -load -Xclang ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so -Xclang -add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs -Xclang check-ipc [...] -c ../../src/bignum.cc -o obj/v8_base/bignum.o

Doesn't complain.

What about jochen's suggestion in comment 1? Is there a define that might disable the clang plugin in code?

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

commit 70429e7a201ccec21fdc71dc62508a0f20b9261b
Author: machenbach <machenbach@chromium.org>
Date: Tue Sep 06 11:16:57 2016

Reland of [gn] Switch win clang bot to gn (patchset #1 id:1 of https://codereview.chromium.org/2311853002/ )

Reason for revert:
Trying reland with plugin switched off.

Original issue's description:
> Revert of [gn] Switch win clang bot to gn (patchset #1 id:1 of https://codereview.chromium.org/2310863002/ )
>
> Reason for revert:
> Fails compilation:
> https://build.chromium.org/p/client.v8/builders/V8%20Win64%20-%20clang
>
> Original issue's description:
> > [gn] Switch win clang bot to gn
> >
> > BUG= chromium:474921 
> > NOTRY=true
> >
> > Committed: https://crrev.com/1740587772bbd1ffb7b79a90deb16519e8d47588
> > Cr-Commit-Position: refs/heads/master@{#39156}
>
> TBR=jochen@chromium.org,vogelheim@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= chromium:474921 
>
> Committed: https://crrev.com/49e13ea6eab4158915a75b95ccc9f01892924d9e
> Cr-Commit-Position: refs/heads/master@{#39158}

TBR=jochen@chromium.org,vogelheim@chromium.org
NOTRY=true
BUG= chromium:474921 , chromium:644096 

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

[modify] https://crrev.com/70429e7a201ccec21fdc71dc62508a0f20b9261b/infra/mb/mb_config.pyl

Here's my theory:

The plugin blacklists files in v8 here: https://cs.chromium.org/chromium/src/tools/clang/plugins/ChromeClassTester.cpp?rcl=0&l=233

On posix, it makes the path to each file absolute here: https://cs.chromium.org/chromium/src/tools/clang/plugins/ChromeClassTester.cpp?rcl=0&l=127

On windows, it doesn't form absolute file paths. The bots use "v8" somewhere in the build path, but it's not part of the short name (which is just "src/base/bits.cc" or some such). So on Windows, the suppression doesn't work.


I think we should make this consistent across platforms. The easiest change for now is probably to just do something realpath()-y on Windows too.

roughly like so: https://codereview.chromium.org/2318733002/

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

commit df0eb747fca2699d116ad80d0ccbd967317fdbd4
Author: thakis <thakis@chromium.org>
Date: Fri Sep 09 20:55:54 2016

clang plugin: Compute absolute paths on Windows as well.

The motivation is to suppress diagnostics on the v8 bot,
which have "v8" somewhere on the path, but not in the
filename segments the compiler sees.  This makes the
behavior on Windows match the behavior on non-Windows.

BUG= 644096 
NOTRY=true

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

[modify] https://crrev.com/df0eb747fca2699d116ad80d0ccbd967317fdbd4/tools/clang/plugins/ChromeClassTester.cpp

After the next clang roll, you should be able to remove no_plugins again

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

commit 4b850c5bf7d78377cc2e0b00097f9d9947bbdfb5
Author: hans <hans@chromium.org>
Date: Fri Sep 09 23:28:14 2016

Revert of clang plugin: Compute absolute paths on Windows as well. (patchset #6 id:100001 of https://codereview.chromium.org/2318733002/ )

Reason for revert:
The bots are sad. From https://build.chromium.org/p/chromium.fyi/builders/ClangToTLinux%20%28dbg%29/builds/4060/steps/gclient%20runhooks/logs/stdio

/b/c/b/ClangToTLinux__dbg_/src/tools/clang/plugins/ChromeClassTester.cpp: In member function ‘bool ChromeClassTester::InBannedDirectory(clang::SourceLocation)’:
/b/c/b/ClangToTLinux__dbg_/src/tools/clang/plugins/ChromeClassTester.cpp:129:41: error: ‘resolvedPath’ was not declared in this scope
   } else if (realpath(filename.c_str(), resolvedPath)) {
                                         ^
/b/c/b/ClangToTLinux__dbg_/src/tools/clang/plugins/ChromeClassTester.cpp:130:33: error: redeclaration of ‘char resolvedPath [4096]’
     char resolvedPath[MAXPATHLEN];
                                 ^
/b/c/b/ClangToTLinux__dbg_/src/tools/clang/plugins/ChromeClassTester.cpp:129:41: error: ‘<typeprefixerror>resolvedPath’ previously declared here
   } else if (realpath(filename.c_str(), resolvedPath)) {
                                         ^

Original issue's description:
> clang plugin: Compute absolute paths on Windows as well.
>
> The motivation is to suppress diagnostics on the v8 bot,
> which have "v8" somewhere on the path, but not in the
> filename segments the compiler sees.  This makes the
> behavior on Windows match the behavior on non-Windows.
>
> BUG= 644096 
> NOTRY=true
>
> Committed: https://crrev.com/df0eb747fca2699d116ad80d0ccbd967317fdbd4
> Cr-Commit-Position: refs/heads/master@{#417701}

TBR=dcheng@chromium.org,thakis@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 644096 

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

[modify] https://crrev.com/4b850c5bf7d78377cc2e0b00097f9d9947bbdfb5/tools/clang/plugins/ChromeClassTester.cpp

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

commit 681568d4e7a08a988d677d8ad76d79faf87e7d2e
Author: thakis <thakis@chromium.org>
Date: Sat Sep 10 02:27:56 2016

clang plugin: Compute absolute paths on Windows as well.

The motivation is to suppress diagnostics on the v8 bot,
which have "v8" somewhere on the path, but not in the
filename segments the compiler sees.  This makes the
behavior on Windows match the behavior on non-Windows.

BUG= 644096 
NOTRY=true

Committed: https://crrev.com/df0eb747fca2699d116ad80d0ccbd967317fdbd4
Review-Url: https://codereview.chromium.org/2318733002
Cr-Original-Commit-Position: refs/heads/master@{#417701}
Cr-Commit-Position: refs/heads/master@{#417804}

[modify] https://crrev.com/681568d4e7a08a988d677d8ad76d79faf87e7d2e/tools/clang/plugins/ChromeClassTester.cpp

Thanks for the fix! When you say "clang roll", is it enough to point to an up-to-date https://chromium.googlesource.com/chromium/src/tools/clang/+/master ? Or do we need to wait for a new clang version?

You have to wait for a new prebuilt binary. I just filed a bug for the next roll and made this bug here blocked on the new one.

Thanks!

Plugin change is deployed as of after https://codereview.chromium.org/2361513002 – if that's still in the tree in a day, try removing your workaround.

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

commit 01cd881b870ceb11b837506f6fd7abd635d1fbcd
Author: machenbach <machenbach@chromium.org>
Date: Fri Sep 23 13:26:09 2016

[gn] Use clang plugins on win clang bot

BUG= chromium:644096 
TBR=thakis@chromium.org
NOTRY=true

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

[modify] https://crrev.com/01cd881b870ceb11b837506f6fd7abd635d1fbcd/infra/mb/mb_config.pyl

Stayed green after reactivating. Thanks for fixing!