RFR JDK-8232624: Java cannot start: NewStringPlatform missing

[David Holmes]

On 21/10/2019 10:14 pm, Alexey Ivanov wrote:
> Hi David,
> 
> On 21/10/2019 02:19, David Holmes wrote:
>> Hi Alexey,
>>
>> On 21/10/2019 9:37 am, Alexey Ivanov wrote:
>>> Hi David,
>>>
>>> On 20/10/2019 23:59, David Holmes wrote:
>>>> Hi Alexey,
>>>>
>>>> On 21/10/2019 2:20 am, Alexey Ivanov wrote:
>>>>> Hello,
>>>>>
>>>>> Please review the following fix which it brings back 
>>>>> NewStringPlatform alias for JNU_NewStringPlatform. Without it, 32 
>>>>> bit Windows build of Java does not work.
>>>>>
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8232624
>>>>> webrev: http://cr.openjdk.java.net/~aivanov/8232624/webrev.00/
>>>>
>>>> Not sure this is the correct fix. The problem as I see it is that 
>>>> NewStringPlatform was not declared as a JNICALL previously whereas 
>>>> the JNU_NewStringPLatform is. That affects the linkage on 32-bit 
>>>> Windows only.
>>>
>>> Yes, JNICALL affects 32-bit Windows only. And this is exactly why 
>>> "JNU_NewStringPLatform" cannot be found by its name on 32-bit 
>>> Windows; "NewStringPlatform" is not declared as JNICALL and it can be 
>>> found by its undecorated name on 32-bit Windows and all the other 
>>> platforms. I believe it's the reason why this alias exists.
>>
>> So what would happen if we drop the JNICALL from JNU_NewStringPLatform?
> 
> Yes, it will be found by its undecorated name too.
> But I'd rather not change the calling conventions of functions with JNU_ 
> prefix.

Yes you are right. All the JNU_ methods are declared as JNICALL and they 
all work fine.

The issue/problem here is that we have the JVM using os::dll_lookup to 
find a method back in libjava and that can't be a JNICALL else the 
lookup won't work (without jumping through extra hoops).


> So, I think the proposed patch is the easiest and safest way to fix 
> 32-bit Windows build.

Yes I now agree. Thanks for bearing with me.

>>> Another way to fix it is to lookup the undecorated name first and, if 
>>> it fails, to lookup the decorated name, which makes the code harder 
>>> to read.
>>>
>>> With this patch, I'm reverting the code to the state it was before.
>>
>> Yes, but Claes didn't like the way it was before :) so I'm hoping we 
>> can keep his cleanup whilst still allowing Windows to work correctly.
> 
> Probably, Claes thought "NewStringPlatform" wasn't used. Yet it proved 
> that "NewStringPlatform" is still used.
> 
> If required, I can create a follow-up issue to re-do the cleanup as Alan 
> suggested.

Okay. Though I'm not sure what form that might take.

Thanks,
David