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

Fri May 29 01:34:58 UTC 2015

On 27/05/2015 11:40 PM, 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 ?

No we don't use bug numbers for naming. Either the tests sit in the jni 
folder or a subfolder, such as the suggested ToStringInInterfaceTest.

David

>
> 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
>>>>>
>>>
>
>
> --
>
> 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
>