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

RESOLVED FIXED in Firefox 47

Status

defect
RESOLVED FIXED
3 years ago
Last year

People

(Reporter: gps, Assigned: gps)

Tracking

unspecified
mozilla47
Dependency tree / graph

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

3 years ago
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.
: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.
Flags: needinfo?(ted)
Assignee

Comment 2

3 years ago
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.
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?
(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
Flags: needinfo?(ted)
(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.
Assignee

Comment 6

3 years ago
This is in fact due to a missing PATH entry. Patch forthcoming.
Assignee: nobody → gps
Status: NEW → ASSIGNED
Summary: Startup crash in xpcshell.exe during PGO build on VS2015 → xpcshell.exe unable to find pgort140.dll during PGO build on Visual Studio 2015
Assignee

Comment 7

3 years ago
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/
Attachment #8724461 - Flags: review?(ted)
Attachment #8724461 - Flags: review?(ted) → review+
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...
Assignee

Comment 9

3 years ago
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 11

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/708e99f3dd44
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47

Updated

Last year
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.