If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

SpiderMonkey exception error messages are sometimes lies

RESOLVED WORKSFORME

Status

()

Core
JavaScript Engine
RESOLVED WORKSFORME
7 years ago
5 years ago

People

(Reporter: mossop, Assigned: brendan)

Tracking

(Blocks: 1 bug, {regression})

Trunk
x86
Mac OS X
regression
Points:
---

Firefox Tracking Flags

(blocking2.0 .x+)

Details

(Reporter)

Description

7 years ago
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 addon6@tests.mozilla.org in app-profile
*** LOG addons.xpi: Migrating staged install of addon7@tests.mozilla.org in app-profile
*** LOG addons.xpi: Processing install of addon6@tests.mozilla.org in app-profile
*** ERROR addons.xpi: Failed to install staged add-on addon6@tests.mozilla.org in app-profile: TypeError: aManifests[aLocation.name][stageDirEntry] is null
*** LOG addons.xpi: Processing install of addon7@tests.mozilla.org in app-profile
*** ERROR addons.xpi: Failed to install staged add-on addon7@tests.mozilla.org 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.

Updated

7 years ago
blocking2.0: --- → ?
(Reporter)

Comment 1

7 years ago
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.
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

7 years ago
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?
Marking blocking because it's a regression and memory safety may be involved. It seems on the edge, though.
blocking2.0: ? → final+

Updated

7 years ago
Assignee: general → pbiggar
(Assignee)

Comment 5

7 years ago
Anyone have a reduced testcase?

/be
Blocks: 622261
Whiteboard: softblocker

Comment 6

7 years ago
The first bad revision is:
changeset:   58030:15f8aa161de5
user:        Dave Townsend <dtownsend@oxymoronical.com>
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

7 years ago
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.
(Assignee)

Comment 8

7 years ago
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
If it's that old, it seems like a non-blocker.
blocking2.0: final+ → ?
(Assignee)

Comment 10

7 years ago
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
(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...
(Assignee)

Comment 12

7 years ago
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
(Assignee)

Updated

7 years ago
Assignee: pbiggar → brendan
(Assignee)

Comment 13

7 years ago
Thanks to Paul for reducing.

/be
Status: NEW → ASSIGNED
Looks like this doesn't meet the basic blocking criteria. But we'll call it 2.x because it does look important.
blocking2.0: ? → .x
Whiteboard: softblocker
(Assignee)

Comment 15

5 years ago
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
(Assignee)

Comment 16

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

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.