IonMonkey: Differential Testing: is undefined vs. undefined has no properties

RESOLVED FIXED in mozilla23

Status

()

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

People

(Reporter: decoder, Assigned: evilpie)

Tracking

(Blocks: 2 bugs, {testcase})

Trunk
mozilla23
x86_64
Linux
testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fuzzblocker])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
The following testcase shows different behavior with no options vs. --no-ti on mozilla-central revision 20d1a5916ef6:


var a = [];
for (var i = 0; i < 1000; i++) {
  a.x = {}
}
a[i][0] = 0;


$ opt64/js test.js
test.js:5:0 TypeError: can't convert undefined to object


$ opt64/js --no-ti test.js
test.js:5:0 TypeError: a[i] is undefined


This is a fuzzblocker because it keeps popping up very frequently.
(Reporter)

Comment 1

5 years ago
Seeing the same also for null (foo is null vs. null has no properties).
(Assignee)

Comment 2

5 years ago
Created attachment 698420 [details] [diff] [review]
fix

I hate this code. We should just get rid of the special error message. This fixes it, but I am not sure if it has bad effects on other stuff.
Tom, is this patch up for a review?
Flags: needinfo?(evilpies)
(Assignee)

Comment 4

5 years ago
I think we can take this patch, this should strictly just improve the error reporting.
Flags: needinfo?(evilpies)
(Assignee)

Updated

5 years ago
Assignee: general → evilpies
(Assignee)

Comment 5

5 years ago
Verified, still passes and fixes the issue.
(Assignee)

Updated

5 years ago
Attachment #698420 - Flags: review?(jdemooij)
Comment on attachment 698420 [details] [diff] [review]
fix

Review of attachment 698420 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, but please add a testcase that fails without the patch (and works with the patch + --baseline-eager)
Attachment #698420 - Flags: review?(jdemooij) → review+
(Assignee)

Comment 7

5 years ago
This fails with baseline at the moment.
(Assignee)

Comment 8

5 years ago
Jan, any ideas on how to fix this?
Flags: needinfo?(jdemooij)
(In reply to Tom Schuster [:evilpie] from comment #8)
> Jan, any ideas on how to fix this?

Changing ToObject to ToObjectFromStack in DoSetElemFallback seems to work for me..
Flags: needinfo?(jdemooij)
(Assignee)

Comment 10

5 years ago
Created attachment 744776 [details] [diff] [review]
v2

You are right. Failure on my part.
Attachment #698420 - Attachment is obsolete: true
Attachment #744776 - Flags: review?(jdemooij)

Updated

5 years ago
Attachment #744776 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/a7e44219367a
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.