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)
Core
JavaScript Engine
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)
(deleted),
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
brendan
:
review+
dveditz
:
approval1.8.0.7+
mconnor
:
approval1.8.1+
|
Details | Diff | Splinter Review |
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.
Comment 1•18 years ago
|
||
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
Assignee | ||
Comment 2•18 years ago
|
||
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
Assignee | ||
Updated•18 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•18 years ago
|
||
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 4•18 years ago
|
||
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-
Assignee | ||
Comment 5•18 years ago
|
||
(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.
Assignee | ||
Comment 6•18 years ago
|
||
Attachment #229308 -
Attachment is obsolete: true
Attachment #229328 -
Flags: review?(brendan)
Attachment #229308 -
Flags: superreview?(shaver)
Comment 7•18 years ago
|
||
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+
Assignee | ||
Comment 8•18 years ago
|
||
I'll check this in once the tree reopens.
Assignee | ||
Comment 9•18 years ago
|
||
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Comment 10•18 years ago
|
||
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+
Comment 11•18 years ago
|
||
Worth getting on branches.
/be
Updated•18 years ago
|
Attachment #229505 -
Flags: review+
Attachment #229505 -
Flags: approval1.8.1?
Attachment #229505 -
Flags: approval1.8.0.6?
Updated•18 years ago
|
Flags: blocking1.8.1? → blocking1.8.1+
Comment 12•18 years ago
|
||
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+
Assignee | ||
Updated•18 years ago
|
Keywords: fixed1.8.1
Comment 13•18 years ago
|
||
verified fixed 1.8.1, 1.9 windows/mac(ppc|tel)/linux 20060728
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1 → verified1.8.1
Comment 14•18 years ago
|
||
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+
Updated•18 years ago
|
Flags: blocking1.8.0.7? → blocking1.8.0.7+
Comment 16•18 years ago
|
||
verified fixed 1.8.0.7 20060811 windows/mac(ppc|tel)/linux
Keywords: fixed1.8.0.7 → verified1.8.0.7
You need to log in
before you can comment on or make changes to this bug.
Description
•