1029469 - Buildsymbols during incremental builds packages PDBs from previous builds

Status: RESOLVED DUPLICATE of bug 1337986

(Firefox Build System :: General, defect)

Description

[Carsten Book [:Tomcat]]

Inbound Tree is closed since we have test failures like 

https://tbpl.mozilla.org/php/getParsedLog.php?id=42345604&tree=Mozilla-Inbound

Exception: Ambiguous symbol file for xul.pdb
WindowsError: [Error 6] The handle is invalid.

its not on every windows build but very frequent in Crash and Reftests and also partly on mochitests.

03:51 <@coop> windows test platform has changed this week, so i’m suspicious of a dev code change
03:51 <@coop> especially since it’s happening on all windows test platforms

Comment 1

[Carsten Book [:Tomcat]]

04:01 <@coop> oh, i meant to say *hasn’t* changed above


so checking code changes etc that could caused this

Comment 2

[Chris Cooper [:coop] (he/him)]

[1:26pm] coop: Tomcat|sheriffduty: so the error message is instructive…it says the ref is ambiguous
[1:26pm] coop: looking at the build log, i see two xul.pdb files being created
[1:28pm] coop: https://coop.pastebin.mozilla.org/5463834
[1:29pm] Tomcat|sheriffduty: hmmm coop  Bug 883339: Enable GTest on Windows except for PGO builds, r=glandium
[1:29pm] coop: checking the build before bbirtles, i only see the build/xul.pdb being created
[1:29pm] coop: and not the gtest
[1:29pm] Tomcat|sheriffduty: landed in https://tbpl.mozilla.org/?tree=Mozilla-Inbound&onlyunstarred=1&rev=3a5789d0e7e0
[1:30pm] coop: Tomcat|sheriffduty: that’s a definite culprit
[1:31pm] Tomcat|sheriffduty: yeah
[1:32pm] coop: so we’re only hitting this on non-PGO because it’s disabled otherwise?
[1:32pm] coop: hence intermittent
[1:32pm] Tomcat|sheriffduty: glandium: ^
[1:34pm] Tomcat|sheriffduty: yeah also no pgo build finished so far 
[1:35pm] Tomcat|sheriffduty: coop: so backing out bug 883339 and see how if the builds still show the problem or not i guess
[1:36pm] coop: Tomcat|sheriffduty: sounds like a plan

Comment 3

[Carsten Book [:Tomcat]]

bug 883339  has been backedout in https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=5a8e5ad43d90 and hoping for green builds

Comment 4

[(not currently active) Ted Mielczarek]

Yeah, so building gtest makes us link a second xul.dll, which means a second xul.pdb is created. We use fix_stack_using_bpsyms.py to symbolize assertion stacks, but it doesn't know how to deal with this situation:
http://hg.mozilla.org/mozilla-central/annotate/e86b84998b18/tools/rb/fix_stack_using_bpsyms.py#l88

Comment 5

[Carsten Book [:Tomcat]]

clobbered the tree and retriggered the build also now to see if this works after the backout from comment #3

Comment 6

[Ed Morley [:emorley]]

Green :-)

Comment 7

[Mike Hommey [:glandium]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #4)
> Yeah, so building gtest makes us link a second xul.dll, which means a second
> xul.pdb is created. We use fix_stack_using_bpsyms.py to symbolize assertion
> stacks, but it doesn't know how to deal with this situation:
> http://hg.mozilla.org/mozilla-central/annotate/e86b84998b18/tools/rb/
> fix_stack_using_bpsyms.py#l88

or... we make xul-gtest emit a different pdb.

Comment 8

[Mike Hommey [:glandium]]

There's something fishy, though. The pdb for xul-gtest should never have gotten in the build symbols in the first place... and that may well have happened because ... drum-roll ... of incremental builds.

Comment 9

[Mike Hommey [:glandium]]

And we only have the problem of duplicated xul symbols in the symbols package because windows builds are scanning for pdb files in the objdir, not in dist/bin (because they aren't in dist/bin), while other builds scan dist/bin or dist/universal.

Comment 10

[Mike Hommey [:glandium]]

There is a real bug here, independent of that landing, and it should be fixed. This affects gtest because of the fact that we have two pdbs with the same name there, but it can also happen if we remove libraries or executables. I think a way out would be to stage the PDBs to some directory under dist, which we could do directly when linking. Could be derived from FINAL_TARGET. Patch incoming.

Comment 11

[Mike Hommey [:glandium]]

Gah, but that doesn't work for third-parties. Sigh.

Comment 12

[Mike Hommey [:glandium]]

Here's a thought... MSVC debugger can find the pdbs when debugging firefox from dist/bin. Presumably, that means the full path to the pdb is somewhere in the executables/dlls. We could have symbolstore.py scan dist/bin and find the pdb by reading that info from the executables/dlls.

Comment 13

[Mike Hommey [:glandium]]

From the result of a try build:
$ strings *.dll *.exe| grep \\.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\accessible\interfaces\msaa\AccessibleMarshal.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\toolkit\crashreporter\injector\breakpadinjector.pdb
D3DCompiler_43.pdb
D3DCompiler_46.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\security\nss\lib\freebl\freebl3.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\layout\media\gkmedias.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\intl\icu\target\lib\icuin52.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\intl\icu\target\lib\icuuc52.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\gfx\angle\src\libEGL\libEGL.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\gfx\angle\src\libGLESv2\libGLESv2.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\memory\mozalloc\mozalloc.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\mozglue\build\mozglue.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\js\src\mozjs.pdb
msvcp100.i386.pdb
msvcr100.i386.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\security\build\nss3.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\security\nss\lib\ckfw\builtins\nssckbi.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\security\nss\lib\softoken\legacydb\nssdbm3.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\memory\replace\jemalloc\replace_jemalloc.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\security\nss\lib\softoken\softokn3.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\toolkit\library\build\xul.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\toolkit\crashreporter\client\crashreporter.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\browser\app\firefox.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\toolkit\components\maintenanceservice\maintenanceservice.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\ipc\app\plugin-container.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\dom\plugins\ipc\hangui\plugin-hang-ui.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\toolkit\mozapps\update\updater\updater.pdb
c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\webapprt\win\webapprt-stub.pdb

Comment 14

[Mike Hommey [:glandium]]

Relevant info:
http://www.debuginfo.com/examples/debugdir.html

Comment 15

[(not currently active) Ted Mielczarek]

I'm pretty sure I have Python code lying around somewhere that can read this info out. It's sort of a pain.

Comment 16

[(not currently active) Ted Mielczarek]

Actually the simplest thing to do is probably just
dumpbin -HEADERS <path to binary> | grep RSDS

On a firefox.exe I've built I get:
$ dumpbin -HEADERS ../debug-mozilla-central/dist/bin/firefox.exe | grep RSDS
    53A090B6 cv           4F 0000B674     A474    Format: RSDS, {CD1BA9F5-C3B3-406F-B195-996A86B50EB5}, 1, d:\build\debug-mozilla-central\browser\app\firefox.pdb

Comment 17

[(not currently active) Ted Mielczarek]

But really, if we want to put gtest in the test package at some point (and we do), we'll have to deal with this anyway. Your patch in bug 883339 seems fine.

Comment 18

[Mike Hommey [:glandium]]

Note, moving xul.dll back in toolkit/library in bug 1041936 triggered this bug again.

Comment 19

[Mike Hommey [:glandium]]

Moving nss to config/external/nss in bug 1049281 triggered this bug again.

Comment 20

[Chris Manchester (limited bugmail, email directly)]

Bug 1337986 ought to mean we're doing the right thing for incremental builds. We moved gtest to the test package a while back and dealt with disambiguating multiple xul symbol files with that.