The default bug view has changed. See this FAQ.

JS Correctness: "reference to undefined property" vs. "is not defined" errors with/without methodjit

RESOLVED FIXED in mozilla11

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: decoder, Assigned: bhackett)

Tracking

(Blocks: 1 bug, {testcase})

Trunk
mozilla11
x86_64
Linux
testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
Maybe bug 622271, but that bug has just been marked WFM.
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.
FWIW, d8 also outputs "ReferenceError: z is not defined"
Created attachment 579151 [details] [diff] [review]
patch

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 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+
https://hg.mozilla.org/integration/mozilla-inbound/rev/c77d88e54bb3
https://hg.mozilla.org/mozilla-central/rev/c77d88e54bb3
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
You need to log in before you can comment on or make changes to this bug.