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

Wed May 27 13:40:09 UTC 2015

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 ?

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