Closed Bug 460117 Opened 11 years ago Closed 11 years ago

TM: Inconsistent results from hasOwnProperty with JIT enabled

Categories

(Core :: JavaScript Engine, defect, P1)

x86
Windows XP
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: dsavage23+mozilla, Assigned: jorendorff)

Details

(Keywords: testcase, verified1.9.1)

Attachments

(1 file)

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
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?
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.
Attached patch v1Splinter Review
Assignee: general → jorendorff
Attachment #344103 - Flags: review?(brendan)
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+
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P1
http://hg.mozilla.org/tracemonkey/rev/3461f4b4f1fd
Status: NEW → RESOLVED
Closed: 11 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-
v 1.9.1, 1.9.2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.