Closed Bug 72773 Opened 24 years ago Closed 24 years ago

Crash in debug JS shell

Categories

(Core :: JavaScript Engine, defect, P1)

x86
Windows NT
defect

Tracking

()

VERIFIED FIXED
mozilla0.9

People

(Reporter: pschwartau, Assigned: brendan)

Details

(Keywords: crash, js1.5)

Attachments

(4 files)

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(?)
Attached file WinNT stack trace + some extra info (deleted) —
Keywords: crash
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
Status: NEW → ASSIGNED
Keywords: js1.5, mozilla0.9
Priority: -- → P1
Target Milestone: --- → mozilla0.9
Attached patch proposed fix (deleted) — Splinter Review
Attached patch proposed fix (deleted) — Splinter Review
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
Attached patch real proposed fix (deleted) — Splinter Review
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
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
I have filed bug 72884 for this issue, and will add test coverage -
Brendan, double click -> double submission is now filed as bug 72906.
r=rogerl
pollmann: thanks. shaver or jband, how about an sr=? /be
sr=jband I trust you ran the tests.
(i did on WinNT, 0 errors)
Fix checked in -- thanks all (yes, I run the fine tests, too). /be
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
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.

Attachment

General

Creator:
Created:
Updated:
Size: