"this.u.v = 1" results in "(void 0) is undefined" instead of "this.u is undefined"

VERIFIED FIXED in mozilla1.9

Status

()

Core
JavaScript Engine
P2
minor
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: Jesse Ruderman, Assigned: brendan)

Tracking

(Blocks: 1 bug, {regression, testcase})

Trunk
mozilla1.9
regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 -
wanted1.9 +
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
js> this.u.v = 1
typein:1: TypeError: (void 0) is undefined

Expected: TypeError: this.u is undefined
Duplicate of this bug: 430674
(Assignee)

Comment 2

10 years ago
Regression from Firefox 2 / JS1.7:

$ ../srcmoz18/Darwin_DBG.OBJ/js
js> this.u.v = 1
typein:1: TypeError: this.u has no properties
js> 
$ ./Darwin_DBG.OBJ/js
js> this.u.v = 1
typein:1: TypeError: (void 0) is undefined
js> 

Regression from patch for bug 363536.

/be
Assignee: general → brendan
Blocks: 363536, 428420
Keywords: regression
OS: Mac OS X → All
Priority: -- → P2
Hardware: PC → All
Target Milestone: --- → mozilla1.9
(Assignee)

Updated

10 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 3

10 years ago
Created attachment 317567 [details] [diff] [review]
forgot JOF_VARPROP for JSOP_GETTHISPROP

See comments in jsopcode.c (the changes to that file are to comments only).

/be
Attachment #317567 - Flags: review?(igor)

Comment 4

10 years ago
I think this might be worth blocking on -- the degraded quality of the error message seems likely to cause confusion.  Also, we have a testcase and a (hopefully soon-to-be-reviewed) patch, and this is a regression.
Flags: blocking1.9?
We wouldn't block on it, but we'd take it.  Chop chop! :)
Flags: wanted1.9+
Flags: blocking1.9?
Flags: blocking1.9-
(Assignee)

Comment 6

10 years ago
Note that this is a one-line (12 char ;-) fix to jsopcode.tbl. The other two file patches contain comment-only changes.

The only explicit use of JOF_VARPROP is in Decompile where the comments changed in jsopcode.c.

The only effect of changing JSOP_GETTHISPROP's mode from 0 to JOF_VARPROP is to bypass the js_GetSrcNote-must-not-return-null requirement in DecompileExpression (under js_DecompileValueGenerator), since JSOP_GETTHISPROP depends on no "base" expression that was compiled into left-context bytecode that would need a source note whose offset gives the displacement of that base from the faulting op. This opcode stands alone and can start a left-context for later "srcnote-based" ops.

So this truly is a low-risk change, and I believe it is a complete fix to the regression. But I crave Igor's review.

/be

Updated

10 years ago
Attachment #317567 - Flags: review?(igor) → review+
(Assignee)

Updated

10 years ago
Attachment #317567 - Flags: approval1.9?
Comment on attachment 317567 [details] [diff] [review]
forgot JOF_VARPROP for JSOP_GETTHISPROP

a1.9+=damons
Attachment #317567 - Flags: approval1.9? → approval1.9+
(Assignee)

Comment 8

10 years ago
Fixed:

js/src/jsopcode.c 3.312
js/src/jsopcode.h 3.66
js/src/jsopcode.tbl 3.115

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED

Comment 9

10 years ago
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-420919.js,v  <--  regress-420919.js
initial revision: 1.1
Flags: in-testsuite+
Flags: in-litmus-
(Assignee)

Updated

10 years ago
Depends on: 431248

Comment 10

10 years ago
v 1.9.0
Status: RESOLVED → VERIFIED

Updated

10 years ago
No longer blocks: 428420

Updated

10 years ago
Blocks: 380236

Updated

10 years ago
No longer blocks: 380236

Updated

10 years ago
Blocks: 428420
You need to log in before you can comment on or make changes to this bug.