Comment 95 for bug 908508

Revision history for this message
In , Landry-openbsd (landry-openbsd) wrote :

(In reply to David Mandelin from comment #82)
> Landry, following up on a hint from billm, I got your patch to work on Linux
> with this addition:
>
> diff -r e7bbcbb6c24a js/src/jscntxt.h
> --- a/js/src/jscntxt.h Tue Mar 13 17:42:33 2012 -0700
> +++ b/js/src/jscntxt.h Tue Mar 13 18:32:46 2012 -0700
> @@ -64,6 +64,7 @@
> #include "js/HashTable.h"
> #include "js/Vector.h"
> #include "vm/Stack.h"
> +#include "assembler/jit/ExecutableAllocator.h"
>
> #ifdef _MSC_VER
> #pragma warning(push)
>
>
> I'm not quite sure what's going on, but it kind of seems like
> ENABLE_ASSEMBLER=1 is not being set for all the files in the build, and so
> different object files get different definitions of JSRuntime, which causes
> the trc field to get overwritten with junk, and then you crash. Your patch
> does modify how ENABLE_ASSEMBLER is set, so it seems vaguely plausible that
> could be the problem.

Aha. Very interesting... a shroedingbug.

> The strange thing is that it seems like js.cpp is the file that doesn't get
> ENABLE_ASSEMBLER=1, but I do see it being set on the command line to build
> js.o. And it also doesn't make much sense that the patch above would solve
> the problem.

If i look at old build logs on amd64, i don't see ENABLE_ASSEMBLER=1 in the js.o build
line, maybe because it's set in js/src/Makefile.in and not in js/src/shell/Makefile.in ?

I'll recheck with a clean tip and only that patch.
I've also pushed the corresponding patchset to https://tbpl.mozilla.org/?tree=Try&rev=88122a478851

For me that change make sense, since that brings ExecutableAllocator definition
to jscntxt.h where as of now, it's used without knowing how it's defined.

>
> Bug 731110 might just make things easy: if you don't have to make execAlloc_
> conditional on ENABLE_ASSEMBLER (because you'll have the assembler), then
> you won't have to modify jscntxt.h and the Makefile.in, and it should just
> work.

Yes, as cameron points out it's also needed for other archs (sparc64, broken anyway,
but i try to keep it in a compilable state with that patch, ia64, see #729447, and maybe others..)

Unfortunately i'm not in a position to write the ppc assembler for sysV ABI.