227170 - jsobject should be defined as jlong.

Status: RESOLVED DUPLICATE of bug 239562

(Core Graveyard :: Java: Live Connect, defect)

Description

[Shanmu]

User-Agent:       Mozilla/5.0 (X11; U; OSF1 alpha; en-US; rv:1.5) Gecko/20031112
Build Identifier: Mozilla/5.0 (X11; U; OSF1 alpha; en-US; rv:1.5) Gecko/20031112

In the mozilla header files jsobject is 
defined to be an int, and all the interfaces into mozilla that pass
jsobject (Java Script objects) are also int.  Unfortunately, a
java script object is not an int, but a pointer. So I can write an
applet that will cause the mozilla crash on a 64-bit OS, because
pointers are 64 bits. 
The definition of jsobject should be changed "to jlong" 
at the following places.

./js/src/liveconnect/nsILiveconnect.h:typedef jint jsobject;
./js/src/liveconnect/nsISecureLiveconnect.h:typedef jint jsobject;


Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Shanmu]

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

Comment 2

[Phil Schwartau]

cc'ing Brendan, Mike on this -

Comment 3

[Kyle Yuan]

I wonder if java script object is a pointer, why we don't just define it as a
pointer type, such as void *, so that it's 32-bit on a 32-bit OS and 64-bit on a
64-bit OS?

Comment 4

[Brendan Eich [:brendan]]

The implementations of liveconnect/nsCLiveConnect.cpp methods that take jsobject
obj parameters all cast obj to (JSObjectHandle*) right away.  Why not just use
that type?  Or void* if you want to hide that type?

I think the jsobject name is quite wrong for this typedef, and it invades the JS
engine's namespace (see http://lxr.mozilla.org/mozilla/source/js/src/README.html
for naming conventions).  I'd rather you rename it lcobject or some such name,
while you're fixing this.  Thanks,

/be

Comment 5

[Kyle Yuan]

Shanmugavelu, did you mean that you run 32-bit mozilla on 64-bit OS or you
re-compile mozilla in 64-bit mode?

Xiaobin, I see that the "jsobject" is also stored as jint(or just int) in the
JPI code. 

Comment 6

[Shanmu]

I build Mozilla on a 64 bit machine and running in the same machine.
I am not running a 32-bit version of mozilla.

Comment 7

[Jack Lu]

Did we claim that we support 64-bit OS ?

Comment 8

[Jack Lu]

In addition to the above comment, just let you know, java plug-in does not 
support 64-bit OS today.

Comment 9

[Brendan Eich [:brendan]]

Mozilla supports 64-bit OS/compiler modes, though -- in particular the LP64
model (longs and pointers are 64 bits).  We don't support LLP64 yet.  But since
we have always used long (typedef'd variously) to hold pointer types, I think
Mozilla's LiveConnect should too.

/be

Comment 10

[Kyle Yuan]

Xiaobin, is that possible to add 64-bit support to the JEA new design? I see the
jre is already available for 64-bit Solaris, why do not we do the same thing for
java-plugin?

Comment 11

[Jack Lu]

Based on the jni_md.h, jint is typedefed as long. Support plug-in in 64-bit is  
not trivial for Java Plug-in. On Windows, this may involve to use out-of-proc 
model etc. We are evaluating it now and may do it in JRE 1.5.1. 

Comment 12

[Kyle Yuan]

taking...

Comment 13

[Kyle Yuan]

Attachment: patch

1. converted jsobject to pointer type and renamed it as "lcJSObject";
2. removed nsISecureLiveconnect.h because it's totally unused;
3. some JNI methods in jsj_JSObject.c are unused when OJI defined;
4. coding-style cleaned up, including tab->space conversion, indent adjustment,
etc.

Comment 14

[Brendan Eich [:brendan]]

How about attaching a patch generated by diff -pwu 8?  If you add "diff -pu 8"
to your .cvsrc, you can add -w optionally from the command line, but always get
good context thanks to -p and 8 (or whatever number of lines you want, but more
than the default).

/be

Comment 15

[Kyle Yuan]

Attachment: patch with diff -Npwu8 for easy reviewing

Comment 16

[Jack Lu]

I am trying to get the point of this bug. jsobejct is defined as jint, not as 
int. And based on jri_md.h, jint is defined as:

   #ifdef IS_64 /* XXX ok for alpha, but not right on all 64-bit architectures 
*/
   typedef unsigned int	juint;
   typedef int	jint;
   #else
   typedef unsigned long	juint;
   typedef long			jint;
   #endif

So if we want to support 64-bit architecture, should we do something to 
this definition first? If the IS_64 only works for alpha CPU, but not right on 
all other 64-bit architecture, why should we use IS_64 here?

Comment 17

[Kyle Yuan]

I can't see any reason to cast a pointer to a int or a long in the OJI side.
IMO, it should be handled in the JPI side.

Comment 18

[Jack Lu]

Kyle,

I am think we should have a better way to fix this issue. jri_md.h is from sun-
java/stubs/include/ and is included by jni.h which could be included by many 
other files. Also if we mistakenly define jint as int in 64-bit architecture, it 
could cause other problems as well. Notice that jsize is also typedefed as jint.

I know in order to support 64-bit chip, JPI side will involve extensive change 
and that won't be done until 1.5.1 release. So there will be no way to verify 
this bug unless using a 64-bit Java Plug-in (Blackdown may support 64-bit, not 
sure?).

Comment 19

[Kyle Yuan]

Xiaobin, 

IS_64 is not only for alpha, it gets defined almost for all popular 64-bit CPU, see:
http://lxr.mozilla.org/mozilla/source/nsprpub/pr/include/md/*64.cfg

the code you listed in comment 16 is here since Mar, 1998 (maybe earlier). Alpha
might be the only 64-bit cpu at that time.

I don't know how the idea to "keep jint 32-bit" comes, but changing jint to
64-bit in a 64-bit environment is a big deal to current code. We need to think
about it carefully.

Comment 20

[Xiaobin Lu]

Kyle, is this a dup of 239562?

Comment 21

[Kyle Yuan]

Yes, but I would rather dup bug 239562 to this one because this one is older and
more straightforward to the problem. Do you have any plan on this?

Comment 22

[Kyle Yuan]

*** This bug has been marked as a duplicate of 239562 ***