437081 - Assert upon executing delete '\u0000'.match('');

Status: RESOLVED DUPLICATE of bug 452329

(Core :: JavaScript Engine, defect)

Description

[Robert Swiecki]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9b5) Gecko/2008050509 Firefox/3.0b5
Build Identifier: Current Spidermoneky from CVS

$ gdb ./Linux_All_DBG.OBJ/js
GNU gdb 6.8-debian
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i486-linux-gnu"...
(gdb) r
Starting program: /home/test/src/mozilla/js/src/Linux_All_DBG.OBJ/js 
js> delete '\u0000'.match('');
Assertion failure: *fp->regs->pc == JSOP_CALL || *fp->regs->pc == JSOP_NEW, at jsstr.c:1221

Program received signal SIGTRAP, Trace/breakpoint trap.
JS_Assert (s=0x815233c "*fp->regs->pc == JSOP_CALL || *fp->regs->pc == JSOP_NEW", file=0x8151fa0 "jsstr.c", ln=1221) at jsutil.c:63
63	    abort();
(gdb) bt
#0  JS_Assert (s=0x815233c "*fp->regs->pc == JSOP_CALL || *fp->regs->pc == JSOP_NEW", file=0x8151fa0 "jsstr.c", ln=1221) at jsutil.c:63
#1  0x080fd30f in match_or_replace (cx=0x8177a20, glob=0x80fd421 <match_glob>, destroy=0, data=0xffffbe50, argc=1, vp=0x8181018) at jsstr.c:1220
#2  0x080fd5fb in str_match (cx=0x8177a20, argc=1, vp=0x8181018) at jsstr.c:1293
#3  0x080a79c1 in js_Invoke (cx=0x8177a20, argc=1, vp=0x8181018, flags=0) at jsinterp.c:1196
#4  0x0812fa8f in js_Interpret (cx=0x8177a20) at jsinterp.c:4899
#5  0x080a85a7 in js_Execute (cx=0x8177a20, chain=0x817b000, script=0x817fc28, down=0x0, flags=2, result=0xffffc76c) at jsinterp.c:1536
#6  0x0806627b in JS_ExecuteScript (cx=0x8177a20, obj=0x817b000, script=0x817fc28, rval=0xffffc76c) at jsapi.c:4895
#7  0x0804bd35 in Process (cx=0x8177a20, obj=0x817b000, filename=0x0, forceTTY=0) at js.c:310
#8  0x0804c503 in ProcessArgs (cx=0x8177a20, obj=0x817b000, argv=0xffffd8e8, argc=0) at js.c:556
#9  0x08052192 in main (argc=0, argv=0xffffd8e8, envp=0xffffd8ec) at js.c:3931


Reproducible: Always




$ ./Linux_All_DBG.OBJ/js -v
JavaScript-C 1.8.0 pre-release 1 2007-10-03

Comment 1

[Jesse Ruderman]

Confirmed on trunk.  Here's a simpler testcase:

delete ''.match('')

Is there something special about |match|?  Should I be fuzzing it and other built-in string functions?

Comment 2

[Brian Crowder]

The assertion here was introduced in bug 421274.  I'm not sure what the difference between JSOP_CALL and JSOP_SETCALL (the op of the pc we're at, at the point of the assertion), though it -seems- to be something to do with the delete here.  Igor?

Jesse:  You should probably be fuzzing over as many built-ins as you can.  :)

Comment 3

[Igor Bukanov]

The code assumes that it is called in a call context to skip the call bytecode and check what comes after it to optimize for boolean context. As this bug demonstartes, JSOP_SETCALL can also be the bytecode that invokes the match method. But just extending the assert to include JSOP_SETCALL is not enough since the match method can also be called through the JSOP_EVAL bytecode:

~ $ cat ~/s/x.js
function f()
{
    return eval("");
}

dis(f);
eval = String.prototype.match;
f.call("");
~ $ ~/m/ff/mozilla/js/src/Linux_All_DBG.OBJ/js -f ~/s/x.js
flags: HEAVYWEIGHT INTERPRETED
main:
00000:  callname "eval"
00003:  string ""
00006:  eval 1
00009:  lineno 3
00012:  return
00013:  stop

Source notes:
  0:     0 [   0] newline 
  1:     6 [   6] pcbase   offset 6
Assertion failure: *fp->regs->pc == JSOP_CALL || *fp->regs->pc == JSOP_NEW, at jsstr.c:1221
trace/breakpoint trap

Thus the assert should include JSOP_EVAL as well. At which point one would want to replace it with assertion that js-CodeSpec[op].flags & JOF_EVAL. But that would not work since JOF_EVAL is missing from JSOP_SETCALL in jsopcode.tbl. So that has to be fixed as well.

Comment 4

[Jesse Ruderman]

I added a list of a dozen builtins to jsfunfuzz, and it managed to find this bug within an hour.  It generated a slightly different testcase:

this.__defineSetter__('t',  '' .match);
t = 1;

Similar code without a setter can trigger the assertion too:

''.match() = 1;

Comment 5

[Brendan Eich [:brendan]]

(In reply to comment #3)
> Thus the assert should include JSOP_EVAL as well. At which point one would want
> to replace it with assertion that js-CodeSpec[op].flags & JOF_EVAL. But that
> would not work since JOF_EVAL is missing from JSOP_SETCALL in jsopcode.tbl. So
> that has to be fixed as well.

Did you mean JOF_INVOKE? But that would not handle the setter case.

/be

Comment 6

[Igor Bukanov]

(In reply to comment #5)
> (In reply to comment #3)
> > Thus the assert should include JSOP_EVAL as well. At which point one would want
> > to replace it with assertion that js-CodeSpec[op].flags & JOF_EVAL. But that
> > would not work since JOF_EVAL is missing from JSOP_SETCALL in jsopcode.tbl. So
> > that has to be fixed as well.
> 
> Did you mean JOF_INVOKE? But that would not handle the setter case.

Yes, I meant JOF_INVOKE and the idea was to add JOF_INVOKE to JOF_SETCALL.

Comment 7

[Robert Sayre]

I don't see this assert anymore with any of these tests. Anyone else?

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

I hope not! It seems like there might be a small optimization win by adding JSOP_SETCALL and JSOP_EVAL handling to the (now) if statement, but JSOP_SETCALL is relatively rare and people replacing eval with String.prototype.match don't deserve optimizations. I'll dupe to the bug that fixed the assertion.

Comment 9

[Christian Holler (:decoder)]

A testcase for this bug was already added in the original bug (bug 452329).