453388 - TM: SpiderMonkey doesn't embed into C apps due to __dso_handle

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[(dormant account)]

Chat log for gal
17:36 < gal> where is dso coming from? I don't even see it with grep
17:37 < gal> h-189:src gal$ strings jstracer.o | grep "_dso"
17:37 < gal> __dso_handle
17:37 < gal> weird
17:37 < taras> it's some internal gcc thing
17:37 < taras> apparently including std headers like iostream causes it
17:38 < gal> do we include iostream?
17:38 < taras> not that i could tell
17:39 < taras> i'm don't have enough gcc/linker-foo to figure this out
17:40 < taras> the dso thing is a hook called on module unload
17:41 < taras> but why it appeared i don't know
17:41 < gal> it seems everyone is running into it and nobody knows what it is
17:41 < gal> I am not sure its C++ issue
17:41 < gal> might be a hook to run C++ destructors
17:41 < gal> on unload
17:41 < gal> or some crazyness
17:41 < gal> I am sure we can fix it
17:41 < gal> what does this break? embedding?
17:42 < taras> yeah
17:45 < jimb> taras: Actually, I think I ecan tell you what __dso_handle is...
17:45 < taras> jimb: can you tell me how to track down the cat that dragged it in?

17:45 < jimb> exit, and atexit, and things like that.
17:45 < jimb> Linking against the C library.
17:46 < gal> wow jstracer references atexit
17:46 < gal> I think I have an idea
17:46 < gal> it might be the global oracle object
17:46 < jimb> The story is, there are some parts of the C library that are *always* linked statically.
17:46 < gal> it has a destructor
17:46 < gal> that destructor might use atexit internally
17:46 < gal> I think I can fix that
17:46 < gal> assign the bug to me, and copy the chat log in there so I don't forget what I just rambled

Comment 1

[Andreas Gal :gal]

Attachment: Allocate oracle dynamically (leaks oracle on shutdown, might need a hook for that).

Comment 2

[Andreas Gal :gal]

taras, could you test this?

Comment 3

[(dormant account)]

Got rid of the dso symbol, now its:
undefined symbol: _Znwj

I've gotten this one when I went tried older revisions before.

Comment 4

[(dormant account)]

Also, _Znwj is "operator new" if that helps

Comment 5

[(dormant account)]

commenting out new Oracle() makes it link.

Comment 6

[Andreas Gal :gal]

Ok, working on it. New patch forthcoming.

Comment 7

[Andreas Gal :gal]

Attachment: Improved patch.

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 336597 [details] [diff] [review]
Improved patch.

Taras will test, I'm sure. Beats reviewing in this case!

My review drive-by: Use JS_ShutDown, that's what it is there for!

/be

Comment 9

[(dormant account)]

Comment on attachment 336597 [details] [diff] [review]
Improved patch.

runs well here

Comment 10

[Andreas Gal :gal]

Ok, we need the JS_ShutDown cleanup and then this can go into TM.

Comment 11

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/e2614011f194

Comment 12

[mdew]

c++ -o jsapi.o -c -I../../dist/include/system_wrappers -include ../../config/gcc_hidden.h -DAVMPLUS_IA32 -DAVMPLUS_LINUX -DLINUX -DFEATURE_NANOJIT -DJS_TRACER -DOSTYPE=\"Linux2.6\" -DOSARCH=Linux -DEXPORT_JS_API  -DJS_USE_SAFE_ARENA  -I. -I.  -I../../dist/include   -I../../dist/include/js -I../../dist/include/nspr     -I../../dist/sdk/include -I.    -fPIC   -fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-long-long -pedantic -fno-strict-aliasing -fshort-wchar -pthread -pipe  -DNDEBUG -DTRIMMED -Os -fstrict-aliasing -finline-limit=50   -DMOZILLA_CLIENT -include ../../mozilla-config.h -Wp,-MD,.deps/jsapi.pp jsapi.cpp
jsapi.cpp: In function ‘void JS_ShutDown()’:
jsapi.cpp:876: error: ‘js_ShutDownJIT’ was not declared in this scope
make[3]: *** [jsapi.o] Error 1
make[3]: Leaving directory `/home/mdew/ff/tracemonkey/js/src'
make[2]: *** [libs_tier_js] Error 2
make[2]: Leaving directory `/home/mdew/ff/tracemonkey'
make[1]: *** [tier_js] Error 2

Comment 13

[Andreas Gal :gal]

Thanks. Working on it.

Comment 14

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/fc4a8cc07c9f

Comment 15

[Andreas Gal :gal]

Please reopen if this still doesn't solve the problem.

Comment 16

[Benjamin Smedberg]

I think this is the wrong solution to the problem. If we link libjs.so with the C++-aware linker, it will pick up libstdc++ automatically and we can safely use static constructors/destructors (note that "GC" and "GCHeap" are statics also). This is manifest in bug 454624.

Comment 17

[Benjamin Smedberg]

Attachment: Link using g++, rev. 1

Not sure who should review this, but I think we want to do this and then back out the two previous patches. Can I get approval for that on the tracemonkey branch? FWIW this fixes bug 454624

Comment 18

[Brian Crowder]

Comment on attachment 337916 [details] [diff] [review]
Link using g++, rev. 1

This means embedders must link with g++ also?

Comment 19

[Benjamin Smedberg]

Embedders who link against libjs.so do not have to use g++... libstdc++ is in the library dependencies and the static initializers/destructors are run by _init and _fini set up in the shared library.

Embedders who link against libjs.a would have to use the C++ linker.

Comment 20

[David Anderson [:dvander] - inactive, e-mail if emergency]

If you're embedding into an app which you don't control and statically links to libstdc++ you could run into problems.  Though for those cases people could always change the Makefile back.

Comment 21

[Benjamin Smedberg]

Embedders who link statically against libstdc++ and dynamically against other libs are walking a tightrope anyway; they should probably use libjs.a

pushed link-with-g++ patch to actionmonkey: http://hg.mozilla.org/tracemonkey/rev/82c472459ec7
and pushed the backout of the prior patches: http://hg.mozilla.org/tracemonkey/rev/66a5b3e1c5bd