549531 - fix strict aliasing warnings about scopeChain (also clean trailing whitespace)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Brendan Eich [:brendan]]

Attachment: fix

I hope I'm not the only one who greps warnings from shell build logs :-P.

/be

Comment 1

[Brendan Eich [:brendan]]

Hrm, I should have checked this first:

$ grep 'scopeChain->' *.h *.cpp
jsfun.cpp:        JS_ASSERT(fp->scopeChain->getPrivate() != fp);
jsfun.cpp:    JS_ASSERT(scopeChain->hasClass(&js_CallClass));
jsfun.cpp:    JS_ASSERT(!scopeChain->getPrivate());
jsinterp.cpp:                  fp->scopeChain->getPrivate() != fp);
jstracer.cpp:                    if (!fp->scopeChain->getPrivate())
jstracer.cpp:                        fp->scopeChain->setPrivate(fp);

Only six hits, while seven address-of uses of scopeChainVal in this bug's patch:

$ grep scopeChainVal *.h *.cpp
jsinterp.h:        jsval       scopeChainVal;
jsrecursion.cpp:              &fp->scopeChainVal,
jstracer.cpp:        if (!visitor.visitStackSlots(&fp->scopeChainVal, 1, fp))
jstracer.cpp:           ? get(&cx->fp->scopeChainVal)
jstracer.cpp:        LIns* scopeChain_ins = get(&cx->fp->scopeChainVal);
jstracer.cpp:        set(&fp->scopeChainVal, INS_NULL(), true);
jstracer.cpp:        set(&fp->scopeChainVal, call_ins);
jstracer.cpp:        set(&fp->scopeChainVal, scopeChain_ins, true);

says we should just make scopeChain be of type jsval and use JSVAL_TO_OBJECT in six places. dmandelin, what do you think?

/be

Comment 2

[Brendan Eich [:brendan]]

$ grep '>scopeChain)' *.cpp *.h
jsemit.cpp:        JS_ASSERT(caller->fun && caller->varobj(cx) == evalcg->scopeChain);
jsfun.cpp:    JSClass *classp = OBJ_GET_CLASS(cx, fp->scopeChain);
jsfun.cpp:                                                   fp->scopeChain);
jsfun.cpp:    callobj = NewCallObject(cx, fp->fun, fp->scopeChain);
jsgc.cpp:    if (fp->scopeChain)
jsinterp.cpp:        JS_ASSERT(fp->scopeChain);
jsinterp.cpp:        JS_ASSERT(OBJ_GET_CLASS(cx, fp->scopeChain) != &js_BlockClass ||
jsinterp.cpp:    STOBJ_SET_PARENT(newChild, fp->scopeChain);
jsobj.cpp:        if (fp->scopeChain)
jsobj.cpp:            fprintf(stderr, "  scopeChain: (JSObject *) %p\n", (void *) fp->scopeChain);
jsops.cpp:    regs.sp[-1] = OBJECT_TO_JSVAL(fp->scopeChain);
jsops.cpp:    JS_ASSERT(regs.sp[-1] == OBJECT_TO_JSVAL(fp->scopeChain));
jsops.cpp:        obj = CloneFunctionObject(cx, fun, fp->scopeChain);
jsscript.cpp:    JS_ASSERT(!caller || cx->fp->scopeChain == caller->scopeChain);
jstracer.cpp:    HashAccum(h, uintptr_t(OBJ_SHAPE(JS_GetGlobalForObject(cx, fp->scopeChain))), ORACLE_MASK);
jstracer.cpp:    VisitGlobalSlots(visitor, cx, JS_GetGlobalForObject(cx, cx->fp->scopeChain),
jstracer.cpp:    VisitSlots(visitor, cx, JS_GetGlobalForObject(cx, cx->fp->scopeChain),
jstracer.cpp:    VisitSlots(visitor, cx, JS_GetGlobalForObject(cx, cx->fp->scopeChain),
jstracer.cpp:    JS_ASSERT(globalObj == JS_GetGlobalForObject(cx, cx->fp->scopeChain));
jstracer.cpp:    JS_ASSERT(cx->fp->scopeChain);
jstracer.cpp:    if (OBJ_SCOPE(JS_GetGlobalForObject(cx, cx->fp->scopeChain))->title.ownercx != cx) {
jstracer.cpp:    JSObject* globalObj = JS_GetGlobalForObject(cx, cx->fp->scopeChain);
jstracer.cpp:    JS_ASSERT(f->globalObj == JS_GetGlobalForObject(cx, cx->fp->scopeChain));
jstracer.cpp:    JSObject* globalObj = JS_GetGlobalForObject(cx, cx->fp->scopeChain);
jstracer.cpp:    JS_ASSERT(chainHead == cx->fp->scopeChain);
jstracer.cpp:        JSObject* globalObj = JS_GetGlobalForObject(cx, cx->fp->scopeChain);

This is big enough I'll take that r+ and run with it. The union assumes that (JSObject *)v and JSVAL_TO_OBJECT(v) are equivalent for the bits stored in v, but we have

jsapi.cpp:JS_STATIC_ASSERT(JSVAL_OBJECT == 0);

guarding this assumption.

/be

Comment 3

[Brendan Eich [:brendan]]

http://hg.mozilla.org/tracemonkey/rev/64c278790bb8

/be

Comment 4

[Brendan Eich [:brendan]]

http://hg.mozilla.org/mozilla-central/rev/64c278790bb8

/be