Closed
Bug 72773
Opened 24 years ago
Closed 24 years ago
Crash in debug JS shell
Categories
(Core :: JavaScript Engine, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla0.9
People
(Reporter: pschwartau, Assigned: brendan)
Details
(Keywords: crash, js1.5)
Attachments
(4 files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
Using standalone JS shells built 2001-03-20 on WinNT. Consider this:
function Cow(name){this.name=name;}
function Calf(str){this.name=str;}
Calf.prototype= Cow;
new Calf().toString();
I get this error in the optimized shell -
TypeError: Function.prototype.toString called on incompatible object
But I CRASH in the debug shell. Either one of the following modifications,
however, gives me the warning instead of the crash:
1. Cow(str){this.name=str;} instead of Cow(name){this.name=name;}
2. new Calf.toString() instead of new Calf().toString()
And either of the following modifications gives no crash or warning:
3. Calf.prototype = Cow(); instead of Calf.prototype = Cow;
4. Calf.prototype = new Cow(); instead of Calf.prototype = Cow;
The latter two make sense to me: they make Calf.prototype an Object object
instead of a Function object, so Function.prototype.toString is not involved(?)
Reporter | ||
Comment 1•24 years ago
|
||
Assignee | ||
Comment 2•24 years ago
|
||
There are a bunch of related bugs. Here's another one:
function Cow(){return arguments}
function Calf(){print(this.length)}
Calf.prototype = Cow();
new Calf.toString();
The general problem is that many getters and setters in the engine, written long
ago, fail to check that their obj parameter is of the right class for them to
safely get the right type of private data structure from JSSLOT_PRIVATE. Patch
coming up.
ECMA dodges this incompatible class problem by not specifying inheritance of
[[Get]] and [[Put]]. SpiderMonkey does inherit getters and setters in order to
avoid creating direct properties in numerous objects, e.g. length in each and
every arguments object (instead, Arguments.prototype.length is shared among all
instances, conserving space).
The original example runs afoul of SpiderMonkey's extension to define formal
parameter names as properties of their function object (Cow.name is inherited by
Calf instances, so setting this.name = str in Calf invokes the setter for name,
js_SetArgument, which fails to check for compatible 'this'/'obj' class).
/be
Assignee: rogerl → brendan
Assignee | ||
Updated•24 years ago
|
Status: NEW → ASSIGNED
Keywords: js1.5,
mozilla0.9
Priority: -- → P1
Target Milestone: --- → mozilla0.9
Assignee | ||
Comment 3•24 years ago
|
||
Assignee | ||
Comment 4•24 years ago
|
||
Assignee | ||
Comment 5•24 years ago
|
||
What, I double-clicked on Submit in the bugzilla attachments page again?
Pollmann, I'm gonna bug you unless you say something fast. Submit must become
insensitive after the first click.
Anyway, ignore those patches. I'm reattaching a better one.
/be
Assignee | ||
Comment 6•24 years ago
|
||
Assignee | ||
Comment 7•24 years ago
|
||
Checkin comments:
- [jsemit.c] Fix horrid stupid bugs generating JSOP_ARGCNT and JSOP_ARGSUB,
where any occurrence of arguments.length or arguments[0], e.g., would be
"optimized" to use those bytecodes. This is just wrong if the occurrence
is an operand of delete, ++, --, or the left-hand-side of an assignment
operator!
- [jsfun.c, jsinterp.c] args_getProperty etc. must use JS_GetInstancePrivate,
not JS_GetPrivate, as the arguments object is exposed, and can be made a
prototype of other objects that do not have private data, or private data
that's a JSStackFrame*. Same goes for fun_getProperty, js_GetArgument, etc.
- [jsfun.c, jsobj.c, jsstr.c] No need to specialize fun_delProperty and
str_delProperty to help convince users and ECMA conformance tests that
fun.length and str.length are not direct properties of instances, but are
instead delegated to Function.prototype.length and String.prototype.length.
This special case is done universally in js_DeleteProperty for all SHARED
and PERMANENT proto-properties.
- [jshash.c] Sneaking this followup-fix for bug 69271 in: use JS_HASH_BITS
rather than hardcoded 32.
- [jsobj.c, jsscope.[ch]] Fix misnamed js_HashValue (it takes a jsid, so it
is now js_HashId).
- [jsscript.c] script_compile needs to call JS_InstanceOf, to ensure that obj
is a Script object.
r/sr= soon, please!
/be
Assignee | ||
Comment 8•24 years ago
|
||
I'm mortified by the [jsemit.c] bug. It makes
print((function (){return ++arguments[0])})(42));
print 42, not 43. Code buddies, save me!
There's still an ECMA deviation here: our Arguments class does not match 10.1.8,
which says arguments objects have as their prototype the original value of the
Object.prototype property. What's more, we delegate properties such as length,
0, 1, etc. to Arguments.prototype. So in SpiderMonkey,
function f(){delete arguments.length; return arguments.length}
print(f(1,2,3));
prints 3, rather than undefined as it should per ECMA. To fix this, I'd like
another bug. Phil, can you pitch one at me (and update the testsuite to cover
all the horrid bugs here)? Thanks.
/be
Reporter | ||
Comment 9•24 years ago
|
||
I have filed bug 72884 for this issue, and will add test coverage -
Comment 10•24 years ago
|
||
Brendan, double click -> double submission is now filed as bug 72906.
Comment 11•24 years ago
|
||
r=rogerl
Assignee | ||
Comment 12•24 years ago
|
||
pollmann: thanks.
shaver or jband, how about an sr=?
/be
Comment 13•24 years ago
|
||
sr=jband I trust you ran the tests.
Comment 14•24 years ago
|
||
(i did on WinNT, 0 errors)
Assignee | ||
Comment 15•24 years ago
|
||
Fix checked in -- thanks all (yes, I run the fine tests, too).
/be
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 16•23 years ago
|
||
Testcase added to JS testsuite:
js/tests/ecma_3/Object/regress-72773.js
Testcase passes on WinNT,Linux in debug/optimized JS shells;
marking bug VERIFIED -
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•