355992 - "Assertion failure: strncmp(rval, js_function_str, 8) == 0 && rval[8] == ' '" with non-function setter

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

Split from bug 352455 comment 4.

js> function() { return { x setter: 3 }; }
Assertion failure: strncmp(rval, js_function_str, 8) == 0 && rval[8] == ' ', at
jsopcode.c:3191

Should probably be a type error or even a syntax error.

Comment 1

[Jesse Ruderman]

I just saw the conversation in #content that makes it sound like ({ x setter: 3 }) can't be an error, so I guess the decompiler should learn to decompile it as "({x setter: 3})" or throw "I can't decompile this".

Comment 2

[Brian Crowder]

Attachment: bail instead of assert()

Not sure if there is a better errort reporting mechanism here or not.  It seems uncommon for the Disassembler to throw exceptions or report errors, so I tried to follow that style by simply returning NULL.  This means that if you're disassembling something that has never been executed and doesn't have compile errors, but DOES have exhibit this particular runtime error, you'll get no feedback.

Comment 3

[Brian Crowder]

Yeah, or the decompiler could be smarter, as Jesse suggests.

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 241677 [details] [diff] [review]
bail instead of assert()

No silent failures.

Instead just use the strncmp test in an if-else that uses the old getter: 3 syntax if 3 instead of a function object is what ends up decompiled (NB: not disassembled) in rval.

/be

Comment 5

[Brian Crowder]

Attachment: use old getter/setter decompile for non-functions

This uses the old decompilation code for non-function cases, as recommended by Brendan.  (no silent failure, but error-reporting doesn't happen unless the non-function code is actually executed)

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 241753 [details] [diff] [review]
use old getter/setter decompile for non-functions

r=me, thanks.

/be

Comment 7

[:Gavin Sharp [email: [e-mail]]]

mozilla/js/src/jsopcode.c 	3.193

Comment 8

[Bob Clary [:bc] (inactive)]

RCS file: /cvsroot/mozilla/js/tests/js1_5/Regress/regress-355992.js,v
done
Checking in regress-355992.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-355992.js,v  <--  regress-355992.js
initial revision: 1.1
done

Comment 9

[Bob Clary [:bc] (inactive)]

Note that this stills show the assert in bug 356250 until it is fixed.

Comment 10

[Bob Clary [:bc] (inactive)]

verified fixed 1.9 20061014 windows/linux

Comment 11

[Brian Crowder]

Comment on attachment 241753 [details] [diff] [review]
use old getter/setter decompile for non-functions

Tested patch and applies cleanly for me on 1.8.1.1 branch.

Comment 12

[Jay Patel [:jay]]

Comment on attachment 241753 [details] [diff] [review]
use old getter/setter decompile for non-functions

Approved for 1.8.1 branch, a=jay for drivers.  Please land this asap, thanks!

Comment 13

[:Gavin Sharp [email: [e-mail]]]

mozilla/js/src/jsopcode.c 	3.89.2.66

Comment 14

[Bob Clary [:bc] (inactive)]

verified fixed 20061123 1.8.1.1 windows/linux/mac*, 1.9 windows/linux