Update: it appears to be reproducible. This is reliably crashing for me in my current state:

> ninja -C out\debug_component obj/base/base/precompile.cc.obj

The crucial setting to make this crash happen seems to be set DEPOT_TOOLS_WIN_TOOLCHAIN=0. Unfortunately this suggests that the repro is dependent on having the right local install of VS. I have the preview version (15.5) installed along with its associated header files. I packaged up my local preview install so that the bots can test this:

https://chromium-review.googlesource.com/747801

I'm still waiting on the try jobs, but using these settings locally with the gn args listed above I reliably get a crash with this command:

ninja -C out\debug_component obj/base/base/precompile.cc.obj

The try jobs don't reproduce the crash - wrong settings I guess - but it should be easy to patch in the CL (git cl patch or just copy the has into build\vs_toolchain.py) then "gclient runhooks" "gn clean" "ninja ..."

I tried to reproduce without success: 

- checked out 5699d8ccd4f8abed4b1b7258b1e096fa0ce26328
- patched in https://chromium-review.googlesource.com/747801
- gclient sync
- gn args as per #0
- ninja -C out\release obj/base/base/precompile.cc.obj

And it compiled fine :-/

Is there anything else I should do?

argh! I hadn't patched in 747801 correctly. Let me try again..

Yes, with the correct patch applied it reproduces nicely.

Preprocessing the output and running on that doesn't trigger the assert.

> Preprocessing the output and running on that doesn't trigger the assert.

Those are the best.

Any progress on this? This may eventually prevent us from moving to the VS 15.5 toolchain which we may want to do due to conformance improvements and linker improvements.

I have done local testing by installing the latest VS 2017 preview, which can side-by-side install with the latest VS 2017 stable. I can then use the vs2017_install environment variable to choose between them:

This selects preview - currently 15.5.0 Preview 5.0
set vs2017_install=C:\Program Files (x86)\Microsoft Visual Studio\Preview\Professional

This selects stable - currently 15.4.2
set vs2017_install=C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional

I tested with an empty args.gn file. In order to ensure that I am switching smoothly between configurations I do a "gn clean" between builds. I found that the failure only happens with the Preview version so it is clearly triggered by something in those header files.

I tried preprocessing the file but I could only do that by removing the PCH directives and at that point the error didn't happen - I didn't even get to the stage of generating or building a preprocessed file because I first had to remove the PCH directives and that resolved the issue. I guess that was obvious given that the failure happens inside PCHGenerator::HandleTranslationUnit.

So, the issue is somehow triggered by the VS 2017 header files interacting with generation of precompiled headers.

For some reason I can't get local variables in the leaf function that calls _wassert but the assembly language was fairly easy to decode. There are two entries in the FilenamesByID vector, but the problem is that ID is 00000000ffffffff. That's an interesting number. At first I thought it was -1, and it is probably *supposed* to be -1, but somehow it got zero extended instead of sign extended. The relevant code is here and it is definitely treating r14 as a 64-bit value:

00007ff6`f5978f21 488b86a8000000  mov     rax,qword ptr [rsi+0A8h]
00007ff6`f5978f28 488b8eb0000000  mov     rcx,qword ptr [rsi+0B0h]
00007ff6`f5978f2f 4829c1          sub     rcx,rax
00007ff6`f5978f32 48c1f903        sar     rcx,3
00007ff6`f5978f36 4c39f1          cmp     rcx,r14

So, my guess is that -1 is supposed to be a sentinel for "no file name" but a coding error or compiler error is causing this to be translated to an incorrect 64-bit value which leads to the assert. Source indexing isn't enabled so I can't easily examine the source code so I'm stopping now.

FWIW.

Sorry, somehow there was nobody on the cc list.
I'll look at this today.

The repro also works on Linux, with the help of https://chromium.googlesource.com/chromium/src/+/master/docs/win_cross.md
I like living in the future :-)

That's cool that it repros on Linux. That also shows that the problem (the apparent zero extension instead of sign extension of -1) is probably a coding error rather than a compiler bug.

I look forward to a fix so that I can do additional testing with the 15.5 preview headers.

Embarrassingly short repro:

$ echo '#line 32' | bin/clang -cc1 -emit-pch -o /dev/null -x c -
clang: ../tools/clang/include/clang/Basic/SourceManagerInternals.h:105: llvm::StringRef clang::LineTableInfo::getFilename(unsigned int) const: Assertion `ID < FilenamesByID.size() && "Invalid FilenameID"' failed.
#0 0x0000000001e78354 PrintStackTraceSignalHandler(void*) (bin/clang+0x1e78354)
#1 0x0000000001e786b6 SignalHandler(int) (bin/clang+0x1e786b6)
#2 0x00007f89ba4b6330 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x10330)
#3 0x00007f89b909ec37 gsignal /build/eglibc-SvCtMH/eglibc-2.19/signal/../nptl/sysdeps/unix/sysv/linux/raise.c:56:0
#4 0x00007f89b90a2028 abort /build/eglibc-SvCtMH/eglibc-2.19/stdlib/abort.c:91:0
#5 0x00007f89b9097bf6 __assert_fail_base /build/eglibc-SvCtMH/eglibc-2.19/assert/assert.c:92:0
#6 0x00007f89b9097ca2 (/lib/x86_64-linux-gnu/libc.so.6+0x2fca2)
#7 0x0000000002e6b366 clang::ASTWriter::WriteSourceManagerBlock(clang::SourceManager&, clang::Preprocessor const&) (bin/clang+0x2e6b366)
#8 0x0000000002e8ad58 clang::ASTWriter::WriteASTCore(clang::Sema&, llvm::StringRef, std::string const&, clang::Module*) (bin/clang+0x2e8ad58)
#9 0x0000000002e87b5e clang::ASTWriter::WriteAST(clang::Sema&, std::string const&, clang::Module*, llvm::StringRef, bool) (bin/clang+0x2e87b5e)
#10 0x0000000002eccc44 clang::PCHGenerator::HandleTranslationUnit(clang::ASTContext&) (bin/clang+0x2eccc44)
#11 0x000000000248460c clang::MultiplexConsumer::HandleTranslationUnit(clang::ASTContext&) (bin/clang+0x248460c)
#12 0x0000000002cbae36 clang::ParseAST(clang::Sema&, bool, bool) (bin/clang+0x2cbae36)
#13 0x000000000245aea0 clang::FrontendAction::Execute() (bin/clang+0x245aea0)
#14 0x00000000024076c1 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (bin/clang+0x24076c1)
#15 0x00000000024f1cbb clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (bin/clang+0x24f1cbb)
#16 0x0000000000865249 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (bin/clang+0x865249)
#17 0x0000000000862cb6 main (bin/clang+0x862cb6)
#18 0x00007f89b9089f45 __libc_start_main /build/eglibc-SvCtMH/eglibc-2.19/csu/libc-start.c:321:0
#19 0x000000000085fc42 _start (bin/clang+0x85fc42)
Stack dump:
0.	Program arguments: bin/clang -cc1 -emit-pch -o /dev/null -x c - 
1.	<eof> parser at end of file
Aborted (core dumped)


It seems clang was not ready for when the first #line directive doesn't have a filename. Should be easy to fix.

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

Retitling.

Fixed in r319707.
Needs a Clang roll.

Until we get a clang roll you can work around this bug by adding this to your gn args:

enable_precompiled_headers = false

This works because the bug only happens when using precompiled headers.

This bug appears to be the only problem with building Chrome using the VS 15.5 header files (confirmed by using the workaround in comment #20). This bug is being hit by more and more developers now that VS 15.5 has shipped, especially since it is no longer possible (for most people) to install older versions of VS. Raising the priority (although I assume that the clang team was already going to ship this as soon as possible).

Thanks for confirming, that's good news given that we have it fixed. We're working actively on getting the Clang version updated.

Clang roll is in, this should be fixed. It looks like the new SDK still has issues, though.

I can confirm that this issue is fixed.

The one remaining issue is with an abuse of extern constexpr in the Fall Creators Update SDK (16299) tracked in  bug 780056 , which is worked around by not using that SDK.

Can this be merged to M64? Thanks.

This bug requires manual review: M64 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

The commit is https://crrev.com/0d72bb14 for reference.

Merging over a new compiler to the beta branch seems risky.

@brucedawson Is there anything specific in this clang roll that you're concerned about? M64 is (I believe) the first release branch building with clang by default on Windows, so barring specific concerns to the contrary it might actually be a good idea to use the same version that master branch developers are also using.

Unless someone is trying to build M64 with the 15.5 headers, I don't see any reason to merge this. The PCH crash probably isn't affecting M64.

You can check the clang roll blocker list to see if there are any *other* reasons we might want to merge the clang roll, but none of them look urgent for M64.

@rnk: I (and probably others) are building M64 with 15.5 because MS doesn't give us an option during installation as brucedawson@ mentioned above. However, I accept that Google infrastructure might still be building release branches with an older VS version and the clang changes may be undesirable in that environment. Consequently, I withdraw my merge request :)

> @brucedawson Is there anything specific in this clang roll that you're concerned about?

Nothing specific, but changing compilers is always risky because of the potential for unintended consequences due to compiler bugs or differences that have unexpected consequences.