Mozilla

This would appear to be an optimizer bug (or abuse-of-optimizer). Rev 3.105
of jsinterp.c collapsed js_GetArgument and js_GetLocalVariable to simply
do 'return JS_TRUE;'. Both are then used as function pointers in jsparse.c.
This seems to get mangled somehow.

With rev 3.105, at the crash in Variables, |currentGetter| and |currentSetter|
have the same value and both (according to the msvc debugger) point to 
js_GetLocalVariable. |fun| is null.

But when I stuck a printf into each of js_GetArgument and js_GetLocalVariable,
the crash did not happen, currentGetter and currentSetter had different values,
and fun (presumably) points to a valid JSFunction.

Someone file a bug with M$.  They're in violation of ISO C.

/be

Sorry to say, but rpott's patch doesn't quite fix the problem. (The insertion
of the '/Od /Og...' flags has to move below '$(CFLAGS)' since CFLAGS also adds
'/O1' in an optimized build). However, '-Gy' is set by OS_CFLAGS in
config/WIN32 and winds up as part of CFLAGS. The problem is that while setting
'/Od' will unset '/O1', it doesn't appear to unset '-Gy' when set
explicity. So, without a way to munge the CFLAGS (or set it privately for
mozilla/js), this won't work for nmake builds.

But, I think we can fix this by setting some linker flags, and that it is
possibly the right way to fix this. Here's what I (unfortunately only vaguely)
understand.

Enabling '/Gy' allows "the compiler to package individual functions in the
form of packaged functions (COMDATs)."  Linker option /OPT:REF will "exclude
unreferenced packaged functions" and will turn on /OPT:ICF. /OPT:ICF will
"perform identical COMDAT folding", and can be disabled with /OPT:NOICF.

So, the fix is add /OPT:REF /OPT:NOICF to the linker flags for optimized
builds on win32 (for both nmake and gmake builds). I'll attach a diff
comparing the /VERBOSE linker output for the current default build, and a
second build with those options enabled. You can see that the only difference
is the elimation of the ICF's, and that we were only saving 79 bytes anyways
(some of the savings ill-gotten as well). Note that js_GetArgument and 
js_GetLocalVariable were part of a group of five functions that were being
folded together (all of which just return true in the function body).

[What I don't understand is that in the current nmake build system, I don't
see us setting /OPT:REF anywherer, but it seems to be taking effect
nonetheless (based on a comparison of the /VERBOSE output from the linker from
a run with those explictly enabled, and a current default build).]

Maybe pschwartau can run the js regression tests in an optimized gmake (or
nmake) build with jsinterp.c reverted to rev 1.105 (the crashing rev) and see
if this patch produces any problems.

((p.s., the "Discarded" section, visible at the top of the diff, notes dead
code that is thrown out. Or, at least, if I add the function below to
jsinterp.c, I get these lines added in the /VERBOSE linker output:

  Discarded "`string'" (??_C@_0CB@NOEM@unique?5but?5uncalled?5function@) \
            from jsinterp.obj
  Discarded _js_JunkFunction from jsinterp.obj

+ JSBool
+ js_JunkFunction(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
+ {
+     printf("unique but uncalled function");
+     return JS_TRUE;
+ }

However, I'm not clear why, e.g., js_str_escape is discarded, but I didn't
follow all of the trail of defines, etc., to see if it is indeed not
ultimately used. But, to be clear, we were already discarding js_str_escape,
etc., even without this proposed linker change, so adding these options is
not changing that situation.))