Hi Ali, Magnus,
I absolutely agree this change has to be reviewed by someone from
serviceability.
There are several options:
1. Add -export:jdwpTransport_OnLoad to LDFLAGS for Windows
http://mail.openjdk.java.net/pipermail/build-dev/2018-November/023935.html
so it partially reverts the changes from
https://bugs.openjdk.java.net/browse/JDK-8200178
2. Remove JNICALL (__stdcall) modifier, eventually changing the calling
convention to __cdecl.
http://mail.openjdk.java.net/pipermail/build-dev/2018-November/023969.html
3. Use inline /export option via #pragma:
#pragma comment(linker, "/export:jdwpTransport_OnLoad=
_jdwpTransport_OnLoad@16")
as referenced in
https://docs.microsoft.com/en-ie/cpp/cpp/dllexport-dllimport?view=vs-2017
https://docs.microsoft.com/en-ie/cpp/build/reference/exports?view=vs-2017
The third options still needs to be tested to make sure it does not
break other platforms builds.
Regards,
Alexey
On 19/11/2018 12:19, Magnus Ihse Bursie wrote:
Hi Ali,
From a build perspective this change looks OK. I'm not aware of the
finer details on the OnLoad mechanism, like what calling convention is
to be used. So maybe this is a no-go from that view.
I'm cc:ing servicability so they can have a look at it.
/Magnus
On 2018-11-18 13:07, Ali İnce wrote:
Hi Alexey,
Below is a new patch content that removes JNICALL modifiers from the exported
functions of interest. This results in exported functions not to be name
decorated and use __cdecl calling convention.
Do you have any more suggestions, or would you please guide me on next steps?
Thanks,
Ali
# HG changeset patch
# User Ali Ince<ali.i...@gmail.com>
# Date 1542542415 0
# Sun Nov 18 12:00:15 2018 +0000
# Node ID a41df44e13e1b761f4b3f05a17ed18953067ae39
# Parent 16d5ec7dbbb49ef3f969e34be870e3f917ea89ff
Remove JNICALL modifiers to prevent name decoration on 32-bit windows builds
diff -r 16d5ec7dbbb4 -r a41df44e13e1
src/jdk.jdi/share/native/libdt_shmem/shmemBack.c
--- a/src/jdk.jdi/share/native/libdt_shmem/shmemBack.c Tue Aug 14 14:28:23
2018 +0200
+++ b/src/jdk.jdi/share/native/libdt_shmem/shmemBack.c Sun Nov 18 12:00:15
2018 +0000
@@ -339,7 +339,7 @@
return JDWPTRANSPORT_ERROR_NONE;
}
-JNIEXPORT jint JNICALL
+JNIEXPORT jint
jdwpTransport_OnLoad(JavaVM *vm, jdwpTransportCallback* cbTablePtr,
jint version, jdwpTransportEnv** result)
{
diff -r 16d5ec7dbbb4 -r a41df44e13e1
src/jdk.jdwp.agent/share/native/include/jdwpTransport.h
--- a/src/jdk.jdwp.agent/share/native/include/jdwpTransport.h Tue Aug 14
14:28:23 2018 +0200
+++ b/src/jdk.jdwp.agent/share/native/include/jdwpTransport.h Sun Nov 18
12:00:15 2018 +0000
@@ -140,7 +140,7 @@
void (*free)(void *buffer); /* Call this for all deallocations */
} jdwpTransportCallback;
-typedef jint (JNICALL *jdwpTransport_OnLoad_t)(JavaVM *jvm,
+typedef jint (*jdwpTransport_OnLoad_t)(JavaVM *jvm,
jdwpTransportCallback
*callback,
jint version,
jdwpTransportEnv** env);
diff -r 16d5ec7dbbb4 -r a41df44e13e1
src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c
--- a/src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c Tue Aug
14 14:28:23 2018 +0200
+++ b/src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c Sun Nov
18 12:00:15 2018 +0000
@@ -1019,7 +1019,7 @@
return JDWPTRANSPORT_ERROR_NONE;
}
-JNIEXPORT jint JNICALL
+JNIEXPORT jint
jdwpTransport_OnLoad(JavaVM *vm, jdwpTransportCallback* cbTablePtr,
jint version, jdwpTransportEnv** env)
{
On 16 Nov 2018, at 23:03, Alexey Ivanov<alexey.iva...@oracle.com> wrote:
Hi Ali,
It's exactly what I referred to.
Yes, it does change the calling convention.
Yet it's not a (big) problem because both parties will use the same calling
convention.
Using a DLL from another build will not be possible. But it's not a good idea
to do so anyway.
Mapfiles and export options have been removed
byhttps://bugs.openjdk.java.net/browse/JDK-8200178 to simplify managing
exported functions. So that to add or remove an exported function, you don't
have modify makefiles and/or mapfiles. You just edit the source files.
Regards,
Alexey
On 16/11/2018 22:42, Ali İnce wrote:
Hi Alexey,
Thanks for your reply.
I will definitely give it a try though I’m not sure if that’ll be a breaking
change. This is because JNICALL modifier is defined to be __stdcall on windows
which is both specified on jdwpTransport_OnLoad exported functions both for
dt_socket.dll and dt_shmem.dll.
The __stdcall is ignored on x64 platforms and also name decoration is not
applied
(https://docs.microsoft.com/en-gb/cpp/build/reference/decorated-names?view=vs-2017).
I believe that’s why we don’t experience any problems on x64 builds.
Removing JNICALL (thus __stdcall) modifier (apparently only applies to x86
builds) will result in the calling convention to be changed, and thus will
change how parameters are ordered and also the stack cleanup rules. I think
this may result in problems in programs that are designed to load shared
libraries and its exported functions at runtime (not bound by link time which
probably won’t cause any issues - assuming that they use the shared function
signature) - as
inhttps://github.com/AdoptOpenJDK/openjdk-jdk11u/blob/5f01925b80ed851b133ee26fbcb07026ac04149e/src/jdk.jdwp.agent/share/native/libjdwp/transport.c#L99.
Any thoughts?
Regards,
Ali
On 15 Nov 2018, at 22:14, Alexey Ivanov<alexey.iva...@oracle.com> wrote:
Hi Ali,
Can the issue be resolved by using different call modifiers on the affected
functions?
Some build / link issues were resolved by adding / removing JNICALL modifier,
see
https://bugs.openjdk.java.net/browse/JDK-8201226
http://mail.openjdk.java.net/pipermail/build-dev/2018-April/021553.html
https://bugs.openjdk.java.net/browse/JDK-8202476
http://mail.openjdk.java.net/pipermail/build-dev/2018-May/021913.html
Regards,
Alexey
On 15/11/2018 21:43, Ali İnce wrote:
Hi,
An issue (https://github.com/AdoptOpenJDK/openjdk-build/issues/709) raised
against AdoptOpenJDK jdk11 32-bit windows builds led us to the name decoration
problem on built 32-bit dlls - specifically dt_socket.dll and dt_shmem.dll.
Whereas 64-bit MSVC builds don’t apply any name decorations on exported
functions, 32-bit builds apply them - which requires either def files or
/export flags to be specified on the linker arguments.
Although the expected linker arguments were present on previous jdk makefiles,
they were removed in jdk11 makefiles.
Below is the proposed patch, which can also be browsed
athttps://github.com/AdoptOpenJDK/openjdk-jdk11u/pull/1.
Would you please have a look and let me know of any points I may have missed (I
don’t have any background information about why the export flags were removed
in jdk11)?
Regards,
Ali
diff --git a/make/lib/Lib-jdk.jdi.gmk b/make/lib/Lib-jdk.jdi.gmk
index 197b95c2e2..ac6ebf5591 100644
--- a/make/lib/Lib-jdk.jdi.gmk
+++ b/make/lib/Lib-jdk.jdi.gmk
@@ -37,6 +37,7 @@ ifeq ($(OPENJDK_TARGET_OS), windows)
jdk.jdwp.agent:include \
jdk.jdwp.agent:libjdwp/export, \
LDFLAGS := $(LDFLAGS_JDKLIB), \
+ LDFLAGS_windows := -export:jdwpTransport_OnLoad, \
LIBS := $(JDKLIB_LIBS), \
))
diff --git a/make/lib/Lib-jdk.jdwp.agent.gmk b/make/lib/Lib-jdk.jdwp.agent.gmk
index 0bc93e0d35..0382e55325 100644
--- a/make/lib/Lib-jdk.jdwp.agent.gmk
+++ b/make/lib/Lib-jdk.jdwp.agent.gmk
@@ -37,6 +37,7 @@ $(eval $(call SetupJdkLibrary, BUILD_LIBDT_SOCKET, \
libjdwp/export, \
LDFLAGS := $(LDFLAGS_JDKLIB) \
$(call SET_SHARED_LIBRARY_ORIGIN), \
+ LDFLAGS_windows := -export:jdwpTransport_OnLoad, \
LIBS_linux := -lpthread, \
LIBS_solaris := -lnsl -lsocket, \
LIBS_windows := $(JDKLIB_LIBS) ws2_32.lib, \
