RFR (S): 8072588 - JVM crashes in JNI if toString is declared as an interface method

Wed May 27 13:02:46 UTC 2015

That's why I am asking, cause if all files for all tests will be just in 
a jni folder, it will be a real mess.
That's why I do not understand why we can not group files in folder with 
a bug number as a name underneath "jni" folder to operate faster between 
the folders, otherwise developers must use some guideline on how to name 
the folders of their bugs.

27.05.2015 15:33, Andreas Eriksson пишет:
> I'm not sure, I just find it easier to navigate multi-file tests if 
> they are grouped in a folder.
> This test is one main file that is run by jtreg (ToStringTest.java) 
> and three other files used by it.
> Stefan / David H, should I move all four test files to be in 
> test/runtime/jni/ directly?
>
> - Andreas
>
> On 2015-05-27 14:20, Stas Smirnov wrote:
>> According to the new naming scheme all jni tests must be placed in 
>> one folder test/runtime/jni.
>> Do I understand correctly, and it is missing on the wiki, that if 
>> there are several source files for the test, developer must group 
>> those files in a folder underneath the component folder using a 
>> specific name that reflects the test logic?
>>
>> 27.05.2015 12:01, Andreas Eriksson пишет:
>>> Alright, thanks.
>>>
>>> I'll move the tests back to 
>>> test/runtime/jni/ToStringInInterfaceTest/ and get it pushed 
>>> tomorrow, unless anyone objects.
>>>
>>> - Andreas
>>>
>>> On 2015-05-27 10:48, Stefan Karlsson wrote:
>>>> Hi Andreas,
>>>>
>>>> On 2015-05-27 10:44, Andreas Eriksson wrote:
>>>>> Hi,
>>>>>
>>>>> Thanks for looking at this.
>>>>>
>>>>> I'm getting some conflicting instructions on where the tests 
>>>>> should be placed.
>>>>> According to Stas and David Simms they should be grouped by CR.
>>>>> That was why I moved them from 
>>>>> test/runtime/jni/ToStringInInterfaceTest/ (webrev.01) to 
>>>>> test/runtime/jni/8072588/ (webrev.02).
>>>>> Should I move them back? Or move them some other place?
>>>>
>>>> This was discussed some time ago and we decided to use descriptive 
>>>> names instead of CR numbers. See this page:
>>>> https://wiki.openjdk.java.net/display/HotSpot/Naming+HotSpot+JTReg+Tests 
>>>>
>>>>
>>>> Thanks,
>>>> StefanK
>>>>
>>>>>
>>>>> Thanks,
>>>>> Andreas
>>>>>
>>>>> On 2015-05-27 02:51, David Holmes wrote:
>>>>>> Hi Andreas,
>>>>>>
>>>>>> We don't use bug numbers to name tests any more.
>>>>>>
>>>>>> Otherwise the change seems okay.
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>> On 27/05/2015 1:10 AM, Andreas Eriksson wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Could a Reviewer please take a look at this?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Andreas
>>>>>>>
>>>>>>> On 2015-05-20 14:03, David Simms wrote:
>>>>>>>>
>>>>>>>> Looks good
>>>>>>>>
>>>>>>>> /David Simms
>>>>>>>>
>>>>>>>> On 20/05/15 13:11, Andreas Eriksson wrote:
>>>>>>>>> After some feedback from David and Stas the tests were moved.
>>>>>>>>> Also,mMissing copyright headers were added.
>>>>>>>>>
>>>>>>>>> This is the latest webrev:
>>>>>>>>> http://cr.openjdk.java.net/~aeriksso/8072588/webrev.02/
>>>>>>>>>
>>>>>>>>> Could a Reviewer please look at it?
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Andreas
>>>>>>>>>
>>>>>>>>> On 2015-05-06 12:52, Andreas Eriksson wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> David, I've added the native test to the test suite, could 
>>>>>>>>>> you take
>>>>>>>>>> a look please?
>>>>>>>>>>
>>>>>>>>>> I still need a Reviewer to take a look at this as well.
>>>>>>>>>>
>>>>>>>>>> New webrev: 
>>>>>>>>>> http://cr.openjdk.java.net/~aeriksso/8072588/webrev.01/
>>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8072588
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Andreas
>>>>>>>>>>
>>>>>>>>>> On 2015-03-20 10:14, David Simms wrote:
>>>>>>>>>>>
>>>>>>>>>>> Looks good to me (not a 'R'-eviewer)
>>>>>>>>>>>
>>>>>>>>>>> I'd like to have your test checked in if possible. Native JNI
>>>>>>>>>>> testing has just recently been added, we can talk about the 
>>>>>>>>>>> details
>>>>>>>>>>> off-list.
>>>>>>>>>>>
>>>>>>>>>>> /David Simms
>>>>>>>>>>>
>>>>>>>>>>> On 19/03/15 17:45, Andreas Eriksson wrote:
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> Could someone please take a look at this?
>>>>>>>>>>>>
>>>>>>>>>>>> Regards,
>>>>>>>>>>>> Andreas
>>>>>>>>>>>>
>>>>>>>>>>>> On 2015-03-10 17:10, Andreas Eriksson wrote:
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Please review this fix for a JNI issue in 
>>>>>>>>>>>>> jni_invoke_nonstatic.
>>>>>>>>>>>>>
>>>>>>>>>>>>> The problem is that when toString is declared as an interface
>>>>>>>>>>>>> method it still has a vtable index, not an itable index as
>>>>>>>>>>>>> jni_invoke_nonstatic expects.
>>>>>>>>>>>>>
>>>>>>>>>>>>> This fix checks for a valid itable index instead of 
>>>>>>>>>>>>> checking if
>>>>>>>>>>>>> the holder is an interface.
>>>>>>>>>>>>> I also moved a duplicated check for JNI_VIRTUAL to be done 
>>>>>>>>>>>>> in one
>>>>>>>>>>>>> check instead.
>>>>>>>>>>>>> The change has been verified to fix the problem with a 
>>>>>>>>>>>>> small JNI
>>>>>>>>>>>>> test, and has passed a jprt run with the hotspot testset.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Webrev:
>>>>>>>>>>>>> http://cr.openjdk.java.net/~aeriksso/8072588/webrev.00/
>>>>>>>>>>>>>
>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>> Andreas
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>>>>
>>>
>>
>>
>> -- 
>>
>> Best regards,
>> Stas Smirnov
>>
>> Stas Smirnov | Java VM Runtime SQE
>> Phone: +7 812 3346130 | Mobile: +7 921 9262241
>> Oracle Development SPB, LLC
>> 10th Krasnoarmeyskaya 22A, St. Petersburg, 190103, Russia
>>
>

-- 

Best regards,
Stas Smirnov

Stas Smirnov | Java VM Runtime SQE
Phone: +7 812 3346130 | Mobile: +7 921 9262241
Oracle Development SPB, LLC
10th Krasnoarmeyskaya 22A, St. Petersburg, 190103, Russia