Closed Bug 604210 Opened 13 years ago Closed 13 years ago

Referencing property of undefined produces cryptic "(void 0) is undefined" message


(Core :: JavaScript Engine, defect)

Windows 7
Not set



Tracking Status
blocking2.0 --- -


(Reporter: jono, Assigned: luke)



(Keywords: regression, testcase, Whiteboard: fixed-in-tracemonkey)


(2 files, 1 obsolete file)

This is on Firefox 4.0b6 on Windows 7.

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6)
Gecko/20100101 Firefox/4.0b6

Steps to reproduce:

The simplest Javascript code I've been able to come up with to reproduce the message is as follows:

let undefObj = undefined;

This produces an error in the javascript error console saying:
(void 0) is undefined

Expected behavior:
A more informative error message such as "undefObj is undefined" would make debugging a lot easier.
Attached file HTML test case
FWIW, it only happens in function context -- I get the right result in global context.
Any chance of finding a regression range here, Jono? *bats eyelashes*
blocking2.0: --- → ?
The first bad revision is:
changeset:   b16d966d3b6f
user:        Luke Wagner
date:        Fri Jul 30 14:49:29 2010 -0700
summary:     Bug 579183 - loosen-up StackSegment invariants - part 1 - decompiler (r=brendan)
Before that patch, the error message was the expected "undefObj is undefined".
Attached patch fix (obsolete) — Splinter Review
The issue in the examples is that JSOP_GETLOCALPROP is reporting an error for a value that is above the simulated stack depth (the one pushed by JSOP_GETLOCALPROP before jumping into JSOP_GETPROP).  The search done by JSDVG_SEARCH_STACK starts at regs.sp and finds a match, but can't use the pc from pcstack because sp is above said simulated depth.  Before the fingered patch, this would fall into the case of "just decompile the current pc" (which gives pretty results).  After the patch, it causes us to fall back to the "just print the value" case (which gives (void 0)).  The attached patch just undoes the last part of (, which, I see now, wasn't necessary.
Assignee: general → lw
Attachment #483409 - Flags: review?(brendan)
Oops, forgot the fixins.
Attachment #483409 - Attachment is obsolete: true
Attachment #483410 - Flags: review?(brendan)
Attachment #483409 - Flags: review?(brendan)
> its usually what we want

The comment makes me think this code could use additional tests, maybe even including a currently-failing test.
I think it has long been understood that JSDVG_SEARCH_STACK was only a heuristic and by no means always gives the right error message.  For one, its searching for a match on the stack *by value*.
But, for example, one of the "contrived" cases I was referring to (which, incidentally, gives the wrong message before and after b16d966d3b6f) is[1,2], {});

which prints

  test.js:1: TypeError:[1, 2], {}) is not a function
Comment on attachment 483410 [details] [diff] [review]
fix, test, comment

Jesse nitted "its" and I will nit "anyways" -- "anyway". Maybe a comma would help, too. The last sentence starting with "If above" does run on.

Attachment #483410 - Flags: review?(brendan) → review+
JSDVG_SEARCH_STACK is cheese from old days. We should try to get rid of it but not with lame blame. We should propagate more precise spindex or whatever the right blame cursor is (it might be quite abstract, to cover properties too).

blocking2.0: ? → -
Thanks; the comments reflected the hour in which they were written ;-)
Whiteboard: fixed-in-tracemonkey
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.