Closed Bug 1315512 Opened 8 years ago Closed 8 years ago

LZMA/XZ library compression breaks x86 Fennec builds

Categories

(Firefox Build System :: Android Studio and Gradle Integration, defect)

Product:
Firefox Build System
Component:
Android Studio and Gradle Integration
Platform:
x86
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox52 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla52
Tracking Flags:
Tracking Status
firefox52 --- fixed

People

(Reporter: nalexander, Assigned: nalexander)

References

Details

Not sure what's going on here, but I see this with an x86 Nightly, with a locally built x86 build (compile environment), and a locally built x86 artifact build (disable compile environment).

The APK appears to have a STORE'd assets/x86/libnss3.so, which does have a valid XZ header (_7zXZ).

11-05 21:48:50.672  3337  3371 E GeckoLibLoad: Load sqlite start
11-05 21:48:50.673  3337  3371 E GeckoLinker: XZ decoding: unsupported header options
11-05 21:48:50.673  3337  3371 E GeckoLinker: Error decoding XZ file /data/user/0/org.mozilla.fennec/cache/libnss3.so
11-05 21:48:50.673  3337  3371 E GeckoLinker: Not a seekable zstream
11-05 21:48:50.673  3337  3371 E GeckoLibLoad: Couldn't get a handle to libnss3!
11-05 21:48:50.674  3337  3371 E GeckoLibLoad: Throw
11-05 21:48:50.675  3337  3371 E GeckoLibLoad: Load sqlite done
11-05 21:48:50.677  3337  3371 E GeckoCrashHandler: >>> REPORTING UNCAUGHT EXCEPTION FROM THREAD 173 ("Gecko")
11-05 21:48:50.677  3337  3371 E GeckoCrashHandler: java.lang.Exception: Error loading sqlite libraries
11-05 21:48:50.677  3337  3371 E GeckoCrashHandler: 	at org.mozilla.gecko.mozglue.GeckoLoader.loadSQLiteLibsNative(Native Method)
11-05 21:48:50.677  3337  3371 E GeckoCrashHandler: 	at org.mozilla.gecko.mozglue.GeckoLoader.loadSQLiteLibs(GeckoLoader.java:222)
11-05 21:48:50.677  3337  3371 E GeckoCrashHandler: 	at org.mozilla.gecko.GeckoThread.initGeckoEnvironment(GeckoThread.java:398)
11-05 21:48:50.677  3337  3371 E GeckoCrashHandler: 	at org.mozilla.gecko.GeckoThread.run(GeckoThread.java:496)
gbrown: CC to you to file follow-up to run *some* test to make sure we can start Fennec on x86.  Right now we run no *actual* Fennec tests against x86 builds, just highly artificial xpcshell tests.
The issue is that xz-embedded is mis-configured, resulting in BCJ not being legal on Android x86.  The following patch addresses the issue.  I would prefer to make this an exhaustive switch, but I can't since it's not clear what the legal target CPU space really is.

esawin, what do you think?

diff --git a/modules/xz-embedded/moz.build b/modules/xz-embedded/moz.build
--- a/modules/xz-embedded/moz.build
+++ b/modules/xz-embedded/moz.build
@@ -13,19 +13,21 @@ UNIFIED_SOURCES += [
     'src/xz_crc64.c',
     'src/xz_dec_bcj.c',
     'src/xz_dec_lzma2.c',
     'src/xz_dec_stream.c',
 ]

 ALLOW_COMPILER_WARNINGS = True

-if CONFIG['TARGET_CPU'] == 'arm':
+if CONFIG['TARGET_CPU'].startswith('arm'):
+    # Accept arm, armv7, etc.
     if CONFIG['MOZ_THUMB2']:
         DEFINES['XZ_DEC_ARMTHUMB'] = 1
     else:
         DEFINES['XZ_DEC_ARM'] = 1
-elif CONFIG['TARGET_CPU'] == 'x86':
+elif '86' in CONFIG['TARGET_CPU']:
+    # Accept x86, x86_64, i386, i686, etc.
     DEFINES['XZ_DEC_X86'] = 1

 DEFINES['XZ_USE_CRC64'] = 1

 Library('xz-embedded')
Flags: needinfo?(esawin)
XZ_DEC_X86 should work for both, x86 and x86_64. I'm not sure why the current configuration would cause the issues on x86, BCJ filters are optional.

To me it looks like we're using a non-compatible filter for the target platform, e.g., XZ_DEC_ARMTHUMB on x86.

The patch looks good in any case.
Flags: needinfo?(esawin)
(In reply to Eugen Sawin [:esawin] from comment #3)
> XZ_DEC_X86 should work for both, x86 and x86_64. I'm not sure why the
> current configuration would cause the issues on x86, BCJ filters are
> optional.
> 
> To me it looks like we're using a non-compatible filter for the target
> platform, e.g., XZ_DEC_ARMTHUMB on x86.

No, the issue is that the target is "i{3,6}86", not "x86" at all.

> The patch looks good in any case.

OK, can you land it?  I'm on leave until January :)
Flags: needinfo?(esawin)
(In reply to Nick Alexander :nalexander (leave until January 2017) from comment #4)
> (In reply to Eugen Sawin [:esawin] from comment #3)
> > XZ_DEC_X86 should work for both, x86 and x86_64. I'm not sure why the
> > current configuration would cause the issues on x86, BCJ filters are
> > optional.
> > 
> > To me it looks like we're using a non-compatible filter for the target
> > platform, e.g., XZ_DEC_ARMTHUMB on x86.
> 
> No, the issue is that the target is "i{3,6}86", not "x86" at all.

Which should not cause any issues, we would just not use any BCJ filter in this case, which are optional.

> > The patch looks good in any case.
> 
> OK, can you land it?  I'm on leave until January :)

Sure!
Flags: needinfo?(esawin)
(In reply to Eugen Sawin [:esawin] from comment #5)
> (In reply to Nick Alexander :nalexander (leave until January 2017) from
> comment #4)
> > (In reply to Eugen Sawin [:esawin] from comment #3)
> > > XZ_DEC_X86 should work for both, x86 and x86_64. I'm not sure why the
> > > current configuration would cause the issues on x86, BCJ filters are
> > > optional.
> > > 
> > > To me it looks like we're using a non-compatible filter for the target
> > > platform, e.g., XZ_DEC_ARMTHUMB on x86.
> > 
> > No, the issue is that the target is "i{3,6}86", not "x86" at all.
> 
> Which should not cause any issues, we would just not use any BCJ filter in
> this case, which are optional.

Oh, I see your confusion: you test target CPU in the xz-embedded moz.build (https://dxr.mozilla.org/mozilla-central/source/modules/xz-embedded/moz.build#26), but you test CPU_ARCH in the packaging code (https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/action/package_fennec_apk.py?q=%22--lzma2%22&redirect_type=single#81).  Those two things are different, leading to BCJ in packaging and no support at runtime.
(In reply to Nick Alexander :nalexander (leave until January 2017) from comment #6)
> (In reply to Eugen Sawin [:esawin] from comment #5)
> > (In reply to Nick Alexander :nalexander (leave until January 2017) from
> > comment #4)
> > > (In reply to Eugen Sawin [:esawin] from comment #3)
> > > > XZ_DEC_X86 should work for both, x86 and x86_64. I'm not sure why the
> > > > current configuration would cause the issues on x86, BCJ filters are
> > > > optional.
> > > > 
> > > > To me it looks like we're using a non-compatible filter for the target
> > > > platform, e.g., XZ_DEC_ARMTHUMB on x86.
> > > 
> > > No, the issue is that the target is "i{3,6}86", not "x86" at all.
> > 
> > Which should not cause any issues, we would just not use any BCJ filter in
> > this case, which are optional.
> 
> Oh, I see your confusion: you test target CPU in the xz-embedded moz.build
> (https://dxr.mozilla.org/mozilla-central/source/modules/xz-embedded/moz.
> build#26), but you test CPU_ARCH in the packaging code
> (https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/
> action/package_fennec_apk.py?q=%22--lzma2%22&redirect_type=single#81). 
> Those two things are different, leading to BCJ in packaging and no support
> at runtime.

Wow, thanks, forgot about that. Can I land this with r=nalexander?
(In reply to Eugen Sawin [:esawin] from comment #7)
> (In reply to Nick Alexander :nalexander (leave until January 2017) from
> comment #6)
> > (In reply to Eugen Sawin [:esawin] from comment #5)
> > > (In reply to Nick Alexander :nalexander (leave until January 2017) from
> > > comment #4)
> > > > (In reply to Eugen Sawin [:esawin] from comment #3)
> > > > > XZ_DEC_X86 should work for both, x86 and x86_64. I'm not sure why the
> > > > > current configuration would cause the issues on x86, BCJ filters are
> > > > > optional.
> > > > > 
> > > > > To me it looks like we're using a non-compatible filter for the target
> > > > > platform, e.g., XZ_DEC_ARMTHUMB on x86.
> > > > 
> > > > No, the issue is that the target is "i{3,6}86", not "x86" at all.
> > > 
> > > Which should not cause any issues, we would just not use any BCJ filter in
> > > this case, which are optional.
> > 
> > Oh, I see your confusion: you test target CPU in the xz-embedded moz.build
> > (https://dxr.mozilla.org/mozilla-central/source/modules/xz-embedded/moz.
> > build#26), but you test CPU_ARCH in the packaging code
> > (https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/
> > action/package_fennec_apk.py?q=%22--lzma2%22&redirect_type=single#81). 
> > Those two things are different, leading to BCJ in packaging and no support
> > at runtime.
> 
> Wow, thanks, forgot about that. Can I land this with r=nalexander?

Sure, land whatever version you think is best, r=me (or r=you, since I wrote the patch).
Component: First Run → Build Config & IDE Support
OS: Unspecified → Android
Hardware: Unspecified → x86
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b2721e0ef0ef
Fix XZ library decompression on x86. r=esawin
Assignee: nobody → nalexander
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/b2721e0ef0ef
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 52 → mozilla52
You need to log in before you can comment on or make changes to this bug.