MOZ_CRASHREPORTER=1 not defined in default aarch64 build but is in default i686 build
Categories
(Firefox Build System :: General, defect)
Tracking
(firefox66 verified, firefox67 verified)
People
(Reporter: cpearce, Assigned: gsvelto)
References
Details
(Whiteboard: [qa-triaged])
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
lizzard
:
approval-mozilla-beta+
|
Details |
I'm trying to get an x86 plugin-container.exe to host an x86 GMP for use from an aarch64 parent process. One of the problems I encountered was that if you don't specify --disable-crashreporter in an i686 build, MOZ_CRASHREPORTER gets defined to 1. Whereas if you don't specify --disable-crashreporter in an aarch64 build, MOZ_CRASHREPORTER is defined to ''.
This discrepency caused my parent aarch64 process to not append the command line args used by plugin-container to init the crash reporter in the GMP process [1], but the x86 plugin-container was expecting them and so consumed command line args that it wasn't supposed to and which were required later [2] and so we crashed on startup.
[1] https://searchfox.org/mozilla-central/rev/78cd247b5d7a08832f87d786541d3e2204842e8e/toolkit/xre/nsEmbedFunctions.cpp#509
[2] https://searchfox.org/mozilla-central/rev/78cd247b5d7a08832f87d786541d3e2204842e8e/toolkit/xre/nsEmbedFunctions.cpp#599
This will likely be addressed by bug 1513284. In particular see bug 1513284 comment 4.
Comment 2•6 years ago
|
||
Gabriele, is there a reason to keep the crashreporter disabled locally at this point?
Assignee | ||
Comment 3•6 years ago
|
||
No, there should be no difference between the two platforms. We're probably missing some bit in the default configuration. I'll check.
Assignee | ||
Comment 4•6 years ago
|
||
We're missing a bit in old-configure.in. I'll fix it.
Assignee | ||
Comment 5•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Pushed by gsvelto@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6d30d431b5d8 Ensure that MOZ_CRASHREPORTER is properly defined in Windows/AArch64 builds r=glandium
Comment 7•6 years ago
|
||
bugherder |
Assignee | ||
Comment 8•6 years ago
|
||
Comment on attachment 9040401 [details]
Bug 1524142 - Ensure that MOZ_CRASHREPORTER is properly defined in Windows/AArch64 builds
Beta/Release Uplift Approval Request
Feature/Bug causing the regression
User impact if declined
The crash reporter is not enabled by default in builds
Is this code covered by automated tests?
No
Has the fix been verified in Nightly?
Yes
Needs manual test from QE?
Yes
If yes, steps to reproduce
See comment 0
List of other uplifts needed
None
Risk to taking this patch
Low
Why is the change risky/not risky? (and alternatives if risky)
We're only defining an environment variable during the build
String changes made/needed
None
Comment on attachment 9040401 [details]
Bug 1524142 - Ensure that MOZ_CRASHREPORTER is properly defined in Windows/AArch64 builds
Let's take this now for beta 6 and verify the results in beta.
Updated•6 years ago
|
Comment 10•6 years ago
|
||
bugherder uplift |
Comment 11•6 years ago
|
||
I have attempted to validate this fix, but these builds don't seem to run on Lenovo Yoga C630-13Q50 with Windows 10 Home (v1803).
I have attempted to install this installer:
http://archive.mozilla.org/pub/firefox/nightly/2019/02/2019-02-07-21-44-23-mozilla-central/firefox-67.0a1.en-US.win64-aarch64.installer.exe
which installed successfully, but simply did not open.
I have also tried these:
http://archive.mozilla.org/pub/firefox/nightly/2019/02/2019-02-07-21-44-23-mozilla-central/firefox-67.0a1.en-US.win64-aarch64.zip
http://archive.mozilla.org/pub/firefox/nightly/2019/02/2019-02-07-09-48-41-mozilla-central/firefox-67.0a1.en-US.win64-aarch64.zip
Assignee | ||
Comment 12•6 years ago
|
||
I believe that we're running into bug 1526250 which makes it hard to verify this, Chris is the problem fixed for you in recent builds?
Reporter | ||
Comment 13•6 years ago
|
||
I updated my tree, and I see MOZ_CRASHREPORTER being set by configure in both my aarch64 and i686 builds:
$ grep MOZ_CRASHREPORTER obj-i686-dbg/config.status obj-aarch64-dbg/config.status
obj-i686-dbg/config.status: 'MOZ_CRASHREPORTER': '1',
obj-i686-dbg/config.status: 'MOZ_CRASHREPORTER_INJECTOR': '1',
obj-i686-dbg/config.status: 'MOZ_CRASHREPORTER': '1',
obj-i686-dbg/config.status: 'MOZ_CRASHREPORTER_INJECTOR': '1',
obj-aarch64-dbg/config.status: 'MOZ_CRASHREPORTER': '1',
obj-aarch64-dbg/config.status: 'MOZ_CRASHREPORTER_INJECTOR': '',
obj-aarch64-dbg/config.status: 'MOZ_CRASHREPORTER': '1',
Note that MOZ_CRASHREPORTER_INJECTOR is not making it into the defines section on the aarch64 build, but is on the i686 build. Is that expected?
My mozconfigs:
$ cat i686.dbg.mozconfig
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj-i686-dbg
ac_add_options --enable-debug
ac_add_options --enable-debug-symbols
ac_add_options --disable-optimize
ac_add_options --target=i686
mk_add_options AUTOCLOBBER=1
ac_add_options --disable-tests
#opt!
export WIN32_REDIST_DIR="C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Redist\MSVC\14.16.27012\x86\Microsoft.VC141.CR"T
ac_add_options --disable-launcher-process
$ cat aarch64.dbg.mozconfig
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj-aarch64-dbg
#ac_add_options --disable-optimize
ac_add_options --enable-debug
ac_add_options --enable-debug-symbols
ac_add_options --disable-tests
mk_add_options AUTOCLOBBER=1
ac_add_options --target=aarch64
export WIN32_REDIST_DIR="C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Redist\MSVC\14.16.27012\arm64\Microsoft.VC141.CRT"
ac_add_options --disable-launcher-process
Comment 14•6 years ago
|
||
(In reply to Chris Pearce [:cpearce (GMT+13)] from comment #13)
Note that MOZ_CRASHREPORTER_INJECTOR is not making it into the defines section on the aarch64 build, but is on the i686 build. Is that expected?
MOZ_CRASHREPORTER_INJECTOR
is an x86-only thing for use with the Flash plugin so that sounds OK and shouldn't cause problems.
Assignee | ||
Comment 15•6 years ago
|
||
As Ted said, I think we can mark this as verified then.
Comment 16•6 years ago
|
||
Considering that the Aarch64 builds can now run and considering the fact that this uplift is marked so that is should be verified manually, can you please tell me how I should verify it?
I have attempted to verify this fix in Nightly first, by confirming that the "--enable-crashreporter" property is displayed in a build with the fix, while it doesn't display in a build prior to the fix (in the "about:buildconfig" page). The problem is that the property is displayed in builds both prior and after the fix. I assume my steps to reproduce are incorrect.
Builds tested:
- Prior to fix: Nightly v67.0a1 (2019-02-01) (64-bit) (Aarch64 build)
- After the fix: nightly v67.0a1 (2019-02-11) (64-bit) (Aarch64 build)
How should I verify this fix? Thanks.
Comment 17•6 years ago
|
||
After talking with :gsvelto on slack, it appears that this issue was verified by the reporter and should be marked as such.
Thank you!
Description
•