"Assertion failure: cursor == (uint8 *)copy->messageArgs[0] + argsCopySize, at jsexn.c" or "Assertion failure: cursor == (uint8 *)copy->messageArgs[0] + argsCopySize, at jsexn.cpp"

VERIFIED FIXED

Status

()

Core
JavaScript Engine
--
critical
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: gkw, Assigned: mrbkap)

Tracking

(Blocks: 1 bug, 5 keywords)

Trunk
x86
Linux
assertion, fixed1.8.1.21, testcase, verified1.9.0.7, verified1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +
blocking1.9.0.7 +
wanted1.9.0.x +
blocking1.8.1.next +
wanted1.8.1.x +
blocking1.8.0.next +
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?] fixed-in-tracemonkey)

Attachments

(6 attachments, 2 obsolete attachments)

(Reporter)

Description

9 years ago
Created attachment 357094 [details]
linux gdb backtrace

function f() { eval("(function() { switch(x, x) { default: for(x2; <x><y/></x>;) (function(){}) <x><y/></x>;break; case (+<><x><y/></x></>): break;   }; })()"); }
gczeal(2);
f();

asserts debug js trunk shell (with and without -j) and 1.9.0.x debug js shell at Assertion failure: cursor == (uint8 *)copy->messageArgs[0] + argsCopySize, at ../jsexn.cpp:188

Compiling 1.9.0.x opt with gczeal seems to work as expected. Security-sensitive because it involves gczeal.

Thanks Waldo for guiding me along this one.
Flags: blocking1.9.1?
Flags: blocking1.9.0.7?
(Reporter)

Updated

9 years ago
Summary: "Assertion failure: cursor == (uint8 *)copy->messageArgs[0] + argsCopySize, at jsexn.c" → "Assertion failure: cursor == (uint8 *)copy->messageArgs[0] + argsCopySize, at jsexn.c" or "Assertion failure: cursor == (uint8 *)copy->messageArgs[0] + argsCopySize, at jsexn.cpp"
(Assignee)

Comment 1

9 years ago
Created attachment 357275 [details] [diff] [review]
Fix

Easy fix. I gotta say, I like the ability to use stack-based helper classes in js/src now.
Assignee: general → mrbkap
Status: NEW → ASSIGNED
Attachment #357275 - Flags: review?(jwalden+bmo)
Attachment #357275 - Flags: review?(jwalden+bmo) → review+
(Assignee)

Comment 2

9 years ago
http://hg.mozilla.org/tracemonkey/rev/d77738e770cd
Whiteboard: fixed-in-tracemonkey
(Reporter)

Comment 3

9 years ago
Created attachment 357288 [details] [diff] [review]
patch backport

Here's the 1.9.0.x patch backport.
Attachment #357288 - Flags: review?(mrbkap)
Attachment #357288 - Flags: approval1.9.0.7?
(Reporter)

Comment 4

9 years ago
Created attachment 357289 [details]
diff between trunk and 1.9.0.x patch

To ensure nothing creepy sets in. :)
(Reporter)

Comment 5

9 years ago
Comment on attachment 357288 [details] [diff] [review]
patch backport

Oops, something wonky in test compiles.

===

cc -o Darwin_DBG.OBJ/jsregexp.o -c -Wall -Wno-format -MMD -g3 -DXP_UNIX -DSVR4 -DSYSV -D_BSD_SOURCE -DPOSIX_SOURCE -DDARWIN -DX86_LINUX  -DDEBUG -DDEBUG_skywalker -DEDITLINE -IDarwin_DBG.OBJ  jsregexp.c
jsregexp.c: In function ‘js_NewRegExpObject’:
jsregexp.c:4310: error: ‘JSAutoTempValueRooter’ undeclared (first use in this function)
jsregexp.c:4310: error: (Each undeclared identifier is reported only once
jsregexp.c:4310: error: for each function it appears in.)
jsregexp.c:4310: error: syntax error before ‘tvr’
make[1]: *** [Darwin_DBG.OBJ/jsregexp.o] Error 1
make: *** [all] Error 2
Attachment #357288 - Attachment is obsolete: true
Attachment #357288 - Flags: review?(mrbkap)
Attachment #357288 - Flags: approval1.9.0.7?
(Reporter)

Comment 6

9 years ago
jorendorff on IRC helped me out by telling me that mrbkap's patch "was changing from a C style of doing things to a C++ style of doing things" and suggested the following patch:

Index: jsregexp.c
===================================================================
RCS file: /cvsroot/mozilla/js/src/jsregexp.c,v
retrieving revision 3.200
diff -u -8 -p -r3.200 jsregexp.c
--- jsregexp.c	11 Aug 2008 18:24:13 -0000	3.200
+++ jsregexp.c	16 Jan 2009 03:05:35 -0000
@@ -4303,20 +4303,20 @@ js_NewRegExpObject(JSContext *cx, JSToke
     JSString *str;
     JSObject *obj;
     JSRegExp *re;
     JSTempValueRooter tvr;
 
     str = js_NewStringCopyN(cx, chars, length);
     if (!str)
         return NULL;
+    JS_PUSH_TEMP_ROOT_STRING(cx, str, &tvr);
     re = js_NewRegExp(cx, ts,  str, flags, JS_FALSE);
     if (!re)
         return NULL;
-    JS_PUSH_TEMP_ROOT_STRING(cx, str, &tvr);
     obj = js_NewObject(cx, &js_RegExpClass, NULL, NULL, 0);
     if (!obj || !JS_SetPrivate(cx, obj, re)) {
         js_DestroyRegExp(cx, re);
         obj = NULL;
     }
     if (obj && !js_SetLastIndex(cx, obj, 0))
         obj = NULL;
     JS_POP_TEMP_ROOT(cx, &tvr);


However this causes the following assertion: Assertion failure: (cx)->tempValueRooters == (&pc->tempRoot), at jsparse.c:186 and a backtrace is:

(gdb) bt
#0  JS_Assert (s=0x10939c "(cx)->tempValueRooters == (&pc->tempRoot)", file=0x109340 "jsparse.c", ln=186) at jsutil.c:63
#1  0x000a5111 in js_FinishParseContext (cx=0x2005b0, pc=0xbfffdaa4) at jsparse.c:186
#2  0x000a61f8 in js_CompileScript (cx=0x2005b0, obj=0x17d100, principals=0x0, tcflags=2048, chars=0x202430, length=137, file=0x0, filename=0x201d81 "typein", lineno=1) at jsparse.c:671
#3  0x0008706f in obj_eval (cx=0x2005b0, obj=0x17d000, argc=1, argv=0x80609c, rval=0xbfffdcfc) at jsobj.c:1326
#4  0x0007b7ab in js_Invoke (cx=0x2005b0, argc=1, vp=0x806094, flags=2) at jsinterp.c:1304
#5  0x0006e6ca in js_Interpret (cx=0x2005b0) at jsinterp.c:4864
#6  0x0007c099 in js_Execute (cx=0x2005b0, chain=0x17d000, script=0x2023d0, down=0x0, flags=0, result=0xbffff628) at jsinterp.c:1546
#7  0x0001c910 in JS_ExecuteScript (cx=0x2005b0, obj=0x17d000, script=0x2023d0, rval=0xbffff628) at jsapi.c:4895
#8  0x00002c8b in Process (cx=0x2005b0, obj=0x17d000, filename=0x0, forceTTY=0) at js.c:310
#9  0x000035d8 in ProcessArgs (cx=0x2005b0, obj=0x17d000, argv=0xbffff760, argc=0) at js.c:556
#10 0x00009025 in main (argc=0, argv=0xbffff760, envp=0xbffff764) at js.c:3931
(In reply to comment #6)
> jorendorff on IRC helped me out by telling me that mrbkap's patch "was changing
> from a C style of doing things to a C++ style of doing things" and suggested
> the following patch:
> 
> Index: jsregexp.c
> ===================================================================
> RCS file: /cvsroot/mozilla/js/src/jsregexp.c,v
> retrieving revision 3.200
> diff -u -8 -p -r3.200 jsregexp.c
> --- jsregexp.c    11 Aug 2008 18:24:13 -0000    3.200
> +++ jsregexp.c    16 Jan 2009 03:05:35 -0000
> @@ -4303,20 +4303,20 @@ js_NewRegExpObject(JSContext *cx, JSToke
>      JSString *str;
>      JSObject *obj;
>      JSRegExp *re;
>      JSTempValueRooter tvr;
> 
>      str = js_NewStringCopyN(cx, chars, length);
>      if (!str)
>          return NULL;
> +    JS_PUSH_TEMP_ROOT_STRING(cx, str, &tvr);
>      re = js_NewRegExp(cx, ts,  str, flags, JS_FALSE);
>      if (!re)
>          return NULL;
> -    JS_PUSH_TEMP_ROOT_STRING(cx, str, &tvr);

Don't return without popping the tvr.

/be
(Reporter)

Comment 8

9 years ago
Created attachment 357302 [details] [diff] [review]
working 1.9.0 patch

Thanks brendan and mrbkap, this is the next iteration of the patch, untested yet though.
Attachment #357289 - Attachment is obsolete: true
Attachment #357302 - Flags: review?(mrbkap)
Attachment #357302 - Flags: approval1.9.0.7?
(Assignee)

Updated

9 years ago
Attachment #357302 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 9

9 years ago
Comment on attachment 357302 [details] [diff] [review]
working 1.9.0 patch

Yep.
(Assignee)

Comment 10

9 years ago
I have tested that Gary's patch works in my 1.9.0 tree.
Whiteboard: fixed-in-tracemonkey → fixed-in-tracemonkey [needs 1.9.1 landing, baking]

Updated

9 years ago
Flags: blocking1.9.1? → blocking1.9.1+
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.7?
Flags: blocking1.9.0.7+
Whiteboard: fixed-in-tracemonkey [needs 1.9.1 landing, baking] → [sg:critical?] fixed-in-tracemonkey [needs 1.9.1 landing, baking]

Comment 11

9 years ago
http://hg.mozilla.org/mozilla-central/rev/d77738e770cd
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 12

9 years ago
Created attachment 357953 [details]
e4x/Regress/regress-473709.js

Updated

9 years ago
Flags: in-testsuite+
Flags: in-litmus-
Attachment #357302 - Flags: approval1.9.0.7? → approval1.9.0.7+
Comment on attachment 357302 [details] [diff] [review]
working 1.9.0 patch

Approved for 1.9.0.7, a=dveditz for release-drivers.
Flags: blocking1.8.1.next?
(Assignee)

Comment 14

9 years ago
Checked into the 1.9.0 branch.
Keywords: fixed1.9.0.7
Whiteboard: [sg:critical?] fixed-in-tracemonkey [needs 1.9.1 landing, baking] → [sg:critical?] fixed-in-tracemonkey [needs 1.9.1 landing]

Comment 15

9 years ago
regressed by bug 443039
Blocks: 443039
(Assignee)

Comment 16

9 years ago
Bob, was that comment really meant for this bug?

Comment 17

9 years ago
mrbkap: yes. i just bisected it on x86_64 and thought i would comment it here.
qawanted: bc or gary, do we want or need this on the 1.8.1 branch?
Keywords: qawanted
(Assignee)

Comment 19

9 years ago
Dan, yes we do.
Blake, can you work up a 1.8.1 patch?
Flags: blocking1.8.1.next? → blocking1.8.1.next+
Keywords: qawanted
(Assignee)

Comment 21

9 years ago
Created attachment 360952 [details] [diff] [review]
Fix for the 1.8.1 branch

Comment 22

9 years ago
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c0779e4abe0a
Keywords: fixed1.9.1
Whiteboard: [sg:critical?] fixed-in-tracemonkey [needs 1.9.1 landing] → [sg:critical?] fixed-in-tracemonkey

Comment 23

9 years ago
v 1.9.0, 1.9.1, 1.9.2
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.0.7, fixed1.9.1 → verified1.9.0.7, verified1.9.1
(Assignee)

Updated

9 years ago
Attachment #360952 - Flags: approval1.8.1.next?
Flags: wanted1.8.1.x+

Comment 24

9 years ago
Created attachment 363094 [details] [diff] [review]
1.8.0 one, based on the 1.8.1
Comment on attachment 360952 [details] [diff] [review]
Fix for the 1.8.1 branch

Approved for 1.8.1.21, a=dveditz for release-drivers.
Attachment #360952 - Flags: approval1.8.1.next? → approval1.8.1.next+
(Assignee)

Comment 26

9 years ago
Checked into MOZILLA_1_8_BRANCH.
Keywords: fixed1.8.1.21
Group: core-security

Updated

9 years ago
Attachment #363094 - Flags: approval1.8.0.next+

Comment 27

9 years ago
Comment on attachment 363094 [details] [diff] [review]
1.8.0 one, based on the 1.8.1

a=asac for 1.8.0

Updated

9 years ago
Flags: blocking1.8.0.next+

Comment 28

9 years ago
http://hg.mozilla.org/tracemonkey/rev/547fc4916d3e
/cvsroot/mozilla/js/tests/e4x/Regress/regress-473709.js,v  <--  regress-473709.js
initial revision: 1.1
(Reporter)

Updated

9 years ago
Duplicate of this bug: 381547
(Reporter)

Updated

9 years ago
Attachment #357302 - Attachment description: untested patch → working 1.9.0 patch
You need to log in before you can comment on or make changes to this bug.