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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jono Xia]

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;
dump(undefObj.foo);

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.

Comment 1

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Attachment: HTML test case

FWIW, it only happens in function context -- I get the right result in global context.

Comment 2

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Any chance of finding a regression range here, Jono? *bats eyelashes*

Comment 3

[Jesse Ruderman]

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)

Comment 4

[Jesse Ruderman]

Before that patch, the error message was the expected "undefObj is undefined".

Comment 5

[Luke Wagner [:luke]]

Attachment: fix

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 (https://bugzilla.mozilla.org/attachment.cgi?id=460220&action=diff), which, I see now, wasn't necessary.

Comment 6

[Luke Wagner [:luke]]

Attachment: fix, test, comment

Oops, forgot the fixins.

Comment 7

[Jesse Ruderman]

> its usually what we want

It's

Comment 8

[Jesse Ruderman]

The comment makes me think this code could use additional tests, maybe even including a currently-failing test.

Comment 9

[Luke Wagner [:luke]]

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*.

Comment 10

[Luke Wagner [:luke]]

But, for example, one of the "contrived" cases I was referring to (which, incidentally, gives the wrong message before and after b16d966d3b6f) is

  Array.prototype.sort.call([1,2], {});

which prints

  test.js:1: TypeError: Array.prototype.sort.call([1, 2], {}) is not a function

Comment 11

[Brendan Eich [:brendan]]

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.

/be

Comment 12

[Brendan Eich [:brendan]]

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).

/be

Comment 13

[Luke Wagner [:luke]]

Thanks; the comments reflected the hour in which they were written ;-)
http://hg.mozilla.org/tracemonkey/rev/d395cc77e0aa

Comment 14

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/d395cc77e0aa