Opened 5 years ago

Closed 2 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:


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 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)

mozjs52.patch (4.6 KB ) - added by Ludovic Rousseau 5 years ago.
preliminary patch to build 0ad using system libmozjs-52

Download all attachments as: .zip

Change History (47)

by Ludovic Rousseau, 5 years ago

Attachment: mozjs52.patch added

preliminary patch to build 0ad using system libmozjs-52

comment:1 by Ludovic Rousseau, 5 years ago

Nobody interested in using a supported libmozjs library?

comment:2 by elexis, 5 years ago

First we have to update to SpiderMonkey 45, work was started here but there are still a number of things 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 elexis, 5 years ago

Keywords: mozjs removed
Summary: Build 0ad using system libmozjs-52?Build 0ad using system libmozjs-52 / SpiderMonkey52

comment:4 by Stan`, 5 years ago

Can't we skip to 52 directly ?

comment:5 by elexis, 5 years ago

Keywords: rfc added
Milestone: BacklogWork 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 Michael Shigorin, 5 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 elexis, 5 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:8 by Michael Shigorin, 5 years ago

Heh, I won't see 60 on e2k for quite some time I guess...

comment:9 by elexis, 5 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 the latest release is SM52 and no sucessor officially scheduled yet.

comment:10 by elexis, 4 years ago

Keywords: rfc removed
Milestone: Work In ProgressAlpha 24

comment:11 by Michael Shigorin, 4 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 elexis, 4 years ago

Notice that our SM includes few patches: The updated version might also require some, implementation will tell.

comment:13 by Michael Shigorin, 4 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 elexis, 4 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 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 I'll review what wraitii and Itms did so far, an update of SM is overdue either way.

comment:15 by elexis, 4 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: ​
Review by: leper.
Itms came up with the same patch for the SpiderMonkey 45 update independently.

comment:16 by fabio, 4 years ago

Cc: fabio added

comment:17 by Itms, 4 years ago

In 22052:

SpiderMonkey-related changes in preparation for the upgrade to SpiderMonkey 45, refs #4893.

Reviewed By: wraitii, elexis
Differential Revision:

comment:18 by Krinkle, 4 years ago

Cc: Krinkle added

comment:19 by Ludovic Rousseau, 4 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:20 by Itms, 4 years ago

In 22516:

Improvements to simulation hotloading before the SM upgrade, refs #4893.

SM45 will enforce property attributes described at We thus cannot hotload properties with the DontDelete attribute.
Fix an oversight in the hotloading code.
Rename a confusing parameter.

Reviewed By: wraitii
Differential Revision:

comment:21 by elexis, 3 years ago

In 22622:

Replace ScriptInterface ErrorReporter layered hack from rP8063 with proper code, refs rP8975.

Avoids preparation for the SpiderMonkey 45 update, refs #4893 / D1510 .
Add JSAutoRequest missing from rP8063 too.

Differential Revision:
Reviewed By: Itms

comment:22 by Itms, 3 years ago

In 22627:

Upgrade SpiderMonkey to version 45.0.2, refs #4893.

Thanks to wraitii, elexis, Krinkle and historic_bruno for contributions and comments, and to gentz, madpilot, s0600204 and Stan for testing and indirect contributions.

Differential Revision:

comment:23 by Itms, 3 years ago

In 22629:

Revert use of the isRunOnce flag after removal in rP22627 of compile'n go, refs #4893.

This breaked a SM assertion, which was only caught in debug mode. It could have led to subtle bugs during the compilation of JS scripts.
Still set the flag to its default value, because SM devs wanted to change the default in the future.

comment:24 by elexis, 3 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:

comment:25 by Michael Shigorin, 3 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:26 by elexis, 3 years ago

In 22660:

Update and premake5.lua with the with-system-mozjs45 flag from SpiderMonkey 45 in rP22627, refs #4893.

Differential Revision:

comment:27 by elexis, 3 years ago

In 22661:

Replace JS_THIS_OBJECT and JS::CallReceiver in preparation for SpiderMonkey 49 and 61.

Differential Revision:
Refs #4893, D1699, D2142, D844, rP7259, ...
Tested on: clang 8, VS2015, Jenkins

comment:28 by wraitii, 3 years ago

Wasn't too sure where to drop this, but it appears SM70 might have serious performance optimisations for our use cases:

comment:29 by elexis, 3 years ago

TODO: It seems there is a SpiderMonkey segfault, see #5636, reproducible with:

	Object.defineProperty(this, "f", {
	    get: function() {
	        return (function() {

comment:30 by Itms, 3 years ago

In 23214:

Patch SpiderMonkey 45 against bug, refs #4893.

Reported By: Dansasbu
Tested By: Dansasbu, Stan

Differential Revision:

comment:31 by Itms, 2 years ago

Owner: set to Itms

The migration to SM52 is ready to be tested. You can find the code at

Here is the forum post describing the upgrade in detail:

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:32 by wraitii, 2 years ago

In 24157:

Remove serializablePrototype code

The script interface has code to serialize/deserialize JS objects with a user-defined prototype. That code is usable in the AI, but currently unused (and there are no plans to use it in the long run).

Removing it allows removing more code down the line, which helps with the SM45-52 migration.

Collaboration with itms.

Refs #4893

Differential Revision:

comment:33 by wraitii, 2 years ago

In 24167:

Use Symbols to store JS object references when serialising and delete ObjectIDCache

When serialising JS objects, we keep track of any encountered object, and serialize it only once. Any further serialisation instead stores an ID referring to the original object (essentially an opaque pointer).
The trouble of course is to have a unique, persistent identifier for such an object.
svn uses an ObjectIDCache, essentially a "JS Object -> ID" map (which internally is essentially a "JS heap pointer -> ID" map).

JS, since ES15, includes a "Symbol" primitive type, which is a unique, immutable identifier. They are also not iterable by or GetOwnPropertyName or related.
This means they can be used to store the tag directly on the object (since it's impossible overwrite a user property).
Thanks to this, we can forgo ObjectIDCache in the serializers, and since following D2897 it becomes unused, we can delete it, along with the Finalization code it used.

Part of SM52 migration, stage: SM45-compatible changes.

Patch by: Itms

Tested By: Freagarach

Refs #4893

Differential Revision:

comment:34 by wraitii, 2 years ago

In 24169:

Use JS::Trace over CallXTracer

CallXTracer functions were removed in

Part of the SM52 migration, stage: SM45 compatible.

Patch by: Itms

Tested By: Freagarach

Refs #4893

Differential Revision:

comment:35 by wraitii, 2 years ago

In 24170:

Remove DefPersistentRooted and unneeded includes.

DefPersistentRooted is essentially a wrapper around unique_ptr and has no real reason to exist.

Part of SM52 migration, stage: SM45 compatible.

Patch by: Itms

Tested by: Freagarach

Refs #4893

Differential Revision:

comment:36 by wraitii, 2 years ago

In 24171:

Encapsulate runtime creation.

  • Makes it easier to change down the line (and change is coming)
  • Allows making g_ScriptRuntime thread-local easily.
  • Remove ParentRuntime, which is not used at the moment.

Part of the SM52 migration, stage: SM45 compatible.

Patch by: Itms

Tested By: Freagarach

Refs #4893

Differential Revision:

comment:37 by wraitii, 2 years ago

In 24176:

Wrap JSAutoRequest and replace usage of JSContext* with the wrapper.

JSAutoRequest is required before calling into most JSAPI methods, for GC reasons.
Calling it is required and fragile as one must not forget.
Further, SM52 and later make manipulating JSContext* dangerous as that can cross Compartment(Realm in SM68) barriers (and ScriptInterface now matches a Compartment).

The solution to both problems is to avoid using JSContext* in 0 A.D. itself. To achieve this, a Request class is introduced, and must be used to access a JSContext* from a scriptInterface. Further, Request is passed to other ScriptInterface functions isntead of JSContext*, making it obvious that the caller has already called it, reducing errors and redundant JSAutoRequest calls.
Only JSNative functions now get a naked JSContext* without protection, but the likelihood of forgetting a request is lower since many ScriptInterface functions now expect it.

JSContext* is directly passed to JSAPI functions only.

Part of the SM52 migration, stage: SM45 compatible

Based on a patch by: Itms

Tested By: Freagarach

Refs #4893

Differential Revision:

comment:38 by wraitii, 2 years ago

In 24177:

Mass rename CxPrivate to CmptPrivate.

As part of the SM45->52 migration, a ScriptInterface becomes a wrapper around a JSCompartment, not a JSContext, thus we ought to store private data for the compartment and not the context.
This is a mass rename of CxPrivate to CmptPrivate to match that before the actual changes.

Part of the SM52 migration, stage: SM45 compatible

Patch by: Itms

Tested By: Freagarach

Refs #4893

Differential Revision:

comment:39 by wraitii, 2 years ago

In 24180:

Explicitly make ScriptInterface a Compartment wrapper.

ScriptInterface is now a wrapper around a JSCompartment, and thus always has a well-defined global.

The error reporter is moved to ScriptRuntime in anticipation of that handling JSContext in a later diff.

Part of the SM52 migration, stage: SM45 compatible.

Patch by: Itms

Tested By: Freagarach

Refs #4893

Differential Revision:

comment:40 by wraitii, 2 years ago

In 24181:

Rename ScriptRuntime to ScriptContext

SM52 essentially replaces JSRuntime with JSContext (though JSContext itself was replaced with JSCompartment).
To prepare for this migration, rename all Runtime-related things to Context.

Part of the SM52 migration, stage: SM45 compatible.

Patch by: Itms

Refs #4893

Differential Revision:

comment:41 by wraitii, 2 years ago

In 24182:

Fix JS OOM in tests following rP24180

rP24180 made ScriptInterface not recreate a new JSContext since it becomes a compartment wrapper, but that means we need to GC tests or they might OOM.

To make it mostly seamless, GC on any test setup. The JS tests are pretty close to the 16Mb limit as it stands so GC them manually too, for good measure.

Refs #4893

Differential Revision:

comment:42 by wraitii, 2 years ago

Owner: changed from Itms to wraitii
Priority: Should HaveRelease Blocker

comment:43 by wraitii, 2 years ago

In 24187:

Improve JS Exception handling.

  • Check for pending exceptions after function calls and script executions.
  • Call LOGERROR instead of JS_ReportError when there is a conversion error in FromJSVal, since that can only be called from C++ (where JS errors don't really make sense). Instead, C++ callers of FromJSVal should handle the failure and, themselves, either report an error or simply do something else.
  • Wrap JS_ReportError since that makes updating it later easier.

This isn't a systematical fix since ToJSVal also ought return a boolean for failures, and we probably should trigger errors instead of warnings on 'implicit' conversions, rather a preparation diff.

Part of the SM52 migration, stage: SM45 compatible (actually SM52 incompatible, too).

Based on a patch by: Itms

Comments by: Vladislavbelov, Stan`

Refs #742, #4893

Differential Revision:

comment:44 by wraitii, 2 years ago

In 24202:

[SM52 1/2] Upgrade Spidermonkey build system and binaries to 52.9.1

Of note is the change to static linking on *nix (MacOS already used it). The logic is that we can only use one single version of SM, so that advantage of dynamic linking is lost regardless. We might also see performance gains in the future with LTO enabled. It is also slightly easier to distribute the program as a result. Expect a negligible size increase in the binary size (offset somewhat by no longer needing to distribute .so files). Finally, it streamlines the build script.

Also noteworthy in this commit:

  • The MacOS build script is folded back into the general build script.
  • the perl/sed command is replaced by patching the configuration file, which at least warns if it starts failing in the future.

Binaries for windows provided by @Itms
The bulk of the patching was also done by @Itms.

Tested by: Stan, Freagarach

Refs #4893

Differential Revision:

comment:45 by wraitii, 2 years ago

In 24205:

[SM52 3/2] Revert static linking change on linux - other fixes.

This reverts static linking on linux from rP24202, it appears to fail to link and/or crash, depending.

  • Commit the tarball.
  • Update

Refs #4893

Differential Revision:

comment:46 by wraitii, 2 years ago

Resolution: fixed
Status: newclosed

Closed in r24203 (not sure why it didn't trigger).

Note: See TracTickets for help on using tickets.