Opened 6 years ago
Closed 3 years ago
#4893 closed enhancement (fixed)
Build 0ad using system libmozjs-52 / SpiderMonkey52
| Reported by: | Ludovic Rousseau | Owned by: | wraitii |
|---|---|---|---|
| Priority: | Release Blocker | Milestone: | Alpha 24 |
| Component: | Core engine | Keywords: | |
| Cc: | fabio, Krinkle | Patch: |
Description
I maintain the Debian package of 0ad. 0ad fails to build on arm64 because mozjs83 fails to build. The error is:
/<<PKGBUILDDIR>>/libraries/source/spidermonkey/mozjs-38.0.0/js/src/jit/shared/CodeGenerator-shared-inl.h:232:10: error: 'class js::jit::MacroAssembler' has no member named 'verifyHeapAccessDisassembly'
masm.verifyHeapAccessDisassembly(begin, end,
^~~~~~~~~~~~~~~~~~~~~~~~~~~
See https://buildd.debian.org/status/fetch.php?pkg=0ad&arch=arm64&ver=0.0.22-1&stamp=1508351579&raw=0 for the complete build log.
One solution would be to use the mozjs package provided by Debian and already available for arm64.
Debian unstable provides libmozjs-52-dev. I tried to patch 0ad (work-in-progress patch attached) to use this mozjs version but the build fails with:
../../../source/scriptinterface/NativeWrapperDecls.h:79:51: error: ‘jsval’ has not been declared
static bool callMethod(JSContext* cx, uint argc, jsval* vp);
^
../../../source/scriptinterface/NativeWrapperDecls.h:79:51: note: in definition of macro ‘OVERLOADS’
[...]
../../../source/scriptinterface/NativeWrapperDefns.h:133:50: error: cannot convert ‘int*’ to ‘JS::Value*’ for argument ‘2’ to ‘JS::CallArgs JS::CallArgsFromVp(unsigned int, JS::Value*)’
JS::CallArgs args = JS::CallArgsFromVp(argc, vp); \
it looks like the 0ad code needs some changes to migrate from mozjs-38 to mozjs-52.
Is there any interest to support a more recent (and supported, with security bugs fixed, etc.) version of mozjs for 0ad?
Attachments (1)
Change History (47)
by , 6 years ago
| Attachment: | mozjs52.patch added |
|---|
comment:2 by , 6 years ago
First we have to update to SpiderMonkey 45, work was started here https://github.com/leper/0ad/tree/sm45 but there are still a number of things todo (https://github.com/leper/0ad/blob/380f534acbeb04214307b7e6d8d49ee89146b39c/SM45_TODO).
You can find some previous SM upgrade tickets on trac, that is a considerable amount of work, much more than a small patch fixing some compile warnings.
For instance the SM38 upgrade was done in 35 commits: r18689
comment:3 by , 6 years ago
| Keywords: | mozjs removed |
|---|---|
| Summary: | Build 0ad using system libmozjs-52? → Build 0ad using system libmozjs-52 / SpiderMonkey52 |
comment:5 by , 6 years ago
| Keywords: | rfc added |
|---|---|
| Milestone: | Backlog → Work In Progress |
We have to find out what has changed meanwhile. Pyrogenesis code that is incompatible to sm45 will be incompatble to sm52 too, so skipping will be more work as far as I understand.
On the other hand compile warnings should always be addressed, so we should review the uploaded patch either way.
comment:6 by , 6 years ago
I'd be willing to donate some money for this work, ALT's e2k port has mozjs52 but getting mozjs38 there is problematic right now.
comment:7 by , 6 years ago
There is a patch with some days of started work for sm60: Phab:D1510. (I still don't know if it will be easier to jump to the latest stable or to progress one release at a time. The patch looked like it might be manageable to jump.)
comment:9 by , 6 years ago
Our sm60 patch could be next month or next decade too.
About the version, Itms mentioned in an internal thread that the next 'LTS' is SM60, but I couldn't verify this for now. According to https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Releases the latest release is SM52 and no sucessor officially scheduled yet.
comment:10 by , 5 years ago
| Keywords: | rfc removed |
|---|---|
| Milestone: | Work In Progress → Alpha 24 |
comment:11 by , 5 years ago
2 elexis: as previously noted, I'm willing to donate some money, especially if someone can do the transition from mozjs38 with an explicit stop at mozjs52 (that will gain a whole new target platform). Let it be 50 euros to be more specific, and please email me with proper contact if that's acceptable.
comment:12 by , 5 years ago
Notice that our SM includes few patches: https://trac.wildfiregames.com/browser/ps/trunk/libraries/source/spidermonkey The updated version might also require some, implementation will tell.
comment:13 by , 5 years ago
Notice that our SM includes few patches
It's fine as long as these are fixes not upstream-incompatible kludges for a particular project :-) SM52 is getting skipped unconditionally, right?.. (I know the "you need it you do it" rule, no moaning)
comment:14 by , 5 years ago
SM52 is getting skipped unconditionally, right?
By default 0ad is compiled with the spidermonkey version in 0ad/libraries/source/spidermonkey/, one can supposedly compile with a shared library using --with-system-mozjs38 wiki:BuildInstructions in case you mean that.
stop at mozjs52
Until there is a new version of spidermonkey that is tagged as "release" that is? Perhaps it wouldn't be too bad for pyrogenesis/0ad to to update beyond the most recent "release" as wraitii did, if the "release" doesn't really differ in stability or maintenance from the most recent version. Debian for instance has mozjs60 in their repository, but mozjs52 is still the most recent "release". The release packe for mozjs52 is completely empty https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Releases/52 and I can't even find the download via the method they describe after browsing through hundreds of mozjs52 patches. So I suppose "sm52" is the "prerelease" from https://ftp.mozilla.org/pub/spidermonkey/prereleases/52/pre1/mozjs-52.9.1pre1.tar.bz2 I'll review what wraitii and Itms did so far, an update of SM is overdue either way.
comment:15 by , 5 years ago
In r20062:
Replace deprecated jsval with JS::Value.
Remove unused SGUIBaseSettings and GUI comment.
Fix indentation of a macro, refs D794.
Differential Revision: https://code.wildfiregames.com/D838
Review by: leper.
Itms came up with the same patch for the SpiderMonkey 45 update independently.
comment:16 by , 5 years ago
| Cc: | added |
|---|
comment:18 by , 5 years ago
| Cc: | added |
|---|
comment:19 by , 5 years ago
The next Debian version (buster) should be released in July 2019. It will provide libmozjs-60-0 and libmozjs-52-0.
It would be fine to target support of libmozjs-60-0 (instead of version 52) if possible.
comment:24 by , 5 years ago
explicit stop at mozjs52
Meanwhile Firefox / SpiderMonkey is at revision 68, so it sounds hurtful not to go further.
I was wondering whether one could introduce a macro that changes the code to work with 52 at option, but that will probably not scale, because we probably want to start using more recnet spidermonkey version features, in specific rooting a std::vector or std::map in a JSContext as demonstrated in this sm60 example: https://github.com/spidermonkey-embedders/spidermonkey-embedding-examples/blob/esr60/examples/tracing.cpp
comment:25 by , 5 years ago
For one, I'm still eager to donate e.g. $50 for support of Firefox 52's mozjs *if* that comes feasible (I understand that it can be days or weeks of work more and the suggestion might be just inappropriate). My personal reason is that it's the topmost version of the engine I've got on e2k (Elbrus 2000) architecture, and getting 0ad here would be so nice :-)
comment:28 by , 5 years ago
Wasn't too sure where to drop this, but it appears SM70 might have serious performance optimisations for our use cases: https://hacks.mozilla.org/2019/08/the-baseline-interpreter-a-faster-js-interpreter-in-firefox-70/
comment:29 by , 4 years ago
TODO: It seems there is a SpiderMonkey segfault, see #5636, reproducible with:
Object.defineProperty(this, "f", {
get: function() {
[].keys
return (function() {
f();
});
}
});
f();
comment:31 by , 4 years ago
| Owner: | set to |
|---|
The migration to SM52 is ready to be tested. You can find the code at https://github.com/na-Itms/0ad/tree/spidermonkey
Here is the forum post describing the upgrade in detail: https://wildfiregames.com/forum/index.php?/topic/28583-spidermonkey-52-upgrade/
To address concerns above about upgrading to newer versions: we are going to try and use more recent versions, also available in the package managers. We do not plan to stick with SM52, unless forced by lack of manpower or unexpected issues.
comment:42 by , 3 years ago
| Owner: | changed from to |
|---|---|
| Priority: | Should Have → Release Blocker |
comment:46 by , 3 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
Closed in r24203 (not sure why it didn't trigger).
preliminary patch to build 0ad using system libmozjs-52