Building JS library from scratch

[phkb]

Could the JS in play via GDB be using an even stricter form of "use strict" that might enforce explicit array declarations, or something like that? This is way outside my field of knowledge!

[another_commander]

Don't know. I would say it's rather improbable but the intricacies of gdb are not my field either.

[mcarans]

I had to fix the oolite-contracts-helpers.js file like this to get past the failure:

Code: Select all

this.$clientNames = ["", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", ""];

I'm now looking at the next point of failure.

This is not the right approach though. As phkb already stated, the Oolite code works fine the way it is with the distributed js dll. You should not try to debug Oolite's code at this stage, but identify the root cause of the mozjs crash instead. With the above "fix" you are simply side-stepping the problem.

The JavaScript contained a bug in which memory was being written to that had not being initialised as I understand it, something like a buffer overflow or are you saying the original JavaScript was valid? If not, are you saying that you would like JavaScript files with these sorts of bugs to just work? If this is the case, then there seems little point in me continuing my investigation as I am highly doubtful it will be possible given so much has changed since the last time the library was built and those changes might well affect memory allocation given concerns over buffer overflows as attack vectors etc. What do you think?

The thing that puzzles me is why the debug build worked fine without gdb despite these JS issues. It is only the release build that fails with or without gdb.

[another_commander]

The JavaScript contained a bug in which memory was being written to that had not being initialised as I understand it, something like a buffer overflow or are you saying the original JavaScript was valid? If not, are you saying that you would like JavaScript files with these sorts of bugs to just work? If this is the case, then there seems little point in me continuing my investigation as I am highly doubtful it will be possible given so much has changed since the last time the library was built and those changes might well affect memory allocation given concerns over buffer overflows as attack vectors etc. What do you think?

The thing that puzzles me is why the debug build worked fine without gdb despite these JS issues. It is only the release build that fails with or without gdb.

I am saying that the original JS was valid. Any crashes or errors you are getting at this stage are more likely to originate from the way mozjs is built rather than Oolite's code.

The debug mozjs uses different code paths compared to the release one. That could explain the debug build working and release crashing.

[mcarans]

I am saying that the original JS was valid. Any crashes or errors you are getting at this stage are more likely to originate from the way mozjs is built rather than Oolite's code.

The debug mozjs uses different code paths compared to the release one. That could explain the debug build working and release crashing.

Ah OK. Then next step is to diff your build with https://github.com/OoliteProject/spidermonkey-ff4 to see what's different about it and apply those changes to mine.

EDIT: I see quite a few differences, so I think it will be easier to start with your version and apply any changes I had to make to mine to yours.

[mcarans]

Just to double-check, you are building with JIT (just-in-time compiler) enabled, as well as the flags needed by Oolite, right? When I was building it I had to set these flags:

Code: Select all

-DJS_TRACER -DFEATURE_NANOJIT -DAVMPLUS_64BIT -DAVMPLUS_AMD64 -DJS_CPU_X64 -DJS_PUNBOX64 -DENABLE_ASSEMBLER -DENABLE_YARR_JIT -DJS_METHODJIT=1 -DJS_MONOIC=1 -DJS_POLYIC=1 -DJS_POLYIC_TYPED_ARRAY=1 -DMOZ_TRACE_JSCALLS=1

...

You set -DJS_METHODJIT=1 above in your flags, yet the js-config.h in your download has:

Code: Select all

/* Some mozilla code uses JS-friend APIs that depend on JS_TRACER being
   correct. */
//#undef JS_TRACER
//#define JS_METHODJIT 1

Why is this commented out? Is methodjit being used or did it cause problems? The reason I ask is that I see methodjit in the stacktrace when using my existing build of mozjs (based off the spidermonkey repo) with Oolite.

[mcarans]

In case it helps, I have packaged my working folders for building js and nspr for Oolite on Windows here.

I was able to build nspr with a few small changes:

Code: Select all

DEFINES	+= -D_NSPR_BUILD_ -Wno-error=incompatible-pointer-types

in nspr4\pr\src\io\Makefile and nspr4\pr\src\md\windows\Makefile.

Code: Select all

LD		= /mingw64/bin/ld

Code: Select all

RC		= /mingw64/bin/windres

in nspr4\config\autoconf.mk

[another_commander]

You set -DJS_METHODJIT=1 above in your flags, yet the js-config.h in your download has:

Code: Select all

/* Some mozilla code uses JS-friend APIs that depend on JS_TRACER being
   correct. */
//#undef JS_TRACER
//#define JS_METHODJIT 1

Why is this commented out? Is methodjit being used or did it cause problems? The reason I ask is that I see methodjit in the stacktrace when using my existing build of mozjs (based off the spidermonkey repo) with Oolite.

I think the commenting out was done to stop js-config from silently un-setting the JS_METHODJIT and JS_TRACER macros that were set to 1 in the makefile. A better way would have probably been to keep them set to 1 in js-config, but it is what it is. In any case, the js32ECMAv5 dll in Oolite's distribution is definitely built with JS_TRACER and JS_METHODJIT defined to 1 and I am certain because I see the output of the make commands as each file is compiled and I see there that the macros are in.

However...

Your observation made me realize that in Oolite's deps, the two macros are also not set because when I set up the include folder for js I just copied over what was there in my setup, with the two macros commented out. Unless I am missing something here, that probably means that Oolite was not getting the JIT part in its executable. - edit: actually the JSEngine starts up with the explicit options JSOPTION_JIT | JSOPTION_METHODJIT | JSOPTION_PROFILING so we have always been OK I guess, still it's good to have consistency and define things correctly in the headers. I have enabled the two macros in my local js-config inside oolite-deps/Windows-deps/x86_64/JS32ECMAv5/include and it still runs fine, so I will commit the change to master later. There could even be a performance boost in loading scripts etc. Anyway, thanks for this catch, can't believe we had it like this all these years...

[mcarans]

I have enabled the two macros in my local js-config inside oolite-deps/Windows-deps/x86_64/JS32ECMAv5/include and it still runs fine, so I will commit the change to master later. There could even be a performance boost in loading scripts etc. Anyway, thanks for this catch, can't believe we had it like this all these years...

Did you set JS_TRACER to 1?

When compiling using your working folders, I had to change in Makefile.ref from:

Code: Select all

DIST = /c/DevelopmentEnvironment/gcc/Msys_x2/1.0/DevlibsBuildFiles64/nspr4/dist

To:

Code: Select all

DIST = $(JS_DIST)

Afterwards, it compiled various files, then I got this error:

Code: Select all

jsapi.cpp: In function 'JSIdArray* JS_Enumerate(JSContext*, JSObject*)':
jsapi.cpp:3980:16: error: cannot convert 'bool' to 'JSIdArray*' in return
 3980 |         return false;
      |                ^~~~~
make[1]: *** [rules.mk:78: WINNT5.1_OPT.OBJ/jsapi.o] Error 1

Have you seen this before?

[another_commander]

Did you set JS_TRACER to 1?

Not exactly to 1 during building the library, but the macro does get defined. This is the command that gets executed during the build for all .cpp files (example from jsapi.cpp which we will talk about momentarily):

g++ -o WINNT5.1_OPT.OBJ/jsapi.o -c -DXP_WIN -DEXPORT_JS_API -DJS_C_STRINGS_ARE_UTF8 -D__STDC_LIMIT_MACROS -D_WINDOWS -DWINVER=0x500 -D_WIN32_WINNT=0x500 -UDEBUG -DNDEBUG -UDEBUG_ -DJS_TRACER -DFEATURE_NANOJIT -DAVMPLUS_64BIT -DAVMPLUS_AMD64 -DJS_CPU_X64 -DJS_PUNBOX64 -DENABLE_ASSEMBLER -DENABLE_YARR_JIT -DJS_METHODJIT=1 -DJS_MONOIC=1 -DJS_POLYIC=1 -DJS_POLYIC_TYPED_ARRAY=1 -DMOZ_TRACE_JSCALLS=1 -DJS_THREADSAFE=1 -DJS_HAS_NATIVE_COMPARE_AND_SWAP=1 -Inanojit -Itracejit -Iyarr -Iassembler -I. -IWINNT5.1_OPT.OBJ -I/d/DriveD/devlibs/nspr438/dist/include/nspr -O2 -Wno-invalid-offsetof jsapi.cpp

As you can see JS_TRACER is a defined macro.

Afterwards, it compiled various files, then I got this error:

Code: Select all

jsapi.cpp: In function 'JSIdArray* JS_Enumerate(JSContext*, JSObject*)':
jsapi.cpp:3980:16: error: cannot convert 'bool' to 'JSIdArray*' in return
 3980 |         return false;
      |                ^~~~~
make[1]: *** [rules.mk:78: WINNT5.1_OPT.OBJ/jsapi.o] Error 1

Have you seen this before?

No but I can see why this is an error. In fact, my compiler throws a warning about it and it is likely that this warning was promoted to error in later versions of gcc or you are building with warnings treated as errors.

jsapi.cpp: In function 'JSIdArray* JS_Enumerate(JSContext*, JSObject*)':
jsapi.cpp16: warning: converting 'false' to pointer type 'JSIdArray*' [-Wconversion-null]

What is happening here is that false is defined as 0, which in gcc 4.7.1 is also what NULL is defined as. So returning false essentially tries to return NULL, i.e. 0. New gcc hates that so it throws an error here (and I tend to agree, returning false when an array is expected is not exactly what I would call "good code"). Try changing false to NULL and it should pass.

[mcarans]

No but I can see why this is an error. In fact, my compiler throws a warning about it and it is likely that this warning was promoted to error in later versions of gcc or you are building with warnings treated as errors.

jsapi.cpp: In function 'JSIdArray* JS_Enumerate(JSContext*, JSObject*)':
jsapi.cpp16: warning: converting 'false' to pointer type 'JSIdArray*' [-Wconversion-null]

What is happening here is that false is defined as 0, which in gcc 4.7.1 is also what NULL is defined as. So returning false essentially tries to return NULL, i.e. 0. New gcc hates that so it throws an error here (and I tend to agree, returning false when an array is expected is not exactly what I would call "good code"). Try changing false to NULL and it should pass.

It's odd but when I diff the Oolite deps folder version it already has NULL so for some reason in your working folders it was changed to False. See: https://github.com/OoliteProject/spider ... .cpp#L3980

The same change from NULL to False has been made in jsfun.cpp, jsiter.cpp and elsewhere in your working folders.

Do you know why these changes were made in your working folders? Was it a compiler bug you came across or some other issue?

[another_commander]

Don't know, maybe my JS source snapshot is older than the one up on github? Back when I was doing this it was the days of Spidermonkey for FF4 going from beta to stable. I remember getting the source as soon as it went stable but maybe other bug fixes happened down the road and then the snapshot for github was created. Just a theory.

One thing is certain though: Those NULL-> false changes are definitely not something I did.

[mcarans]

Don't know, maybe my JS source snapshot is older than the one up on github? Back when I was doing this it was the days of Spidermonkey for FF4 going from beta to stable. I remember getting the source as soon as it went stable but maybe other bug fixes happened down the road and then the snapshot for github was created. Just a theory.

Ok, I'll assume then that the deps version is correct where I come across discrepancies.

[mcarans]

Some good news: I made as few changes as possible to the working folders. I built the js and ran the game. It seemed to work ok in release mode for my brief test of starting it and flying around.

Here you can see the libraries it uses:

Code: Select all

$ cygcheck ./oolite.exe
C:\oolite\oolite.app\oolite.exe
  C:\WINDOWS\system32\ADVAPI32.dll
    C:\WINDOWS\system32\msvcrt.dll
      C:\WINDOWS\system32\ntdll.dll
      C:\WINDOWS\system32\KERNELBASE.dll
    C:\WINDOWS\system32\SECHOST.dll
    C:\WINDOWS\system32\KERNEL32.dll
    C:\WINDOWS\system32\RPCRT4.dll
  C:\WINDOWS\system32\dwmapi.dll
    C:\WINDOWS\system32\win32u.dll
    C:\WINDOWS\system32\USER32.dll
      C:\WINDOWS\system32\GDI32.dll
  C:\msys64\mingw64\bin\libgcc_s_seh-1.dll
    C:\msys64\mingw64\bin\libwinpthread-1.dll
  C:\WINDOWS\system32\GLU32.dll
    C:\WINDOWS\system32\OPENGL32.dll
  C:\msys64\mingw64\bin\gnustep-base-1_31.dll
    C:\msys64\mingw64\bin\libffi-8.dll
    C:\msys64\mingw64\bin\libgnutls-30.dll
      C:\msys64\mingw64\bin\libbrotlidec.dll
        C:\msys64\mingw64\bin\libbrotlicommon.dll
      C:\msys64\mingw64\bin\libbrotlienc.dll
      C:\WINDOWS\system32\CRYPT32.dll
      C:\msys64\mingw64\bin\libgmp-10.dll
      C:\msys64\mingw64\bin\libhogweed-6.dll
        C:\msys64\mingw64\bin\libnettle-8.dll
      C:\msys64\mingw64\bin\libidn2-0.dll
        C:\msys64\mingw64\bin\libiconv-2.dll
        C:\msys64\mingw64\bin\libintl-8.dll
        C:\msys64\mingw64\bin\libunistring-5.dll
      C:\WINDOWS\system32\ncrypt.dll
      C:\msys64\mingw64\bin\libp11-kit-0.dll
        C:\WINDOWS\system32\SHELL32.dll
          C:\WINDOWS\system32\msvcp_win.dll
      C:\msys64\mingw64\bin\libtasn1-6.dll
      C:\WINDOWS\system32\WS2_32.dll
      C:\msys64\mingw64\bin\zlib1.dll
      C:\msys64\mingw64\bin\libzstd.dll
    C:\msys64\mingw64\bin\libicuin77.dll
      C:\msys64\mingw64\bin\libicuuc77.dll
        C:\msys64\mingw64\bin\libicudt77.dll
        C:\msys64\mingw64\bin\libstdc++-6.dll
    C:\WINDOWS\system32\NETAPI32.dll
    C:\msys64\mingw64\bin\libobjc-4.dll
    C:\msys64\mingw64\bin\libxml2-16.dll
      C:\WINDOWS\system32\bcrypt.dll
      C:\msys64\mingw64\bin\liblzma-5.dll
    C:\msys64\mingw64\bin\libxslt-1.dll
  C:\msys64\mingw64\bin\libopenal-1.dll
    C:\WINDOWS\system32\AVRT.dll
    C:\WINDOWS\system32\ole32.dll
      C:\WINDOWS\system32\combase.dll
    C:\WINDOWS\system32\WINMM.dll
  C:\msys64\mingw64\bin\libpng16-16.dll
  C:\msys64\mingw64\bin\SDL.dll
  C:\WINDOWS\system32\SHLWAPI.dll
  C:\msys64\mingw64\bin\libvorbisfile-3.dll
    C:\msys64\mingw64\bin\libvorbis-0.dll
      C:\msys64\mingw64\bin\libogg-0.dll
  C:\oolite\oolite.app\js32ECMAv5.dll
    C:\oolite\oolite.app\nspr4.dll

Note that this is without espeak which I disabled as I haven't built it. If MrFlibble has got the system espeak-ng working on Linux, maybe I can do the same in MSYS2.

[another_commander]

Nice. Would it be possible to package the executable plus all dlls that are not inside System32 into a zip and upload it somewhere? I'd be interested in examining it.