Closed Bug 632206 Opened 13 years ago Closed 13 years ago

xml_elements writes past the end of the argument vector

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
Tracking Status
blocking2.0 --- final+
blocking1.9.2 --- .20+
status1.9.2 --- .20-fixed
blocking1.9.1 --- -
status1.9.1 --- wontfix

People

(Reporter: gkw, Assigned: dmandelin)

References

Details

(Keywords: crash, regression, testcase, Whiteboard: [ccbr][sg:critical?][softblocker][fixed-in-tracemonkey])

Attachments

(2 files)

x = <x/>
for (a = 0; a < 9; a++) {
  x.elements()
}

crashes 64-bit js opt shell on TM changeset db8be4e3f373 with -j at js::ExecuteTree, but does not seem to crash on js debug shell.

s-s because this seems to be accessing a weird memory address at 0xfffb80010050e680

=====

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: 13 at address: 0x0000000000000000
0x000000010019c25b in js::ExecuteTree ()
(gdb) bt
#0  0x000000010019c25b in js::ExecuteTree ()
#1  0x00000001001be04f in js::RecordLoopEdge ()
#2  0x00000001001be45c in js::MonitorLoopEdge ()
#3  0x0000000100094d39 in js::Interpret ()
#4  0x00000001000998f8 in js::Execute ()
#5  0x0000000100016d57 in JS_ExecuteScript ()
#6  0x0000000100005020 in Process ()
#7  0x0000000100009295 in Shell ()
#8  0x00000001000097da in main ()
(gdb) x/i $pc
0x10019c25b <_ZN2jsL11ExecuteTreeEP9JSContextPNS_12TraceMonitorEPNS_12TreeFragmentERjPPNS_10VMSideExitES9_+859>:        incl   0x13c(%rbx)
(gdb) x/b $rbx
0xfffb80010050e680:     Cannot access memory at address 0xfffb80010050e680

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   53579:802d34381fe4
user:        David Mandelin
date:        Fri Sep 03 15:12:38 2010 -0700
summary:     Bug 593497: blacklist using iteration count only if methodjit is enabled, r=dvander
Assignee: general → nnethercote
(In reply to comment #0)
> 
> autoBisect shows this is probably related to the following changeset:
> 
> The first bad revision is:
> changeset:   53579:802d34381fe4
> user:        David Mandelin
> date:        Fri Sep 03 15:12:38 2010 -0700
> summary:     Bug 593497: blacklist using iteration count only if methodjit is
> enabled, r=dvander

There's a good chance that's not the real cause, but that it just exposed the latent bug.  I'll take a look in the next day or two.
Attached patch PatchSplinter Review
I snuck a look at this one while waiting for ubuntu to install. Turns out it's not a tracer bug--rather, xml_elements() writes to vp[2] even when it only takes 1 arg. In this particular test case, that overwrites the save area for $rbx, which holds TreeFragment in ExecuteTree, thus we crash when we come back and try to access it. I think the store is just for rooting and thus is now unnecessary.
Assignee: nnethercote → dmandelin
Status: NEW → ASSIGNED
Attachment #510480 - Flags: review?(lw)
blocking2.0: ? → final+
Whiteboard: [ccbr][sg:critical?] → [ccbr][sg:critical?][softblocker]
(In reply to comment #2)
> xml_elements() writes to vp[2] even when it only
> takes 1 arg. 

s/1 arg/0 args
Attachment #510480 - Flags: review?(lw) → review+
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
http://hg.mozilla.org/tracemonkey/rev/3b23b4d5ed13
Whiteboard: [ccbr][sg:critical?][softblocker] → [ccbr][sg:critical?][softblocker][fixed-in-tracemonkey]
The patch doesn't look like it's 64-bit only. Does this affect all versions, and the 64-bit-only just happens to be due to the testcase?
blocking1.9.1: ? → needed
blocking1.9.2: ? → needed
(In reply to comment #5)
> The patch doesn't look like it's 64-bit only. Does this affect all versions,
> and the 64-bit-only just happens to be due to the testcase?

Correct. The bug affects all versions, but it happens to cause a crash (in test cases uncovered so far) only on 64-bit with the tracer.
OS: Mac OS X → All
Hardware: x86 → All
Summary: TM: 64-bit Crash [@ js::ExecuteTree] → xml_elements writes past the end of the argument vector
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
dmandelin: does this patch work for the 1.9.2 branch?
blocking1.9.1: needed → -
blocking1.9.2: needed → .20+
Attached patch Patch for 1.9.2Splinter Review
Attachment #545787 - Flags: review?(luke)
Attachment #545787 - Flags: approval1.9.2.20?
Attachment #545787 - Flags: review?(luke) → review+
Comment on attachment 545787 [details] [diff] [review]
Patch for 1.9.2

a=LegNeato for releases/mozilla-1.9.2
Attachment #545787 - Flags: approval1.9.2.20? → approval1.9.2.20+
Group: core-security
JSBugMon: This bug has been automatically verified fixed.
Status: RESOLVED → VERIFIED
A testcase for this bug was automatically identified at js/src/jit-test/tests/e4x/bug632206.js.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: