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

Wed May 27 13:50:50 UTC 2015

On 5/27/15 7:40 AM, Stas Smirnov wrote:
> David,
>
> I am confused a bit, you wrote "We don't use bug numbers to name tests 
> any more. " and then "I don't have a preference as to whether these 
> files sit in the jni folder, or are placed in a further subfolder. "
> We do not name tests using bug numbers, only the unit folders, so
> what will be a solution here, it is ok to put new tests this way 
> test/runtime/jni/<bug number>/TestFile1.java ?

The "<bug number>" part of the path would not be acceptable
under the naming guidelines. Bug numbers in the folder names
or in the test names don't lend themselves to easy understanding
of what the test does.

This name:

test/runtime/jni/ToStringInInterfaceTest/...

will help me find a JNI toString() test. It will also help
me find an interface related JNI test...

Dan

>
> 27.05.2015 16:17, David Holmes пишет:
>> On 27/05/2015 11:13 PM, Staffan Larsen wrote:
>>>
>>>> On 27 maj 2015, at 15:12, David Holmes <david.holmes at oracle.com
>>>> <mailto:david.holmes at oracle.com>> wrote:
>>>>
>>>> On 27/05/2015 11:02 PM, Stas Smirnov wrote:
>>>>> 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.
>>>>
>>>> Use of bugs numbers was decided to be too obscure and that instead
>>>> functional descriptive names should be used. There's no hard and fast
>>>> rule on the granularity of grouping.
>>>
>>> See:
>>> https://wiki.openjdk.java.net/display/HotSpot/Naming+HotSpot+JTReg+Tests 
>>>
>>
>> That page is very vague and states:
>>
>> "It is up to each group to organize the file structure within the 
>> corresponding component folder."
>>
>> Looking at what has been done in different areas there are no hard 
>> and fast rules.
>>
>> David
>> -----
>>
>>>>
>>>> I don't have a preference as to whether these files sit in the jni
>>>> folder, or are placed in a further subfolder.
>>>>
>>>> David
>>>> -----
>>>>
>>>>> 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
>>>>>
>>>
>
>