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
