252047 - [W] UMC: Uninitialized memory copy in js_XDRScript {155 occurrences}

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[timeless]

[W] UMC: Uninitialized memory copy in js_XDRScript {155 occurrences}
        Copying 4 bytes from 0x04d7173c (4 bytes at 0x04d7173c uninitialized)
        Address 0x04d7173c is 124 bytes into a 138 byte block at 0x04d716c0
        Address 0x04d7173c points to a HeapAlloc'd block in heap 0x01590000
        Thread ID: 0x1760
        Error location
            js_XDRScript+0xda3   [r:\mozilla\js\src\jsscript.c:582 
ip=0x03cd616a]
                            script->main = script->code + prologLength;
                            memcpy(script->code + length, notes, osrcnotes * 
sizeof(jssrcnote));
                            JS_free(cx, (void *) notes);
                            notes = NULL;
                            if (ntrynotes) {
                                script->trynotes = (JSTryNote *)
                                                   ((jsword)(SCRIPT_NOTES
(script) + nsrcnotes) &
                                                    ~(jsword)
JSTRYNOTE_ALIGNMASK);
                            }
                        }
                    }
                
                    while (ntrynotes) {
                        JSTryNote *tn = &script->trynotes[--ntrynotes];
             =>         uint32 start = (uint32) tn->start,
                               catchLength = (uint32) tn->length,
                               catchStart = (uint32) tn->catchStart;
                
                        if (!JS_XDRUint32(xdr, &start) ||
                            !JS_XDRUint32(xdr, &catchLength) ||
                            !JS_XDRUint32(xdr, &catchStart)) {
                            goto error;
                        }
                        tn->start = (ptrdiff_t) start;
                        tn->length = (ptrdiff_t) catchLength;
                        tn->catchStart = (ptrdiff_t) catchStart;
                    }
                    return JS_TRUE;
                
            fun_xdrObject+0xb83  [r:\mozilla\js\src\jsfun.c:1218 ip=0x03c6e74f]
            js_XDRObject+0x54e   [r:\mozilla\js\src\jsobj.c:3777 ip=0x03ca1d46]
            JS_XDRValue+0x3af    [r:\mozilla\js\src\jsxdrapi.c:544 
ip=0x03ce5c35]
            XDRAtomListElement+0xf4 [r:\mozilla\js\src\jsscript.c:313 
ip=0x03cd37f2]
            XDRAtomMap+0x1f3     [r:\mozilla\js\src\jsscript.c:348 
ip=0x03cd3a9f]
            js_XDRScript+0x636   [r:\mozilla\js\src\jsscript.c:482 
ip=0x03cd59fd]
            JS_XDRScript+0x3d    [r:\mozilla\js\src\jsxdrapi.c:579 
ip=0x03ce5cda]
            nsXULPrototypeScript::Deserialize(nsIObjectInputStream 
*,nsIScriptContext *,nsIURI *,nsCOMArray<nsINodeInfo> const*)+0x201 
[r:\mozilla\content\xul\content\src\nsxulelement.cpp:4478 ip=0x04877110]
            nsXULPrototypeScript::DeserializeOutOfLine(nsIObjectInputStream 
*,nsIScriptContext *)+0x43a 
[r:\mozilla\content\xul\content\src\nsxulelement.cpp:4598 ip=0x04884f1f]
            nsXULPrototypeElement::Deserialize(nsIObjectInputStream 
*,nsIScriptContext *,nsIURI *,nsCOMArray<nsINodeInfo> const*)+0x784 
[r:\mozilla\content\xul\content\src\nsxulelement.cpp:4223 ip=0x048868e1]
            ???            [ip=0x00650068]
            nsXULPrototypeDocument::Read(nsIObjectInputStream *)+0x88f 
[r:\mozilla\content\xul\document\src\nsxulprototypedocument.cpp:433 
ip=0x047a19e7]
        Allocation location
            HeapAlloc+0xc        [C:\WINDOWS\System32\KERNEL32.dll 
ip=0x67e633c8]
            heap_alloc+0x4a      [f:\vs70builds\9466
\vc\crtbld\crt\src\malloc.c:211 ip=0x7c0010d3]
            nh_malloc+0x10       [C:\WINDOWS\System32\MSVCR70.dll:106 
ip=0x7c00107b]
            JS_malloc+0x4c       [r:\mozilla\js\src\jsapi.c:1464 ip=0x03c2b31f]
            js_NewScript+0x6b    [r:\mozilla\js\src\jsscript.c:1056 
ip=0x03cd4358]
            js_XDRScript+0x537   [r:\mozilla\js\src\jsscript.c:460 
ip=0x03cd58fe]
                            return JS_FALSE;
                        if (!JS_XDRUint32(xdr, &version))
                            return JS_FALSE;
                
                        /* To fuse allocations, we need srcnote and trynote 
counts early. */
                        if (magic >= JSXDR_MAGIC_SCRIPT_4) {
                            if (!JS_XDRUint32(xdr, &nsrcnotes))
                                return JS_FALSE;
                            if (!JS_XDRUint32(xdr, &ntrynotes))
                                return JS_FALSE;
                        }
                    }
                
                    if (xdr->mode == JSXDR_DECODE) {
             =>         script = js_NewScript(cx, length, nsrcnotes, ntrynotes);
                        if (!script)
                            return JS_FALSE;
                        if (magic >= JSXDR_MAGIC_SCRIPT_2) {
                            script->main += prologLength;
                            script->version = (JSVersion) (version & 0xffff);
                            script->numGlobalVars = (uint16) (version >> 16);
                
                            /* If we know nsrcnotes, we allocated space for 
notes in script. */
                            if (magic >= JSXDR_MAGIC_SCRIPT_4)
                                notes = SCRIPT_NOTES(script);
                        }
                        *scriptp = script;
                    }
                
            fun_xdrObject+0xb83  [r:\mozilla\js\src\jsfun.c:1218 ip=0x03c6e74f]
            js_XDRObject+0x54e   [r:\mozilla\js\src\jsobj.c:3777 ip=0x03ca1d46]
            JS_XDRValue+0x3af    [r:\mozilla\js\src\jsxdrapi.c:544 
ip=0x03ce5c35]
            XDRAtomListElement+0xf4 [r:\mozilla\js\src\jsscript.c:313 
ip=0x03cd37f2]
            XDRAtomMap+0x1f3     [r:\mozilla\js\src\jsscript.c:348 
ip=0x03cd3a9f]
            js_XDRScript+0x636   [r:\mozilla\js\src\jsscript.c:482 
ip=0x03cd59fd]
            JS_XDRScript+0x3d    [r:\mozilla\js\src\jsxdrapi.c:579 
ip=0x03ce5cda]
            nsXULPrototypeScript::Deserialize(nsIObjectInputStream 
*,nsIScriptContext *,nsIURI *,nsCOMArray<nsINodeInfo> const*)+0x201 
[r:\mozilla\content\xul\content\src\nsxulelement.cpp:4478 ip=0x04877110]
            nsXULPrototypeScript::DeserializeOutOfLine(nsIObjectInputStream 
*,nsIScriptContext *)+0x43a 
[r:\mozilla\content\xul\content\src\nsxulelement.cpp:4598 ip=0x04884f1f]
            nsXULPrototypeElement::Deserialize(nsIObjectInputStream 
*,nsIScriptContext *,nsIURI *,nsCOMArray<nsINodeInfo> const*)+0x784 
[r:\mozilla\content\xul\content\src\nsxulelement.cpp:4223 ip=0x048868e1]
            ???            [ip=0x00650068]
            nsXULPrototypeDocument::Read(nsIObjectInputStream *)+0x88f 
[r:\mozilla\content\xul\document\src\nsxulprototypedocument.cpp:433 
ip=0x047a19e7]
[W] UMC: Uninitialized memory copy in js_XDRScript {155 occurrences}
[W] UMC: Uninitialized memory copy in js_XDRScript {155 occurrences}
[W] UMC: Uninitialized memory copy in js_XDRScript {2 occurrences}
[W] UMC: Uninitialized memory copy in js_XDRScript {2 occurrences}
[W] UMC: Uninitialized memory copy in js_XDRScript {2 occurrences}
[W] UMC: Uninitialized memory copy in js_XDRScript {98 occurrences}
[W] UMC: Uninitialized memory copy in js_XDRScript {98 occurrences}
[W] UMC: Uninitialized memory copy in js_XDRScript {98 occurrences}
[W] UMC: Uninitialized memory copy in js_XDRScript {10 occurrences}
[W] UMC: Uninitialized memory copy in js_XDRScript {10 occurrences}
[W] UMC: Uninitialized memory copy in js_XDRScript {10 occurrences}
[W] UMC: Uninitialized memory copy in js_XDRScript {2 occurrences}
[W] UMC: Uninitialized memory copy in js_XDRScript {2 occurrences}
[W] UMC: Uninitialized memory copy in js_XDRScript {2 occurrences}

Analysis:

+	(JSTryNote*)(jssrcnote*)((JSScript*)scriptp)->code+((JSScript*)scriptp)-
>length	0x04f029f0 {start=0x04f4542d length=0x00000019 catchStart=0x00000003 }
	JSTryNote *
+	(*(*scriptp)).trynotes	0x04f02a60 {start=0xcdcdcdcd length=0xcdcdcdcd 
catchStart=0xcdcdcdcd }	JSTryNote *

There's a code block:
        if (magic < JSXDR_MAGIC_SCRIPT_4) {
...
            if (ntrynotes) {
                script->trynotes = (JSTryNote *)
                                   ((jsword)(SCRIPT_NOTES(script) + nsrcnotes) &
                                    ~(jsword)JSTRYNOTE_ALIGNMASK);
            }
        }
we fall into the missing else block, which is well, missing.

Comment 1

[Brendan Eich [:brendan]]

timeless: your analysis makes no sense.  First, because no old magic number
scripts are being deserialized nowadays.  Second, because trynotes needs to be
reset only if ntrynotes is not zero (otherwise, trynotes is null because the
entire *script is memset to 0 after it is allocated).

These UMCs are harmless, they arise out of the XDR pattern of writing one piece
of code to handle deserialization and serialization.  In the deserialization
case, the loads are overwritten by the JS_XDRUint32 calls, where the load
targets are passed as out parameters (in the serialization case, the loads are
not UMRs and they are passed as in parameters).

I'm inclined to WONTFIX.  Cc'ing shaver for his thoughts.

/be

Comment 2

[Mike Shaver (:shaver emeritus)]

I agree that this looks to be harmless; the data copied is not used, and is
overwritten in what seems to be a correct manner.  I might support a patch to
eliminate this, in order to reduce purify noise in aid of finding real bugs, but
the number of real purify-found bugs in Spidermonkey has been so similar to zero
for so long that it would need to be a wafer-thin patch indeed.

I'd back a WONTFIX here.

Comment 3

[Brendan Eich [:brendan]]

Attachment: patch to consider

Is there any point in #ifdef PURIFY or whatever, or does pfy instrument without
recompilation?	I'm showing my age here!

/be

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 154032 [details] [diff] [review]
patch to consider

shaver says this is wafer-thin.

/be

Comment 5

[timeless]

purify on windows instruments at runtime. so ifdef purify is absolutely useless 
to me. 99% of the time i run optimized non debug builds because purify itself 
makes my builds slow enough.

Comment 6

[Brendan Eich [:brendan]]

Checked in.

/be