621464 - Inconsistency with E4X XML.replace

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

print(<x>AAA</x>.replace())

opt:   undefined         toXMLString: "<x>undefined</x>"
dbg:   AAAundefined      toXMLString: "<x>\n  AAA\n  undefined\n</x>"

The first [revision where dbg outputs AAAundefined] is:
changeset:   1d1fe1d1e626
user:        Luke Wagner
date:        Mon Dec 06 10:26:58 2010 -0800
summary:     Bug 609440, part 4 - make JSString::chars() fallible (r=waldo,dvander,igor,dwitte,njn)

(I guess this was the "fallibile-ize JSString use in ctypes" patch in bug 609440?)

I reduced this from a jsfunfuzz testcase Gary sent me.  The original testcase showed a fragile difference between interp and TM (rather than between opt and dbg).

Comment 1

[Andrew Drake [:adrake]]

This was introduced by the blob:

@@ -6333,17 +6364,25 @@ xml_replace(JSContext *cx, uintN argc, j
<snip>
-    if (argc == 0 || !js_IdValIsIndex(vp[2], &index)) {
+    bool haveIndex;
+    if (argc == 0) {
+        haveIndex = true;
+    } else {
+        if (!js_IdValIsIndex(cx, vp[2], &index, &haveIndex))
+            return JS_FALSE;
+    }
+
+    if (!haveIndex) {

Pre-patch, argc == 0 would take the branch. Post-patch, argc == 0 does not take the branch. No other changes are made beyond this point, and so the index value is read uninitialized later.

Comment 2

[Andrew Drake [:adrake]]

Attachment: Patch v0

Changes = true to = false to preserve pre-regression behavior.

Comment 3

[Andrew Drake [:adrake]]

Attachment: Patch v1

With regression test for test suite.

Comment 4

[Luke Wagner [:luke]]

Bah, thanks!  (And Merry Christmas)

Comment 5

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Nominating not really because this seems to be a definite blocker, but rather this helps fuzzers and a r+ patch is available.

Comment 6

[Luke Wagner [:luke]]

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

Comment 8

[Jan de Mooij [:jandem]]

http://hg.mozilla.org/mozilla-central/rev/e63f5adc4280