306591 - String static methods call with primitive value causes crash

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[nanto_vi (TOYAMA Nao)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; ja-JP; rv:1.7.10) Gecko/20050717 Firefox/1.0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050830 Firefox/1.0+

Calling String static methods, introduced in bug 304828, with primitive value as
first argument cause Firefox crash.


Reproducible: Always

Steps to Reproduce:
1. Open JavaScript Console
2. Evaluate the script: String.toUpperCase("abc");

Actual Results:  
Firefox crashed

Expected Results:  
"ABC"

OK:
String.split(new String("abc"), "");     // ["a", "b", "c"]
String.substring(new Number(123), 1, 2); // "2"
String.toUpperCase(new Boolean(true));   // "TRUE"
String.indexOf(null, "l");               // 2

Crash:
String.split("abc", "");
String.substring(123, 1, 2);
String.toUpperCase(true);
String.indexOf(undefined, "d");

Talkback crashID: TB8899225E

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: make sure this is an object

String, at least, calls JSVAL_TO_OBJECT on the object passed into its
functions, assuming that they are, in fact, objects (as opposed to primitives).
This patch ensures that that assumption isn't violated.

Comment 2

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Attachment: Make sure that we have an object for |this|.

All better, I do believe.

Comment 3

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 194442 [details] [diff] [review]
make sure this is an object

This is better, especially because it doesn't leave the null crash. sr=shaver

Comment 4

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 194443 [details] [diff] [review]
Make sure that we have an object for |this|.

I am slain.

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 194442 [details] [diff] [review]
make sure this is an object

Duh, and stuff.

/be

Comment 6

[Brendan Eich [:brendan]]

Thanks, you guys.

/be

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into MOZILLA_1_8_BRANCH and trunk. Thanks for the heads up.

Comment 8

[Bob Clary [:bc] (inactive)]

Checking in regress-306591.js;
/cvsroot/mozilla/js/tests/js1_6/String/regress-306591.js,v  <--  regress-306591.js
initial revision: 1.1
done

String.indexOf(arg, pattern) should do a conversion to String on arg, but it
fails to do so if arg is undefined or null. I'll file a separate bug.

Comment 9

[Brendan Eich [:brendan]]

That would be bug 312385 -- but it may be INVALID if we all agree on the
rationale for the current generic method dispatcher design.

/be

Comment 10

[Bob Clary [:bc] (inactive)]

verified fixed 1.8.x and trunk (no crash). Tests still (erroneously?) fail due to bug 312385.

Comment 11

[Bob Clary [:bc] (inactive)]

Checking in js1_6/String/regress-306591.js;
/cvsroot/mozilla/js/tests/js1_6/String/regress-306591.js,v  <--  regress-306591.js
new revision: 1.2; previous revision: 1.1
done