Closed Bug 355992 Opened 13 years ago Closed 13 years ago

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

Categories

(Core :: JavaScript Engine, defect, critical)

1.8 Branch
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla1.8.1

People

(Reporter: jruderman, Assigned: crowderbt)

References

(Blocks 1 open bug)

Details

(Keywords: crash, testcase, verified1.8.1.1)

Attachments

(1 file, 1 obsolete file)

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.
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".
Attached patch bail instead of assert() (obsolete) — Splinter Review
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.
Assignee: general → crowder
Status: NEW → ASSIGNED
Attachment #241677 - Flags: review?(brendan)
Yeah, or the decompiler could be smarter, as Jesse suggests.
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
Attachment #241677 - Flags: review?(brendan) → review-
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)
Attachment #241677 - Attachment is obsolete: true
Attachment #241753 - Flags: superreview?(brendan)
Comment on attachment 241753 [details] [diff] [review]
use old getter/setter decompile for non-functions

r=me, thanks.

/be
Attachment #241753 - Flags: superreview?(brendan) → review+
Blocks: js1.7src
Flags: blocking1.8.1.1?
mozilla/js/src/jsopcode.c 	3.193
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
OS: Mac OS X 10.4 → All
Hardware: Macintosh → All
Resolution: --- → FIXED
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
Flags: in-testsuite+
Note that this stills show the assert in bug 356250 until it is fixed.
verified fixed 1.9 20061014 windows/linux
Status: RESOLVED → VERIFIED
Flags: blocking1.8.1.1? → blocking1.8.1.1+
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.
Attachment #241753 - Flags: approval1.8.1.1?
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!
Attachment #241753 - Flags: approval1.8.1.1? → approval1.8.1.1+
mozilla/js/src/jsopcode.c 	3.89.2.66
Keywords: fixed1.8.1.1
Target Milestone: --- → mozilla1.8.1
Version: Trunk → 1.8 Branch
verified fixed 20061123 1.8.1.1 windows/linux/mac*, 1.9 windows/linux
No longer blocks: 349611
Blocks: 349611
You need to log in before you can comment on or make changes to this bug.