Closed Bug 344711 Opened 18 years ago Closed 18 years ago

js engine crashes trying to report a syntax error, due to uninitialized field

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla1.8.1beta2

People

(Reporter: eric.promislow, Assigned: mrbkap)

References

Details

(Keywords: crash, verified1.8.0.7, verified1.8.1)

Attachments

(2 files, 1 obsolete file)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.4) Gecko/20060620 Firefox/1.5.0.4 Flock/0.7.1 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.4) Gecko/20060620 Firefox/1.5.0.4 Flock/0.7.1 % cat test.js var a1 = {abc2 : 1, abc3 : 3}; var j = a1\ .abc2; print(j); % xpcshell test.js Type Manifest File: D:\mozilla\mozilla\ffd\dist\bin\components\xpti.dat nsNativeComponentLoader: autoregistering begins. nsNativeComponentLoader: autoregistering succeeded nsNativeComponentLoader: registering deferred (0) test.js:2: SyntaxError: illegal character: test.js:2: .abc2; test.js:2: .............................................................. ................................................................................ [about 22000 dots printed] StackTrace: > xpcshell.exe!my_ErrorReporter(JSContext * cx=0x00c54448, const char * message=0x00cd4f10, JSErrorReport * report=0x00cd69e0) Line 156 + 0x9 C++ js3250.dll!js_ReportErrorAgain(JSContext * cx=0x00c54448, const char * message=0x00cd4e80, JSErrorReport * reportp=0x00cd69e0) Line 1034 + 0x15 C js3250.dll!js_ReportUncaughtException(JSContext * cx=0x00c54448) Line 1162 + 0x11 C js3250.dll!JS_CompileFileHandleForPrincipals(JSContext * cx=0x00c54448, JSObject * obj=0x00cb3fd0, const char * filename=0x0039711d, _iobuf * file=0x1027c898, JSPrincipals * principals=0x00ca886c) Line 3789 + 0x25 C xpcshell.exe!ProcessFile(JSContext * cx=0x00c54448, JSObject * obj=0x00cb3fd0, const char * filename=0x0039711d, _iobuf * file=0x1027c898) Line 584 + 0x1c C++ xpcshell.exe!Process(JSContext * cx=0x00c54448, JSObject * obj=0x00cb3fd0, const char * filename=0x0039711d) Line 681 + 0x15 C++ xpcshell.exe!ProcessArgs(JSContext * cx=0x00c54448, JSObject * obj=0x00cb3fd0, char * * argv=0x0039710c, int argc=1) Line 833 + 0x11 C++ xpcshell.exe!main(int argc=1, char * * argv=0x0039710c, char * * envp=0x00392f88) Line 1100 + 0x15 C++ xpcshell.exe!mainCRTStartup() Line 398 + 0x11 C kernel32.dll!7c816d4f() kernel32.dll!7c8399f3() The crash occurs in the third line in this code: n = report->tokenptr - report->linebuf; for (i = j = 0; i < n; i++) { if (report->linebuf[i] == '\t') { for (k = (j + 8) & ~7; j < k; j++) { fputc('.', gErrFile); } continue; } fputc('.', gErrFile); j++; } The problem is that report->tokenptr points to garbage (0x00ce4ad6). report->linebug points to 0x00cd4ad8 The code that tries to align the '^' under the bad token ends up triggering a fault in the IO mechanism. Reproducible: Always Steps to Reproduce: 1. Start up js shell (or xpcshell) 2. At prompt 1, type "a1 = {abc2 : 1, abc3 : 3};" 3. At prompt 2, type this (with newlines, without quotes): js> "j = a1\ .abc2;" Actual Results: Many dots and a crash. See above.
confirmed on 1.8.0 and trunk windows xp for (i = j = 0; i < n; i++) { => if (report->linebuf[i] == '\t') { for (k = (j + 8) & ~7; j < k; j++) { fputc('.', gErrFile); } + cx 0x00367b58 {links={...} interpLevel=0 stackLimit=810564 ...} JSContext * + message 0x003678c8 "SyntaxError: illegal character" const char * - report 0x00367628 {filename=0x00367658 "typein" lineno=2 linebuf=0x0036f300 ".abc2; " ...} JSErrorReport * + filename 0x00367658 "typein" const char * lineno 2 unsigned int + linebuf 0x0036f300 ".abc2; " const char * + tokenptr 0x0037f2fe <Bad Ptr> const char * + uclinebuf 0x003677f8 ".abc2; " const unsigned short * + uctokenptr 0x003877f4 <Bad Ptr> const unsigned short * flags 2 unsigned int errorNumber 144 unsigned int + ucmessage 0x00367810 "illegal character" const unsigned short * + messageArgs 0x00000000 const unsigned short * * j 3335 int + ctmp 0x00000000 <Bad Ptr> const char * k 3288 int + tmp 0x003678f0 "" char * + prefix 0x00367918 "typein:2: " char * i 3328 int n 65534 int > js.exe!my_ErrorReporter(JSContext * cx=0x00367b58, const char * message=0x003678c8, JSErrorReport * report=0x00367628) Line 2156 + 0x9 bytes C js32.dll!js_ReportErrorAgain(JSContext * cx=0x00367b58, const char * message=0x00367888, JSErrorReport * reportp=0x00367628) Line 1177 + 0x15 bytes C js32.dll!js_ReportUncaughtException(JSContext * cx=0x00367b58) Line 1160 + 0x11 bytes C js32.dll!JS_CompileUCScriptForPrincipals(JSContext * cx=0x00367b58, JSObject * obj=0x00369290, JSPrincipals * principals=0x00000000, const unsigned short * chars=0x0041d0b8, unsigned int length=19, const char * filename=0x0040801c, unsigned int lineno=2) Line 3904 + 0x36 bytes C js32.dll!JS_CompileUCScript(JSContext * cx=0x00367b58, JSObject * obj=0x00369290, const unsigned short * chars=0x0041d0b8, unsigned int length=19, const char * filename=0x0040801c, unsigned int lineno=2) Line 3871 + 0x1f bytes C js32.dll!JS_CompileScript(JSContext * cx=0x00367b58, JSObject * obj=0x00369290, const char * bytes=0x0013eed4, unsigned int length=19, const char * filename=0x0040801c, unsigned int lineno=2) Line 3840 + 0x1d bytes C js.exe!Process(JSContext * cx=0x00367b58, JSObject * obj=0x00369290, char * filename=0x00000000, int forceTTY=0) Line 260 + 0x2e bytes C js.exe!ProcessArgs(JSContext * cx=0x00367b58, JSObject * obj=0x00369290, char * * argv=0x00363eac, int argc=0) Line 480 + 0x15 bytes C js.exe!main(int argc=0, char * * argv=0x00363eac, char * * envp=0x00362e10) Line 2736 + 0x15 bytes C js.exe!__tmainCRTStartup() Line 586 + 0x17 bytes C kernel32.dll!_BaseProcessStart@4() + 0x23 bytes
Status: UNCONFIRMED → NEW
Ever confirmed: true
I'll look at this.
Assignee: general → mrbkap
Severity: minor → major
OS: Windows XP → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.8.1beta2
Status: NEW → ASSIGNED
Attached patch Potential patch (obsolete) (deleted) — Splinter Review
This patch fixes this bug by not allowing PeekChars to peek over a newline. The actual bug is that in NewToken, we assume that ts->ungetpos refers to a position on the current line, which, when we've peeked over a newline, isn't true.
Attachment #229308 - Flags: superreview?(shaver)
Attachment #229308 - Flags: review?(brendan)
Comment on attachment 229308 [details] [diff] [review] Potential patch The fix is right in concept. Thanks! Minusing just to get the code right. ;-) > static JSBool > PeekChars(JSTokenStream *ts, intN n, jschar *cp) > { > intN i, j; > int32 c; > >- for (i = 0; i < n; i++) { >+ c = '\0'; >+ for (i = 0; i < n && c != '\n'; i++) { > c = GetChar(ts); > if (c == EOF) > break; The test for c being a newline belongs in the same place as teh c == EOF test, not (inverted) in the loop condition, which requires a useless initialization before the loop and a guaranteed degenerate test on the first iteration. > cp[i] = (jschar)c; > } > for (j = i - 1; j >= 0; j--) > UngetChar(ts, cp[j]); > return i == n; Comment before the function that it can't be used to peek into or past end of line. /be
Attachment #229308 - Flags: review?(brendan) → review-
(In reply to comment #4) > The test for c being a newline belongs in the same place as teh c == EOF test, > not (inverted) in the loop condition, which requires a useless initialization > before the loop and a guaranteed degenerate test on the first iteration. This isn't quite true: we need to UngetChar the newline that we just read, which means that we must increment i and add the newline to cp instead of simply breaking out of the loop. I can avoid the initialization and useless test, however, so I'll attach a new patch.
Attached patch Updated patch (deleted) — Splinter Review
Attachment #229308 - Attachment is obsolete: true
Attachment #229328 - Flags: review?(brendan)
Attachment #229308 - Flags: superreview?(shaver)
Comment on attachment 229328 [details] [diff] [review] Updated patch Cool -- bonus points for two spaces after period in the comment :-P. /be
Attachment #229328 - Flags: review?(brendan) → review+
Attached patch Patch to check in (deleted) — Splinter Review
I'll check this in once the tree reopens.
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Checking in regress-344711-n.js; /cvsroot/mozilla/js/tests/js1_5/Regress/regress-344711-n.js,v <-- regress-344711-n.js initial revision: 1.1
Flags: in-testsuite+
Worth getting on branches. /be
Blocks: js1.7
Flags: blocking1.8.1?
Flags: blocking1.8.0.6?
Attachment #229505 - Flags: review+
Attachment #229505 - Flags: approval1.8.1?
Attachment #229505 - Flags: approval1.8.0.6?
Flags: blocking1.8.1? → blocking1.8.1+
Comment on attachment 229505 [details] [diff] [review] Patch to check in a=mconnor on behalf of drivers for checkin to the 1.8 branch
Attachment #229505 - Flags: approval1.8.1? → approval1.8.1+
Keywords: fixed1.8.1
verified fixed 1.8.1, 1.9 windows/mac(ppc|tel)/linux 20060728
Status: RESOLVED → VERIFIED
Comment on attachment 229505 [details] [diff] [review] Patch to check in approved for 1.8.0 branch, a=dveditz for drivers
Attachment #229505 - Flags: approval1.8.0.7? → approval1.8.0.7+
Flags: blocking1.8.0.7? → blocking1.8.0.7+
Fixed on the 1.8.0 branch. /be
Keywords: fixed1.8.0.7
verified fixed 1.8.0.7 20060811 windows/mac(ppc|tel)/linux
Keywords: crash
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: