1251313 - xpcshell.exe unable to find pgort140.dll during PGO build on Visual Studio 2015

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Gregory Szorc [:gps]]

When performing a PGO build (export MOZ_PGO=1) on VS2015, there is a startup crash in xpcshell.exe during the build (xpcshell.exe is invoked as part of doing startup cache foo).

I haven't yet grabbed a stack.

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

:ted, would you know what is going on here?  It looks like xpcshell is really a small wrapper to libxul, maybe something else is odd.

it would be good to know if vs2015 works for opt/debug builds as well.

Comment 2

[Gregory Szorc [:gps]]

I was able to get a non-PGO build on VS2015 to work apparently just fine. This xpcshell.exe crasher (which also impacts firefox.exe) appears to be PGO only.

Strangely, I was unable to get a stack trace, even if I launched xpcshell.exe from within Visual Studio's "start debugging" interface. I did, however, receive an error about a missing .dll that had "pgo" and some version number in the name. Not sure if I was invoking the executable incorrectly or whether the invocation by the build system hit some other error.

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

maybe we can use a static version of xpcshell.exe instead?  or maybe compile xpcshell as opt and use that instead of a pgo version?

Comment 4

[(not currently active) Ted Mielczarek]

(In reply to Gregory Szorc [:gps] from comment #2)
> I was able to get a non-PGO build on VS2015 to work apparently just fine.
> This xpcshell.exe crasher (which also impacts firefox.exe) appears to be PGO
> only.
> 
> Strangely, I was unable to get a stack trace, even if I launched
> xpcshell.exe from within Visual Studio's "start debugging" interface. I did,
> however, receive an error about a missing .dll that had "pgo" and some
> version number in the name. Not sure if I was invoking the executable
> incorrectly or whether the invocation by the build system hit some other
> error.

You probably just need PATH set properly, like:
https://dxr.mozilla.org/mozilla-central/rev/918df3a0bc1c4d07299e4f66274a7da923534577/toolkit/mozapps/installer/packager.py#83

Comment 5

[(not currently active) Ted Mielczarek]

(In reply to Joel Maher (:jmaher) from comment #3)
> maybe we can use a static version of xpcshell.exe instead?  or maybe compile
> xpcshell as opt and use that instead of a pgo version?

We just need to figure out why this is crashing and fix it. It could be a bug in our code or a compiler bug. Unfortunately crashes in this phase of the build are just a pain to diagnose.

Comment 6

[Gregory Szorc [:gps]]

This is in fact due to a missing PATH entry. Patch forthcoming.

Comment 7

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1251313 - Adjust PATH to find pgort140.dll under Visual Studio 2015; r?ted

32-bit PGO builds need to modify the PATH to find pgortXXX.dll. We were
doing this for Visual Studio 2013 (VC12) in 2 locations. We weren't
doing it for Visual Studio 2015. This resulted in a failure to launch
PGO instrumented executables because pgort140.dll could not be found.

This commit refactors the PATH munging to support Visual Studio 2015.

Review commit: https://reviewboard.mozilla.org/r/37035/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/37035/

Comment 8

[(not currently active) Ted Mielczarek]

Comment on attachment 8724461 [details]
MozReview Request: Bug 1251313 - Adjust PATH to find pgort140.dll under Visual Studio 2015; r?ted

https://reviewboard.mozilla.org/r/37035/#review33657

I can't believe I remembered this enough to remark on it, but not enough to notice that we had a special-case for VC2013 only!

::: build/pgo/profileserver.py:61
(Diff revision 1)
> -      vc12dir = os.path.abspath(os.path.join(env["VS120COMNTOOLS"],
> +        for e in ('VS140COMNTOOLS', 'VS120COMNTOOLS'):

I wonder if we can future-proof this by grabbing the MSVC version number from somewhere? I'm not sure we actually subst that from configure, but we could. Might save us some headache the next time around...

Comment 9

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/37035/#review33657

> I wonder if we can future-proof this by grabbing the MSVC version number from somewhere? I'm not sure we actually subst that from configure, but we could. Might save us some headache the next time around...

I'll consider a follow-up.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/708e99f3dd44

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/708e99f3dd44