Closed Bug 584579 Opened 11 years ago Closed 11 years ago

JM: mochitest-plain crash on dom/tests/mochitest/ajax/prototype/test_Prototype.html

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dmandelin, Assigned: dmandelin)

References

Details

Attachments

(2 files)

We crash an a match function called from str_IndexOf. It looks like the argument is a String object that is not getting unwrapped correctly, but instead gets cast directly to a string. Or something.
I think we might be going wrong in the callprop in g().
Attached patch PatchSplinter Review
The cause was that jsop_binary tries to optimize storing a string-tagged result if either operand is a string. But if one operand is a string object, then the result is a string object, so the condition must be strengthened to "one operand is a string, the other is not an object".
Attachment #463395 - Flags: review?(dvander)
Attachment #463395 - Flags: review?(dvander) → review+
http://hg.mozilla.org/projects/jaegermonkey/rev/722420b0c4c8
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
(In reply to comment #2)
> 
> But if one operand is a string object, then the
> result is a string object

Are you sure about that?

  js> typeof("1" + new String('abc'))
  "string"
  js> typeof(new String('abc') + "1") 
  "string"

Am I missing something?
(In reply to comment #4)

Good catch. Something more sinister is at play here. stubs::Add() returns a JSVAL_TYPE_STRING in both cases. Backing out this patch locally, it flows into ic::GetProp, which sees a string object.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
There were two bugs here. The first is that CALLPROP on a statically known string didn't sync the right amount of values when wrapping a primitive. The other is that after wrapping a primitive, it still tracked the value as a string.

The original fix masked the problem by eliminating cases where callprop_str could be used.

http://hg.mozilla.org/projects/jaegermonkey/rev/225495aeb3d5
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Why wrap a string constant with a String object to call a method on it? The implicit conversion should be eliminated if the method is one of the well-known natives on String.prototype. Sorry if that is ruled out here and a possibly user-defined method is being called.

Even if not a well-known native, if the function uses "use strict"; then it'll need to get a primitive string as |this|.

/be
(In reply to comment #7)

Yeah, we inline a primitive-this test and take an out-of-line path to wrap. This path is generic, we don't guess the method at compile-time (yet - seems like future work could be done there, if it's worth it.)
You need to log in before you can comment on or make changes to this bug.