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

[E4X] crash when attribute-op name collides with local var

VERIFIED FIXED

Status

()

Core
JavaScript Engine
--
major
VERIFIED FIXED
12 years ago
12 years ago

People

(Reporter: Axel Hecht, Assigned: shaver)

Tracking

({crash})

Trunk
x86
Linux
crash
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

12 years ago
Using the E4X attribute syntax, I get a crash.

function TOCParser(aElement) {
  var href = aElement.@href;
}

crashes on parsing, the same code with @href2 is fine. Sample js and stacktrace
coming up.

Just 
./run-mozilla.sh ./xpcshell -x -f /tmp/e4x-crash.js
does the trick for me, CVS build from yesterday.
(Reporter)

Comment 1

12 years ago
Created attachment 190008 [details]
js file that crashes, text/plain so that you can actually look.
(Reporter)

Comment 2

12 years ago
Created attachment 190009 [details]
stacktrace

Stack trace. The offending part is a 

default: JS_ASSERT(0);

in LookupArgOrVar, the 

getter == js_GetLocalVariable || getter == js_GetCallVariable

part.
Updating summary a bit; I'm working this now.
Summary: [E4X] aElement.@href crashes on parsing → [E4X] crash when attribute-op name collides with argument
Summary: [E4X] crash when attribute-op name collides with argument → [E4X] crash when attribute-op name collides with local var
(Reporter)

Comment 4

12 years ago
Worked around the first crasher successfully, now the following line in my code
bails at the same place:

var linkURI = gIOS.newURI(linkNode.@toc, null, this.mPluginURI);
Created attachment 190012 [details] [diff] [review]
early out from LookupArgOrVar for QNAMEPART

I'm not sure if we need to do JSOP_QNAMECONST here as well, I'm not yet E4Xy
enough to be confident in how to trigger its generation.  I'll look at that
more today, if Brendan can't just jump in and say!
Assignee: general → shaver
Status: NEW → ASSIGNED
Attachment #190012 - Flags: review?(brendan)
Comment on attachment 190012 [details] [diff] [review]
early out from LookupArgOrVar for QNAMEPART

Great, thanks!

/be
Attachment #190012 - Flags: review?(brendan)
Attachment #190012 - Flags: review+
Attachment #190012 - Flags: approval1.8b4+
Not to worry about JSOP_QNAMECONST -- it's pn_op only in a node whose pn_type is
TOK_DBLCOLON and pn_arity is PN_NAME.  See QualifiedSuffix in jsparse.c.

/be
Fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Flags: testcase?
Resolution: --- → FIXED
(Reporter)

Comment 9

12 years ago
verified, my CVS build eats dogfood nicely now.
Status: RESOLVED → VERIFIED

Comment 10

12 years ago
Checking in regress-301545.js;
/cvsroot/mozilla/js/tests/e4x/Expressions/regress-301545.js,v  <-- 
regress-301545.js
initial revision: 1.1

I couldn't get this to crash on Windows. I would appreciate a Linux friend to
pull by date and check that this testcase really does crash.
Flags: testcase? → testcase+
You need to log in before you can comment on or make changes to this bug.