TM: Inconsistent results from hasOwnProperty with JIT enabled

VERIFIED FIXED

Status

()

Core
JavaScript Engine
P1
normal
VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: David Savage, Assigned: jorendorff)

Tracking

({testcase, verified1.9.1})

unspecified
x86
Windows XP
testcase, verified1.9.1
Points:
---
Bug Flags:
blocking1.9.1 +
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.3) Gecko/2008092417 Firefox/3.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1) Gecko/20081007 Firefox/3.1b1

The following code behaves differently when javascript.options.jit.enabled is true: 


function test(o, props) {
  for (var i=0, len=props.length; i<len; i++) {
    if (o.hasOwnProperty(props[i])) {
      // do something
    } else {
      alert(props[i]+': '+o.hasOwnProperty(props[i]));
    }
  }
};

test({ bar: 123, baz: 123, quux: 123 }, ['bar', 'baz', 'quux']);


Reproducible: Always

Steps to Reproduce:
1. Set javascript.options.jit.enabled to true
2. Run the test function

Actual Results:  
alerts "quux: true"


Expected Results:  
no alert
(Reporter)

Comment 1

10 years ago
Pasted the wrong code.  It should have been: 

function test(o, proplist) {
  var props=proplist.split(/\s+/g);
  for (var i=0, len=props.length; i<len; i++) {
    if (o.hasOwnProperty(props[i])) {
      // do something
    } else {
      alert(props[i]+': '+o.hasOwnProperty(props[i]));
    }
  }
};

test({ bar: 123, baz: 123, quux: 123 }, 'bar baz quux');
This is my fault (IIRC I added hasOwnProperty traceable native support) but you cc-list pals, please feel free to take it.

/be
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.9.1?
(Assignee)

Comment 3

10 years ago
The current code has:

static int32 FASTCALL
Object_p_hasOwnProperty(JSContext* cx, JSObject* obj, JSString *str)
{
    jsid id = ATOM_TO_JSID(STRING_TO_JSVAL(str));
    ...
}

I'm reading this right, atoms have to be flat.  In this case str is a dependent string, thanks to the split().  Patching.
(Assignee)

Comment 4

10 years ago
Created attachment 344103 [details] [diff] [review]
v1
Assignee: general → jorendorff
Attachment #344103 - Flags: review?(brendan)
(Assignee)

Comment 5

10 years ago
I guess we could cut corners here--don't bother really atomizing the string, just create a flat-looking JSString on the stack.  I'd rather just fix the bug.  There are bigger fish to fry.
Comment on attachment 344103 [details] [diff] [review]
v1

r=me, thanks.

/be
Attachment #344103 - Flags: review?(brendan) → review+

Updated

10 years ago
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P1
(Assignee)

Comment 7

10 years ago
http://hg.mozilla.org/tracemonkey/rev/3461f4b4f1fd
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
http://hg.mozilla.org/mozilla-central/rev/37b3fdbb0f07
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-460117.js,v  <--  regress-460117.js
initial revision: 1.1
Flags: in-testsuite+
Flags: in-litmus-
Keywords: fixed1.9.1
v 1.9.1, 1.9.2
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → testcase, verified1.9.1
You need to log in before you can comment on or make changes to this bug.