Closed
Bug 706240
Opened 13 years ago
Closed 13 years ago
JS Correctness: "reference to undefined property" vs. "is not defined" errors with/without methodjit
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla11
People
(Reporter: decoder, Assigned: bhackett1024)
Details
(Keywords: testcase)
Attachments
(1 file)
2.69 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
The following test produces two different ReferenceError versions with options "-m -a" vs. no options on mozilla-central revision e320f9f5536f: function f(code) { a |= code.replace(/s/, ""); f = eval("(function(){" + code + "})") } f("switch(''){default:break;/*DUPTRY525*/}") Output: $ $JS -m -a min.js min.js:2: ReferenceError: a is not defined $ $JS min.js min.js:2: ReferenceError: reference to undefined property "a" I remember that we had a bug open about this at some point but I can't find it. It would be good to fix these inconsistencies to make differential fuzzing results more reliable.
Comment 1•13 years ago
|
||
Maybe bug 622271, but that bug has just been marked WFM.
Comment 2•13 years ago
|
||
You're right, js -a -m -n does indeed output "ReferenceError: z is not defined" in the bug 622271 testcase. I'll leave it up to you to decide whether to re-open that bug or just to deal with it here.
Comment 3•13 years ago
|
||
FWIW, d8 also outputs "ReferenceError: z is not defined"
Assignee | ||
Comment 4•13 years ago
|
||
The method JIT is doing the right thing here, this should get reported as an undefined name and not an undefined property. When executing a GETXPROP in the interpreter, the getProperty treats it as a property access when reporting the error (despite knowing it is at a GETXPROP opcode).
Assignee: general → bhackett1024
Attachment #579151 -
Flags: review?(jwalden+bmo)
Comment 5•13 years ago
|
||
Comment on attachment 579151 [details] [diff] [review] patch Review of attachment 579151 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsobj.cpp @@ +5941,1 @@ > } else { If the consequent block always completes by returning, the else-block doesn't need to be 1) indented or 2) an else-block. Remove the else, de-indent the block contents, and separate the two with a blank line. @@ +5966,5 @@ > + > + /* Ok, bad undefined property reference: whine about it. */ > + if (!js_ReportValueErrorFlags(cx, flags, JSMSG_UNDEFINED_PROP, > + JSDVG_IGNORE_STACK, IdToValue(id), > + NULL, NULL, NULL)) { { on new line, please.
Attachment #579151 -
Flags: review?(jwalden+bmo) → review+
Assignee | ||
Comment 6•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c77d88e54bb3
Comment 7•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c77d88e54bb3
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
You need to log in
before you can comment on or make changes to this bug.
Description
•