New issue
Advanced search Search tips

Issue 696671 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

Building dump_syms in Breakpad with VS2015 likely breaks Chrome official symbol upload

Project Member Reported by scottmg@chromium.org, Feb 27 2017

Issue description

Internal details here https://bugs.chromium.org/p/chromium/issues/detail?id=696257#c8

But the summary is that building dump_syms with VS2015 appears to have broken symbols because msdia140 isn't (yet?) registered on official builders.
 

Comment 1 by mark@chromium.org, Feb 27 2017

Cc: mark@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 27 2017

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

commit 5f3ea1407ad98cdb9d8418a52d3dbeff7ec5033e
Author: scottmg <scottmg@chromium.org>
Date: Mon Feb 27 20:46:55 2017

Roll breakpad 37070c5..eaedc45

eaedc45 Improve stack sanitization unittests.
0ca6751 Handle ntdll only emitting PUBLIC at func entry
c7e826f Make stack sanitization elide pointers to non-executable mappings.
918f3dc Use the correct PC when determining whether to skip storing a stack.
f9d3ab8 minidump: mark Read as override in derived classes
3ff7ca4 Fix compile errors arising from compiling breakpad with clang.
3996c1b libdisasm: add upstream/license details
603f8b6 There is no need to use the main queue just for perform selector.
e6123b1 Appveyor CI for Windows MSVS build
376784d processor: drop set-but-unused variable
122d65d macho_reader_unittest: use EXPECT_FALSE
093fab2 windows: fix build on pre-Win10 systems
620e0fb windows: update gtest/gmock paths
fc92bb3 fix write() unused-result warning
d5f233d Fixed leak of unloaded module lists.

Previous reverted attempt was
https://codereview.chromium.org/2719553002. This rolls in the same code.
However, this time symupload was instead built with VS2013. I confirmed
the link line included:

/LIBPATH:"C:\Program Files (x86)\Microsoft Visual Studio 12.0\/DIA SDK/lib"

and:

c:\src\cr\src\breakpad>dumpbin /headers symupload.exe | grep linker
           12.00 linker version

This is expected to help because the suspicion is that only msdia120.dll
is registered on official bots, not msdia140.dll.

R=mark@chromium.org
BUG= 678874 ,696257, 696671 

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

[modify] https://crrev.com/5f3ea1407ad98cdb9d8418a52d3dbeff7ec5033e/DEPS
[modify] https://crrev.com/5f3ea1407ad98cdb9d8418a52d3dbeff7ec5033e/breakpad/symupload.exe

I had been looking at the wrong logs on the official builders, but when I hunted down the right ones, I see now there's actually a CoCreateInstance message logged, so I'm confident this was an msdia registration problem now.
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 28 2017

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

commit 8af45a18bab6cf4f1dc16a92a229dfa2b7e81dd1
Author: scottmg <scottmg@chromium.org>
Date: Tue Feb 28 18:33:17 2017

Revert of Roll breakpad 37070c5..eaedc45 (patchset #1 id:1 of https://codereview.chromium.org/2718173002/ )

Reason for revert:
Failing for chrome_child.dll on x86 builds only. crbug.com/696911.

Original issue's description:
> Roll breakpad 37070c5..eaedc45
>
> eaedc45 Improve stack sanitization unittests.
> 0ca6751 Handle ntdll only emitting PUBLIC at func entry
> c7e826f Make stack sanitization elide pointers to non-executable mappings.
> 918f3dc Use the correct PC when determining whether to skip storing a stack.
> f9d3ab8 minidump: mark Read as override in derived classes
> 3ff7ca4 Fix compile errors arising from compiling breakpad with clang.
> 3996c1b libdisasm: add upstream/license details
> 603f8b6 There is no need to use the main queue just for perform selector.
> e6123b1 Appveyor CI for Windows MSVS build
> 376784d processor: drop set-but-unused variable
> 122d65d macho_reader_unittest: use EXPECT_FALSE
> 093fab2 windows: fix build on pre-Win10 systems
> 620e0fb windows: update gtest/gmock paths
> fc92bb3 fix write() unused-result warning
> d5f233d Fixed leak of unloaded module lists.
>
> Previous reverted attempt was
> https://codereview.chromium.org/2719553002. This rolls in the same code.
> However, this time symupload was instead built with VS2013. I confirmed
> the link line included:
>
> /LIBPATH:"C:\Program Files (x86)\Microsoft Visual Studio 12.0\/DIA SDK/lib"
>
> and:
>
> c:\src\cr\src\breakpad>dumpbin /headers symupload.exe | grep linker
>            12.00 linker version
>
> This is expected to help because the suspicion is that only msdia120.dll
> is registered on official bots, not msdia140.dll.
>
> R=mark@chromium.org
> BUG= 678874 ,696257, 696671 
>
> Review-Url: https://codereview.chromium.org/2718173002
> Cr-Commit-Position: refs/heads/master@{#453325}
> Committed: https://chromium.googlesource.com/chromium/src/+/5f3ea1407ad98cdb9d8418a52d3dbeff7ec5033e

TBR=mark@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 678874 ,696257, 696671 

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

[modify] https://crrev.com/8af45a18bab6cf4f1dc16a92a229dfa2b7e81dd1/DEPS
[modify] https://crrev.com/8af45a18bab6cf4f1dc16a92a229dfa2b7e81dd1/breakpad/symupload.exe

Appppppparently we need https://codereview.chromium.org/2173533002 to manually flip the LargeAddressAware flag on the binary. Argh, what a mess!
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 28 2017

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

commit 7f67e4310802a5f05d702276d5111c9b99391921
Author: scottmg <scottmg@chromium.org>
Date: Tue Feb 28 21:14:08 2017

Roll breakpad 37070c5..10162f6

10162f6 win: Set LargeAddressAware on symupload
eaedc45 Improve stack sanitization unittests.
0ca6751 Handle ntdll only emitting PUBLIC at func entry
c7e826f Make stack sanitization elide pointers to non-executable mappings.
918f3dc Use the correct PC when determining whether to skip storing a stack.
f9d3ab8 minidump: mark Read as override in derived classes
3ff7ca4 Fix compile errors arising from compiling breakpad with clang.
3996c1b libdisasm: add upstream/license details
603f8b6 There is no need to use the main queue just for perform selector.
e6123b1 Appveyor CI for Windows MSVS build
376784d processor: drop set-but-unused variable
122d65d macho_reader_unittest: use EXPECT_FALSE
093fab2 windows: fix build on pre-Win10 systems
620e0fb windows: update gtest/gmock paths
fc92bb3 fix write() unused-result warning
d5f233d Fixed leak of unloaded module lists.

Previous attempts https://codereview.chromium.org/2719553002 and
https://codereview.chromium.org/2718173002. This time, we have both a
MSVS 2013 build, and the LargeAddressAware flag set on the .exe.

[roll-breakpad-3]c:\src\cr\src>dumpbin /headers breakpad\symupload.exe | grep linker
           12.00 linker version

[roll-breakpad-3]c:\src\cr\src>dumpbin /headers breakpad\symupload.exe | grep large
                   Application can handle large (>2GB) addresses

R=mark@chromium.org
BUG= 678874 ,696257, 696671 ,696911

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

[modify] https://crrev.com/7f67e4310802a5f05d702276d5111c9b99391921/DEPS
[modify] https://crrev.com/7f67e4310802a5f05d702276d5111c9b99391921/breakpad/symupload.exe

Project Member

Comment 7 by bugdroid1@chromium.org, Mar 1 2017

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

commit 73dd82d9969add23c5070c12cdbeba8e9e3f13ff
Author: scottmg <scottmg@chromium.org>
Date: Wed Mar 01 23:19:06 2017

Try x64 binary for symupload

symupload is still failing for chrome_child.dll on the 32 bit official
bots. One last try of making an x64 binary instead, and then I'm going
to give up.

TBR=mark@chromium.org
BUG= 678874 , 696257,  696671 , 696911,697638

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

[modify] https://crrev.com/73dd82d9969add23c5070c12cdbeba8e9e3f13ff/breakpad/symupload.exe

Project Member

Comment 8 by bugdroid1@chromium.org, Mar 2 2017

Labels: merge-merged-3026
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bf0b0926a4ad3ad196540c7a9c31c465591fc645

commit bf0b0926a4ad3ad196540c7a9c31c465591fc645
Author: Scott Graham <scottmg@chromium.org>
Date: Thu Mar 02 02:24:12 2017

Try x64 binary for symupload

symupload is still failing for chrome_child.dll on the 32 bit official
bots. One last try of making an x64 binary instead, and then I'm going
to give up.

TBR=mark@chromium.org
BUG= 678874 , 696257,  696671 , 696911,697638

Review-Url: https://codereview.chromium.org/2723233003
Cr-Commit-Position: refs/heads/master@{#454089}
(cherry picked from commit 73dd82d9969add23c5070c12cdbeba8e9e3f13ff)

Review-Url: https://codereview.chromium.org/2728903002 .
Cr-Commit-Position: refs/branch-heads/3026@{#7}
Cr-Branched-From: fe586ab75aca1b8ab839db23bceac5f621389fed-refs/heads/master@{#453454}

[modify] https://crrev.com/bf0b0926a4ad3ad196540c7a9c31c465591fc645/breakpad/symupload.exe

Status: Fixed (was: Started)
This seems to be fixed by a 2013 x64 binary (fingers crossed)

Sign in to add a comment