39050 - Mozilla crashes when one calls JNI methods Call<Prefix><Type>Method(JNIEnv*, jobject, jclass, jmethodID, ...)

Status: VERIFIED FIXED

(Core Graveyard :: Java: OJI, defect, P3)

Description

[Michail Romanov]

Overview Description: 
When one tries to call JNI methods on the Win32 platform of the 
Call<Prefix><Type>Method(JNIEnv*, jobject, jclass, jmethodID, ...)
type containing jfloat arguments it crashes Mozilla.
The cause of crashing and suggested fix 
see in the last section (Addtitional Information).

Steps to Reproduce: 
1) Goto modules/oji/test/build, read README file and build tests 
2) Goto modules/oji/test/script directory, put one test into         
   OJITestsList.lst file (e.g. JNIEnv_CallFloatMethod_3) and
   run autorun.pl script. Source of the test is in the 
   modules/oji/test/src/JNI/CallingInstanceMethods/CallFloatMethod_3.cpp
   file.

Actual Results: 
Mozilla crashes.

Expected Results: 
It should work OK.

Build Date & Platform Bug Found: 
04/11/2000 Windows NT 4.0

Additional Builds and Platforms Tested On: 

Additional Information: 
Processing of va_list in SunWS and Microsoft compilers is different. 
In particular, according to the specification compiler promotes
float to double before puting the corresponding variable
on stack. Some compilers (such as SunWS) implement
the "back" promotion automatically when one takes data from va_list
(it takes sizeof(double) bytes from va_list and then cast
it to float). But Microsoft compiler remains it for user.

So when one wants to take float from the va_list
he should take sizeof(double) bytes of data and then
cast it to float by hisself. Therefore to fix this bug
one should do the following changes in the
JNIMethod::marshallArgs in ProxyJNI.cpp file (it should
work both with Microsoft and SunWS compilers):
200                 case jlong_type:
201                     jargs[i].j = va_arg(args, jlong);
202                     break;
203                 case jfloat_type:
204                     //jargs[i].f = va_arg(args, jfloat);
                        jargs[i].f = (jfloat)va_arg(args, jdouble);
205                     break;

Also it seems that the following code in the same method
should be corrected (at least for beuty):
188                 case jbyte_type:
189                     jargs[i].b = va_arg(args, jbyte);
190                     break;
191                 case jchar_type:
192                     //jargs[i].b = va_arg(args, jbyte);
                        jargs[i].c = va_arg(args, jchar);
193                     break;

Comment 1

[drapeau]

Thank you very much for the bug report and great description of the problem.  I
have two things to say:

1) I am copying Stanley Ho on this so that he is aware of the problem.

2) If you can put this solution in the form of a patch (cvs diff), and attach
that patch to this bug, I will get it reviewed and checked in.

Great work!

Comment 2

[leger]

Adding crash keyword

Comment 3

[Michail Romanov]

Attachment: This is patch for modules/oji/src/ProxyJNI.cpp file with the suggested fix.

Comment 4

[drapeau]

Nominating for Beta2 keyword.  Also adding Ed Burns to the CC list, so he can
review and apply the suggested patch.  fixing crash problems is important.

Comment 5

[leger]

Putting on [nsbeta2+] radar for beta2 fix. 

Comment 6

[edburns]

Reassigning.

Comment 7

[edburns]

Thanks for your detailed bug report.  However, I cannot build the tests.  I 
have set mozilla home to E:\Projects\mozilla, and JAVAHOME to be C:\jdk1.2.2.

When I run nmake -f Makefile.win in tests\build, I get:


"########### Making make_depends in TestLoader ##############";
"########### Making make_depends in JM ##############";
"########### Making make_depends in TM ##############";
"########### Making make_depends in LCM ##############";
"########### Making make_depends in JNI ##############";
"########### Making all in TestLoader ##############";
NMAKE : fatal error U1073: don't know how to make 'obj/OJITestLoader.obj'
Stop.
NMAKE : fatal error U1077: 'C:\PROGRA~1\MICROS~1\VC98\bin\NMAKE.EXE' : return 
code '0x2'
Stop.
NMAKE : fatal error U1077: 'C:\PROGRA~1\MICROS~1\VC98\bin\NMAKE.EXE' : return 
code '0x2'
Stop.

Can you please help me?

Thanks,

Ed

Comment 8

[edburns]

Applied patch, waiting to hear from testcase author.

Please reassign back to me when you address my testcase building problem.

Thanks,

Ed

Comment 9

[rpallath]

Have checked in  modified sources to reflect new methods in  nsString.h
Updated the M15-OJI-05292000 branch with  these modified tests.
Ed,  could u try it now.

Comment 10

[Igor Kushnirskiy]

Take a look at 37364  (ProxyJNI.cpp does not build with recent GCC snapshots)
that is the same bug as this one. 
Thing I am concerning about is that solution should be platform depended.
There are gonna be different promotions for 32 and 64 bit OS.
Some how we need to take care about that.

Comment 11

[edburns]

I just updated my modules/oji/tests directory and tried to build it with 
a mozilla tree from last Thursday, and here's the result:

OJITestLoader.cpp
../../src/include\ojiapitests.h(113) : error C2664: '__thiscall 
TestResult::TestResult(enum StatusValue,class nsString)' : cannot convert 
parameter 2 from 'char *' to 'class nsString'
        No constructor could take the source type, or constructor overload 
resolution was ambiguous
../../src/include\ojiapitests.h(119) : error C2664: '__thiscall 
TestResult::TestResult(enum StatusValue,class nsString)' : cannot convert 
parameter 2 from 'char *' to 'class nsString'
        No constructor could take the source type, or constructor overload 
resolution was ambiguous
OJITestLoader.cpp(72) : error C2665: 'FAIL' : none of the 3 overloads can 
convert parameter 1 from type 'char [22]'
OJITestLoaderFactory.cpp
../../src/include\ojiapitests.h(113) : error C2664: '__thiscall 
TestResult::TestResult(enum StatusValue,class nsString)' : cannot convert 
parameter 2 from 'char *' to 'class nsString'
        No constructor could take the source type, or constructor overload 
resolution was ambiguous
../../src/include\ojiapitests.h(119) : error C2664: '__thiscall 
TestResult::TestResult(enum StatusValue,class nsString)' : cannot convert 
parameter 2 from 'char *' to 'class nsString'
        No constructor could take the source type, or constructor overload 
resolution was ambiguous
Generating Code...
NMAKE : fatal error U1077: 'cl' : return code '0x2'
Stop.
NMAKE : fatal error U1077: 'C:\PROGRA~1\MICROS~1\VC98\bin\NMAKE.EXE' : return 
code '0x2'
Stop.
NMAKE : fatal error U1077: 'C:\PROGRA~1\MICROS~1\VC98\bin\NMAKE.EXE' : return 
code '0x2'
Stop.
NMAKE : fatal error U1077: 'C:\PROGRA~1\MICROS~1\VC98\bin\NMAKE.EXE' : return 
code '0x2'
Stop.

Compilation exited abnormally with code 2 at Mon Jun 12 16:57:16

I thought this was fixed?

Thanks,

Ed

Comment 12

[rpallath]

Ed,
Try it off the M15-OJI-05292000 branch. New tests are updated in that location.

- raju

Comment 13

[edburns]

Can you please modify mozilla/modules/oji/tests/script/autorun.pl to 
automatically create the BWTest* files?

Comment 14

[edburns]

The testcase fails, but I'm not convinced the testcase is actually getting 
executed.  I say this because I added DebugBreak() into CallFloatMethod_3.cpp, 
yet the debugger never fires up.

Can you please help me.  How do I run the testcase manually?

Ed

Comment 15

[Igor Kushnirskiy]

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

Comment 16

[rpallath]

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

Comment 17

[leger]

Per today's PDT mtg, moving from [nsbeta2+] to [nsbeta2-].

Comment 18

[Christopher Blizzard (:blizzard)]

What's going on with this bug?

Comment 19

[drapeau]

blizzard: nothing is going on with this bug right now.  We're going to work on 
higher-priority bugs for OJI.  This bug was filed from an OJI test case that, 
relative to other bugs on our list, is a corner case.  Should be relatively easy 
to fix, considering the patch is there, but other bugs need more immediate 
attention.  I don't think this will hurt the common-case customer for PR 2.

Comment 20

[Brian Ryner (not reading)]

beard has offered to take a look at this and see if we can fix the compile
issues in redhat 7 beta.

Comment 21

[Patrick C. Beard]

Attachment: A better patch.

Comment 22

[Patrick C. Beard]

Nominating for nsbeta3 since it is a crasher on some platforms.

Comment 23

[Patrick C. Beard]

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

Comment 24

[Kevin B. Hendricks]

The Mozilla bug process is very frustrating.

Can someone please explain why it takes over 3 months to get a bug like this
fixed whena full patch was submitted?  I thought it was taking a long time just
to get my patch submitted from bug 46087 and never realized that this bug and
the fix had been known since May.

I have submitted many patches to glibc, gcc, the linux kernel, etc and have
never had to wait months to get the patch installed.

This seems very very strange to me.

Would someone please take a second and explain?

Thanks,

Kevin

Comment 25

[Patrick C. Beard]

I have a well-tested patch, so I am going to go ahead and check this in.

Comment 26

[Patrick C. Beard]

Patch checked in.

Comment 27

[edburns]

Patrick: thanks for checking this in.  Our understanding at Sun was this bug 
only manifested itself during our automated black box API testing.  As such, it 
was never high enough priority to check in since we felt it couldn't be 
reproduced in a normal user context.  

Comment 28

[shrirang khanzode]

rubberstamp verified based on beard's comments(2000-08-19 10:55 ).