Closed
Bug 621464
Opened 13 years ago
Closed 13 years ago
Inconsistency with E4X XML.replace
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: jruderman, Assigned: adrake)
References
Details
(Keywords: regression, testcase, Whiteboard: [softblocker][fixed-in-tracemonkey])
Attachments
(1 file, 1 obsolete file)
3.05 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
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).
Assignee | ||
Comment 1•13 years ago
|
||
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.
Assignee | ||
Comment 2•13 years ago
|
||
Changes = true to = false to preserve pre-regression behavior.
Assignee: general → adrake
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•13 years ago
|
||
With regression test for test suite.
Attachment #499790 -
Attachment is obsolete: true
Comment 4•13 years ago
|
||
Bah, thanks! (And Merry Christmas)
Assignee | ||
Updated•13 years ago
|
Attachment #499791 -
Flags: review?(lw)
Updated•13 years ago
|
Attachment #499791 -
Flags: review?(lw) → review+
Comment 5•13 years ago
|
||
Nominating not really because this seems to be a definite blocker, but rather this helps fuzzers and a r+ patch is available.
blocking2.0: --- → ?
Updated•13 years ago
|
blocking2.0: ? → betaN+
Whiteboard: softblocker
Comment 6•13 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/e63f5adc4280
Whiteboard: softblocker → [softblocker][fixed-in-tracemonkey]
Comment 8•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/e63f5adc4280
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•