620449 - SpiderMonkey exception error messages are sometimes lies

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

[Dave Townsend [:mossop]]

When I print out an exception message sometimes SpiderMonkey seems to get variable names wrong. This has been going on for some time but I've only just come across a reproducible case I could files with.

Build a current trunk and do the following xpcshell test:

SOLO_FILE=test_migrate1.js make -C toolkit/mozapps/extensions/test check-one

It should pass but included in the start of the log should be the following lines:

*** LOG addons.manager: Application has been upgraded
*** LOG addons.xpi: startup
*** LOG addons.xpi: checkForChanges
*** LOG addons.xpi: Migrating staged install of [e-mail] in app-profile
*** LOG addons.xpi: Migrating staged install of [e-mail] in app-profile
*** LOG addons.xpi: Processing install of [e-mail] in app-profile
*** ERROR addons.xpi: Failed to install staged add-on [e-mail] in app-profile: TypeError: aManifests[aLocation.name][stageDirEntry] is null
*** LOG addons.xpi: Processing install of [e-mail] in app-profile
*** ERROR addons.xpi: Failed to install staged add-on [e-mail] in app-profile: TypeError: aManifests[aLocation.name][stageDirEntry] is null

There is just one problem, there is no reference to "aManifests[aLocation.name][stageDirEntry]" anywhere in the source code. I'm pretty sure the failing line is this: http://hg.mozilla.org/mozilla-central/annotate/e952221c3251/toolkit/mozapps/extensions/XPIProvider.jsm#l1747, we only ever reference "aManifests[aLocation.name][id]" so somehow JS is mistaking one variable name for another (I'm assuming it is only mixing up names and not values at this point).

This is pretty annoying as it makes error messages unreliable for diagnosing problems. I suspect it's a regression.

Comment 1

[Dave Townsend [:mossop]]

Note that I fixed the exception that was being logged in bug 612393 so to reproduce this you'll have to test on a tree older than that or temporarily back it out.

Comment 2

[Nicholas Nethercote [inactive]]

I suggest this shouldn't block 2.0, because it's not clear it's a regression, there's no crash or security problem, and it doesn't appear to be widespread.

Comment 3

[timeless]

njn: if the decompiler is picking a random string from the wrong bucket, why do you think that couldn't possibly be a security problem?

Comment 4

[David Mandelin [:dmandelin]]

Marking blocking because it's a regression and memory safety may be involved. It seems on the edge, though.

Comment 5

[Brendan Eich [:brendan]]

Anyone have a reduced testcase?

/be

Comment 6

[Paul Biggar]

The first bad revision is:
changeset:   58030:15f8aa161de5
user:        Dave Townsend <[e-mail]>
date:        Wed Nov 24 14:43:51 2010 -0800
summary:     Bug 412819: Allow upgrading an add-on to an add-on with a different ID. r=robstrong, a=blocks-betaN


Unfortunately, this is the revision with the symptomatic text, not the cause, so I need to try a different bisecting tactic.

Comment 7

[Paul Biggar]

Reduced testcase (fails with JS too, not just xpcshell):

a = {};
{
  let x = 0;
}
{
  let id = 0;
  a[id].prop = {};
}

Outputs:
TypeError: a[x] is undefined


Changing "a" to "{}" fixes it.
Removing either 'let' fixes it.
Combining the blocks fixes it.

Bisecting next.

Comment 8

[Brendan Eich [:brendan]]

This is an old bug I'm sure, it probably goes back to the dawn of "let" (JS1.7 in Firefox 2 -- you'd be bisecting into cvs.mozilla.org, ouch).

Paul, I can advise on fixing. Find me on IRC?

/be

Comment 9

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

If it's that old, it seems like a non-blocker.

Comment 10

[Brendan Eich [:brendan]]

Hm, not that old:

$ ./Darwin_DBG.OBJ/js
js> a = {};
[object Object]
js> {
  let x = 0;
}
js> {
  let id = 0;
  a[id].prop = {};
}
typein:7: TypeError: a[id] is undefined

This is with a Firefox-3-era js shell. Paul, keep bisecting :-/.

/be

Comment 11

[Shawn Wilsher :sdwilsh]

(In reply to comment #9)
> If it's that old, it seems like a non-blocker.
Although, it certainly has been making it hard to fix blockers when it comes up...

Comment 12

[Brendan Eich [:brendan]]

Duh, non-interactive differs from compilable-unit-at-a-time REPL:

$ ./Darwin_DBG.OBJ/js /tmp/bar.js
/tmp/bar.js:7: TypeError: a[x] is undefined

This one goes all the way back. Can't block hard on this one.

/be

Comment 13

[Brendan Eich [:brendan]]

Thanks to Paul for reducing.

/be

Comment 14

[David Mandelin [:dmandelin]]

Looks like this doesn't meet the basic blocking criteria. But we'll call it 2.x because it does look important.

Comment 15

[Brendan Eich [:brendan]]

Seems fixed now -- given a pbcopy of the test from comment 7:

$ ./Darwin_DBG.OBJ/js
js> a = {};
({})
js> {
  let x = 0;
}
js> {
  let id = 0;
  a[id].prop = {};
}
typein:7:2 TypeError: a[id] is undefined
js> 
$ pbpaste > /tmp/bar.js
$ ./Darwin_DBG.OBJ/js /tmp/bar.js
/tmp/bar.js:7:2 TypeError: a[id] is undefined

Anyone remember what might have fixed this?

/be

Comment 16

[Brendan Eich [:brendan]]

If someone can id the fix, feel free to cite it and change resolution to FIXED.

/be