154338 - garbage string (length 0x40008001) as argv[0] in array_join

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Carsten Juergen Hirsch]

Hi Mozilla team,

I did tray to enter the Toyota webpage of Germany http://www.toyota.de 
unfortunately with version 1.1a the very left frame with some navigation links 
is not displayed. With version 1.0 almost the whole page is not displayed.


Regards

Carsten

Comment 1

[Andreas Kunz]

I can confirm that the navigation column at the left is not displayed in recent
trunk builds (2002062504 on Win2k), but for me it looks exactly the same in 1.0.

And: "very left frame" may sound misleading: I don't think this page uses HTML
frames.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Minimal testcase

The page triggers a JSMSG_OUT_OF_MEMORY using code that I have reduced to this
attachment...  Both lines 5 and 6 will trigger the error; what the site does is
more like line 6, but line 5 is simpler and shorter (though possibly not
equivalent).

Comment 3

[Boris Zbarsky [:bzbarsky]]

And to jseng

Comment 4

[jag (Peter Annema)]

js> ["a","a"].join("H"[0]);
typein:5: out of memory

js> ["a","a"].join("abc".substr(1,1));
typein:10: out of memory

js> var b = "H"[0];
js> ["a","a"].join(b)
typein:15: out of memory

Comment 5

[Steven Cole]

This is not a true memory issue (memory is presumably why timeless added me to
the CC list...); I'm getting a garbage string (length 0x40008001) as argv[0] in
array_join, which is why the allocation fails.

My analysis stops there, though.  

--scole

Comment 6

[Phil Schwartau]

Reassigning to Brendan. Here is an elaboration of Boris' testcase:

var arr = new Array("a", "b");
var x = "Home"[0];
var y = ("Home".split("H"))[1];

print(x);             <--- outputs "H"
print(y);             <--- outputs "ome"

print(x === "H");     <--- outputs "true"
print(y === "ome");   <--- outputs "true"

arr.join("H");        <--- outputs "a, b"    no problem
arr.join("ome");      <--- outputs "a, b"    no problem

arr.join(x);          <--- out of memory error
arr.join(y);          <--- out of memory error

Comment 7

[rogerl (gone)]

Attachment: Use JSSTRING_LENGTH instead of ->length

Old style (pre dependent string fix) reference to length field, should have use
JSSTRING_LENGTH macro instead.

Comment 8

[Phil Schwartau]

Two typos in Comment #6; sorry. Here is the actual output:

var arr = new Array("a", "b");
var x = "Home"[0];
var y = ("Home".split("H"))[1];

print(x);             <--- outputs "H"
print(y);             <--- outputs "ome"

print(x === "H");     <--- outputs "true"
print(y === "ome");   <--- outputs "true"

arr.join("H");        <--- outputs "aHb"      no problem
arr.join("ome");      <--- outputs "aomeb"    no problem

arr.join(x);          <--- out of memory error
arr.join(y);          <--- out of memory error

Comment 9

[Phil Schwartau]

And let me reassign back to rogerl, since he has provided a patch -

Comment 10

[Kenton Hanson (gone)]

Comment on attachment 89291 [details] [diff] [review]
Use JSSTRING_LENGTH instead of ->length

r=khanson

Comment 11

[Phil Schwartau]

Testcase added to JS testsuite:

          mozilla/js/tests/js1_5/Array/regress-154338.js

Comment 12

[jag (Peter Annema)]

Comment on attachment 89291 [details] [diff] [review]
Use JSSTRING_LENGTH instead of ->length

sr=jag

Are there any other places we might've forgotten to patch?

Comment 13

[rogerl (gone)]

Well I believe not. When #56940 was checked in we went through the source pretty
carefully, [I thought!] I'm still not sure how this one got by - it's not like
it's new code (per lxr it's from 1998). I just looked again, by changing the
field name and examining the compile errors and I don't see anything.

Comment 14

[jag (Peter Annema)]

Should this go into the branch?

Comment 15

[rogerl (gone)]

Fix checked in to trunk.

Comment 16

[Brendan Eich [:brendan]]

Let's get this into the 1.0 branch for 1.0.1.

/be

Comment 17

[Brendan Eich [:brendan]]

Comment on attachment 89291 [details] [diff] [review]
Use JSSTRING_LENGTH instead of ->length

Roger, can you check this into the branch today?  I'll set mozilla1.0.1+
(forgot to do that last time, sorry for the spam), so change that to fixed1.0.1
when the patch is in the branch.  Thanks,

/be

Comment 18

[rogerl (gone)]

Fixed in branch, too.

Comment 19

[Phil Schwartau]

*** Bug 168073 has been marked as a duplicate of this bug. ***

Comment 20

[Bob Clary [:bc] (inactive)]

verified fixed 1.9 20060818