I'm working on making ASan 2015-ready. I'm down to 12 test failures, so this doesn't look too big.

I fixed some issues, but we still haven't solved the DIA issues.

What is the DIA issue? Is there a separate bug for it? 578306 is marked as fixed - it would be good to have clarity on what issues are remaining here.

When I locally run package.py, LLVM's build system is able to find DIA and link llvm-symbolizer against it. When we run package.py on the trybot designed to automatically make clang packages, we see test failures suggesting that it couldn't find DIA.

This is the trybot in question: https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_clang_upload/

These are the failing tests:
********************
Failing Tests (10):
    LLVM :: DebugInfo/PDB/DIA/pdbdump-flags.test
    LLVM :: DebugInfo/PDB/DIA/pdbdump-linenumbers.test
    LLVM :: DebugInfo/PDB/DIA/pdbdump-symbol-format.test
    LLVM :: ExecutionEngine/OrcMCJIT/load-object-a.ll
    LLVM :: tools/llvm-pdbdump/class-layout.test
    LLVM :: tools/llvm-pdbdump/enum-layout.test
    LLVM :: tools/llvm-pdbdump/load-address.test
    LLVM :: tools/llvm-pdbdump/regex-filter.test
    LLVM :: tools/llvm-symbolizer/pdb/pdb.test
    UBSan-Standalone-x86_64 :: TestCases/Integer/suppressions.cpp

Yes, the last build on that bot (https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_clang_upload/builds/17) "succeeded", but only because I had already uploaded the package manually.

Yep (I realized that soon after posting the comment #7)

Good news, this fails for me locally too. I guess I'll try and debug a bit.

When this passes for you, do you run package.py in a vanilla cmd shell, or do you use a msvc tools shell with cl in the path?

My local llvm-bootstrap\test\lit.site.cfg has:

config.have_dia_sdk = 1

My local llvm-bootstrap\includ\llvm\Config\confg.h has:

#define HAVE_DIA_SDK 1


So cmake did detect the dia sdk alright. Still test output is:

Command 1 Stderr:
C:\src\chrome\src\third_party\llvm\test\tools\llvm-pdbdump\load-address.test:6:8: error: expected string not found in in
put
; RVA: ---EXTERNALS---
       ^
<stdin>:1:1: note: scanning from here
Reading PDBs is not supported on this platform.
^


Repro:

C:\src\chrome\src\third_party\llvm-bootstrap>bin\llvm-pdbdump.exe -externals ..\llvm\test\tools\llvm-pdbdump\Inputs\LoadAddressTest.pdb
Reading PDBs is not supported on this platform.

The dia sdk is detected alright and is called, but this fails in lib\DebugInfo\PDB\DIA\DIASession.cpp:

DIASession::DIASession(CComPtr<IDiaSession> DiaSession) : Session(DiaSession) {}

PDB_ErrorCode DIASession::createFromPdb(StringRef Path,
                                        std::unique_ptr<IPDBSession> &Session) {
  CComPtr<IDiaDataSource> DiaDataSource;
  CComPtr<IDiaSession> DiaSession;

  // We assume that CoInitializeEx has already been called by the executable.
  HRESULT Result = ::CoCreateInstance(
      CLSID_DiaSource, nullptr, CLSCTX_INPROC_SERVER, IID_IDiaDataSource,
      reinterpret_cast<LPVOID *>(&DiaDataSource));
  if (FAILED(Result))
    return PDB_ErrorCode::NoPdbImpl;
    // ^^ we hit this error return

I think the root cause here is that bot and me don't have VS2015 installed, and rnk probably has. Hence, the newest msdia*.dll is registered for him bot not for the bot or me.

http://stackoverflow.com/questions/2466138/windows-c-how-to-use-a-com-dll-which-is-not-registered is probably what applies to us.

Patch attempt (cribbed from breakpad):

C:\src\chrome\src\third_party\llvm>svn diff
Index: lib/DebugInfo/PDB/DIA/DIASession.cpp
===================================================================
--- lib/DebugInfo/PDB/DIA/DIASession.cpp        (revision 264668)
+++ lib/DebugInfo/PDB/DIA/DIASession.cpp        (working copy)
@@ -18,6 +18,8 @@
 #include "llvm/DebugInfo/PDB/PDBSymbolExe.h"
 #include "llvm/Support/ConvertUTF.h"

+#include <diacreate.h>
+
 using namespace llvm;

 namespace {}
@@ -33,9 +35,26 @@
   HRESULT Result = ::CoCreateInstance(
       CLSID_DiaSource, nullptr, CLSCTX_INPROC_SERVER, IID_IDiaDataSource,
       reinterpret_cast<LPVOID *>(&DiaDataSource));
-  if (FAILED(Result))
-    return PDB_ErrorCode::NoPdbImpl;
+  if (FAILED(Result)) {
+    class DECLSPEC_UUID("3BFCEA48-620F-4B6B-81F7-B9AF75454C7D") DiaSource120;
+    class DECLSPEC_UUID("E6756135-1E65-4D17-8576-610761398C3C") DiaSource140;

+    // If the CoCreateInstance call above failed, msdia*.dll is not registered.
+    // We can try loading the DLL corresponding to the #included DIA SDK, but
+    // the DIA headers don't provide a version. Lets try to figure out which DIA
+    // version we're compiling against by comparing CLSIDs.
+    const wchar_t *msdia_dll = nullptr;
+    if (CLSID_DiaSource == _uuidof(DiaSource120)) // VS2013
+      msdia_dll = L"msdia120.dll";
+    else if (CLSID_DiaSource == _uuidof(DiaSource140)) // VS2015
+      msdia_dll = L"msdia140.dll";
+
+    if (!msdia_dll ||
+        FAILED(NoRegCoCreate(msdia_dll, CLSID_DiaSource, IID_IDiaDataSource,
+                                reinterpret_cast<void **>(&DiaDataSource))))
+      return PDB_ErrorCode::NoPdbImpl;
+  }
+
   llvm::SmallVector<UTF16, 128> Path16;
   if (!llvm::convertUTF8ToUTF16String(Path, Path16))
     return PDB_ErrorCode::InvalidPath;

Rerunning with that applied now.

With that patch:
Failing Tests (2):
    LLVM :: tools/llvm-symbolizer/pdb/pdb.test
    UBSan-Standalone-x86_64 :: TestCases/Integer/suppressions.cpp

So better, but not fixed. Looking more.

C:\src\chrome\src\third_party\llvm-bootstrap>echo 0x401000 | bin\llvm-symbolizer.exe -obj=c:\src\chrome\src\third_party\llvm\test\tools\llvm-symbolizer\pdb\Inputs\test.exe
??
??:0:0

Next try:

M       third_party\llvm\lib\DebugInfo\PDB\DIA\DIASession.cpp
Index: third_party/llvm/lib/DebugInfo/PDB/DIA/DIASession.cpp
===================================================================
--- third_party/llvm/lib/DebugInfo/PDB/DIA/DIASession.cpp       (revision 264668)
+++ third_party/llvm/lib/DebugInfo/PDB/DIA/DIASession.cpp       (working copy)
@@ -18,10 +18,37 @@
 #include "llvm/DebugInfo/PDB/PDBSymbolExe.h"
 #include "llvm/Support/ConvertUTF.h"

+#include <diacreate.h>
+
 using namespace llvm;

-namespace {}
+namespace {

+bool LoadDIA(CComPtr<IDiaDataSource>& DiaDataSource) {
+  if (SUCCEEDED(CoCreateInstance(
+          CLSID_DiaSource, nullptr, CLSCTX_INPROC_SERVER, IID_IDiaDataSource,
+          reinterpret_cast<LPVOID *>(&DiaDataSource))))
+    return true;
+
+  // If the CoCreateInstance call above failed, msdia*.dll is not registered.
+  // We can try loading the DLL corresponding to the #included DIA SDK, but
+  // the DIA headers don't provide a version. Lets try to figure out which DIA
+  // version we're compiling against by comparing CLSIDs.
+  class DECLSPEC_UUID("3BFCEA48-620F-4B6B-81F7-B9AF75454C7D") DiaSource120;
+  class DECLSPEC_UUID("E6756135-1E65-4D17-8576-610761398C3C") DiaSource140;
+  const wchar_t *msdia_dll = nullptr;
+  if (CLSID_DiaSource == _uuidof(DiaSource120)) // VS2013
+    msdia_dll = L"msdia120.dll";
+  else if (CLSID_DiaSource == _uuidof(DiaSource140)) // VS2015
+    msdia_dll = L"msdia140.dll";
+
+  return msdia_dll &&
+      SUCCEEDED(NoRegCoCreate(msdia_dll, CLSID_DiaSource, IID_IDiaDataSource,
+                              reinterpret_cast<LPVOID *>(&DiaDataSource)));
+}
+
+}
+
 DIASession::DIASession(CComPtr<IDiaSession> DiaSession) : Session(DiaSession) {}

 PDB_ErrorCode DIASession::createFromPdb(StringRef Path,
@@ -30,10 +57,7 @@
   CComPtr<IDiaSession> DiaSession;

   // We assume that CoInitializeEx has already been called by the executable.
-  HRESULT Result = ::CoCreateInstance(
-      CLSID_DiaSource, nullptr, CLSCTX_INPROC_SERVER, IID_IDiaDataSource,
-      reinterpret_cast<LPVOID *>(&DiaDataSource));
-  if (FAILED(Result))
+  if (!LoadDIA(DiaDataSource))
     return PDB_ErrorCode::NoPdbImpl;

   llvm::SmallVector<UTF16, 128> Path16;
@@ -41,6 +65,7 @@
     return PDB_ErrorCode::InvalidPath;

   const wchar_t *Path16Str = reinterpret_cast<const wchar_t*>(Path16.data());
+  HRESULT Result;
   if (FAILED(Result = DiaDataSource->loadDataFromPdb(Path16Str))) {
     if (Result == E_PDB_NOT_FOUND)
       return PDB_ErrorCode::InvalidPath;
@@ -71,16 +96,14 @@
   CComPtr<IDiaSession> DiaSession;

   // We assume that CoInitializeEx has already been called by the executable.
-  HRESULT Result = ::CoCreateInstance(
-      CLSID_DiaSource, nullptr, CLSCTX_INPROC_SERVER, IID_IDiaDataSource,
-      reinterpret_cast<LPVOID *>(&DiaDataSource));
-  if (FAILED(Result))
+  if (!LoadDIA(DiaDataSource))
     return PDB_ErrorCode::NoPdbImpl;

   llvm::SmallVector<UTF16, 128> Path16;
   if (!llvm::convertUTF8ToUTF16String(Path, Path16))
     return PDB_ErrorCode::InvalidPath;

+  HRESULT Result;
   const wchar_t *Path16Str = reinterpret_cast<const wchar_t *>(Path16.data());
   if (FAILED(Result =
                  DiaDataSource->loadDataForExe(Path16Str, nullptr, nullptr))) {

That did the trick. patch: http://reviews.llvm.org/D18707

Should be fixed at r265193, so the next roll will hopefully work with the upload bots on all platforms.

Hm, I gave this a try (at r265238), and the tests still fail on the bot: https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_clang_upload/builds/18/steps/package%20clang/logs/stdio :-(

(Filed https://llvm.org/bugs/show_bug.cgi?id=27187 for the unrelated BinaryReaderTest.hello_obj_ppc failure)

(cl: https://codereview.chromium.org/1854933003)

Maybe msdia140.dll is not on PATH on the bot?

The bot should run `setenv.cmd /x64` which should put it on PATH from what I understand.

I'll ssh in later today and try to debug on the bot a bit.

I tried ssh'ing in, but the bot's pretty busted due to  bug 600465  :-/

Here is a sneak peak for what we'll have once https://codereview.chromium.org/1860823006/ is submitted:

https://build.chromium.org/p/tryserver.chromium.win/builders/win_upload_clang/builds/0
Reading PDBs is not supported on this platform.

Which is completely expected.

%PATH% on the bot:

E:\b\build\slave\win_upload_clang\build\src\third_party\llvm-bootstrap>echo %PATH%
echo %PATH%
e:\b\depot_tools\win_toolchain\vs_files\a3796183a9fc4d22a687c5212b9c76dbd136d70d\win_sdk\bin\..\..\win_sdk\bin\x64;e:\b\depot_tools\win_toolchain\vs_files\a3796183a9fc4d22a687c5212b9c76dbd136d70d\win_sdk\bin\..\..\VC\bin\amd64;C:\cygwin\usr\local\bin;C:\cygwin\bin;C:\cygwin\bin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Program Files\Microsoft SQL Server\110\Tools\Binn;C:\Program Files (x86)\Windows Kits\8.1\Windows Performance Toolkit;C:\Program Files (x86)\QuickTime\QTSystem;C:\Program Files (x86)\Microsoft SDKs\TypeScript\1.0;C:\Program Files\Puppet Labs\Puppet\bin;C:\ProgramData\chocolatey\bin;e:\b\depot_tools


LoadDIA() is failing.


msdia140.dll is in DIA SDK/bin/amd64/msdia140.dll, which like rnk guessed above isn't on PATH. If I manually add it, the test starts passing. I guess the fix is to add the right DIA SDK to PATH in the chromium toolchain bundler. I'll do that.

On second thought, vcvarsall doesn't add msdia to the path, so doing that in setenv.cmd is weird. Probably better to add it to PATH in update.py.

On third thought, this also means that breakpad and friends don't have access to msdia*.dll, right?

Does breakpad use msdia140.dll directly? I assumed it went via dbghelp.dll, which the chromium build copies into the build directory so that it is always found.

Maybe update.py should copy msdia140.dll next to llvm-symbolizer.exe so that llvm-symbolizer always works, rather than only working during the test suite execution. If we ever switch clusterfuzz to llvm-symbolizer, we'll want to package it that way.

We can't redistribute msdia.dll. I figured I'd just tweak PATH (https://codereview.chromium.org/1879753002) but rnk suggested we can copy msdia.dll from the toolchain install on the local disk next to llvm-symbolizer when update.py runs, which sounds like a great idea to me. I'll change my CL to do that instead.

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

commit bf3b64ab66179b2435fc7b10cfd091247b8cab87
Author: thakis <thakis@chromium.org>
Date: Tue Apr 12 18:25:18 2016

clang update.py: When running tests on Windows, make sure DIA works.

LLVM's DIA reading code either needs DIA registered in the registry,
or it needs msdia140.dll somewhere.  Copy msdia140.dll from the hermetic
toolchain into the bin/ directory so it's next to llvm-symbolizer.exe
and friends.  (This means the tests that package.py runs will only all
pass for people and bots that have access to the hermetic toolchain.)

Also do this after downloading clang.  (Here it's not guaranteed that
people have access to the hermetic toolchain -- the script will fall
back to a MSVC install and copy from there.  Since this
requires msdia140.dll, this means we now require either the hermetic
toolchain or an installed MSVS2015. If this caused problems, we can
reconsider.)

BUG= 596201 

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

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

[modify] https://crrev.com/bf3b64ab66179b2435fc7b10cfd091247b8cab87/tools/clang/scripts/update.py

Hooray, this worked:

https://build.chromium.org/p/tryserver.chromium.win/builders/win_upload_clang/builds/8/steps/package%20clang/logs/stdio

Only failure is MachOTests/lldMachOTests.exe/BinaryReaderTest.hello_obj_ppc and that's https://llvm.org/bugs/show_bug.cgi?id=27187 and fixed at trunk (I tried it with an oldish clang revision).

The bot's still unusable until  bug 600465  is fixed, though.