[PATCH] Windows 32-bit DLL name decoration

Discussion:

Magnus Ihse Bursie

2018-11-19 12:19:07 UTC

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

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
# 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)
{

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 by https://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

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 in https://github.com/AdoptOpenJDK/openjdk-jdk11u/blob/5f01925b80ed851b133ee26fbcb07026ac04149e/src/jdk.jdwp.agent/share/native/libjdwp/transport.c#L99.
Any thoughts?
Regards,
Ali

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

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 at https://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, \

Alexey Ivanov

2018-11-20 15:18:51 UTC

Permalink

Hi Ali,

I have submitted a JBS issue for you:
https://bugs.openjdk.java.net/browse/JDK-8214122
“Prevent name mangling of jdwpTransport_OnLoad in dt_socket and dt_shell
dynamic link libraries”

Let me know if I got any details wrong.

I've left the bug unassigned. Once reviewed, I can sponsor your commit
if no one else with more experience in the area volunteers.

Regards,
Alexey

Hi Alexey,
I don’t have an account on JBS as I’m not an author yet, my OCA has just been processed. Would it be possible for someone with an author status to create an issue?
Regards,
Ali

Post by Magnus Ihse Bursie
Hi Ali,
The fix looks good to me provided it resolves your problem.
I am not a reviewer so you'll have to get OK from reviewers, likely from build-dev and from core-libs.
Have you submitted the issue in JBS?
https://openjdk.java.net/contribute/
and also
https://openjdk.java.net/sponsor/
Regards,
Alexey

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 in https://github.com/AdoptOpenJDK/openjdk-jdk11u/blob/5f01925b80ed851b133ee26fbcb07026ac04149e/src/jdk.jdwp.agent/share/native/libjdwp/transport.c#L99.
Any thoughts?
Regards,
Ali

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 at https://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, \

Alexey Ivanov

2018-11-20 15:49:50 UTC

Permalink

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=
***@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

Post by Magnus Ihse Bursie
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

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

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

Ali Ince

2018-11-20 17:38:08 UTC

Permalink

Hi Alexey, Magnus,

Thanks for all of your comments on the possible options.

Another option would be to use def files which contain an EXPORTS section
that lists the exported functions (
https://docs.microsoft.com/en-gb/cpp/build/reference/exports?view=vs-2017).
These files can lie next to the native source files and will require a
/DEF:<file-name> option passed to LDFLAGS_windows. Very similar to Option 1
as Alexey suggested, but could be handy especially on other occurrences of
exported functions with JNICALL modifier.

I still believe Option 1 or the one I mentioned above will be a more
conservative approach to fix this issue but I'm a newcomer so I'll be happy
to take the option as suggested.

Regards,

Ali

Post by Alexey Ivanov
Hi Ali, Magnus,
I absolutely agree this change has to be reviewed by someone from
serviceability.
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
#pragma comment(linker, "/export:jdwpTransport_OnLoad=
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
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
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
# 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)
{
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 by https://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
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 in https://github.com/AdoptOpenJDK/openjdk-jdk11u/blob/5f01925b80ed851b133ee26fbcb07026ac04149e/src/jdk.jdwp.agent/share/native/libjdwp/transport.c#L99.
Any thoughts?
Regards,
Ali
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, seehttps://bugs.openjdk.java.net/browse/JDK-8201226http://mail.openjdk.java.net/pipermail/build-dev/2018-April/021553.html
https://bugs.openjdk.java.net/browse/JDK-8202476http://mail.openjdk.java.net/pipermail/build-dev/2018-May/021913.html
Regards,
Alexey
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 at https://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, \

Magnus Ihse Bursie

2018-11-21 12:16:04 UTC

Permalink

I'm not fond of either solution 1 or 3. I really think the signature
should be made correctly at the point of definition in the source code.
We've spent some considerable effort of getting rid of the /export flags
for Windows (and map files for unix), and I don't want to see them creep
back in. Note that option 3 is just option 1 in disguise. The only
single good thing about it is that it allows you to put the compiler
option next to the signature declaration in the source code.

The same goes for def files, as suggested by Ali. It's just yet another
version of option 1 in disguise/map files.

/Magnus

Post by Alexey Ivanov
Regards,
Alexey

Post by Magnus Ihse Bursie
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

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

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

Alexey Ivanov

2018-11-22 17:04:23 UTC

Permalink

Post by Magnus Ihse Bursie

I'm not fond of either solution 1 or 3. I really think the signature
should be made correctly at the point of definition in the source code.

That is why I proposed removing JNICALL (__stdcall) from the function
declaration as we had done for libzip, libjimage [1] and mlib_image [2].

Microsoft recommends using __stdcall for DLLs, at the same time it
decorates the function name making it harder to export it by its plain name.

By removing JNICALL / __stdcall, we make the function use __cdecl
calling convention. The difference between them is that with __stdcall
the callee cleans the stack whereas with __cdecl the caller cleans the
stack.

It shouldn't be a problem as long as all function declarations use the
same calling convention, which, I believe, is the case here.

Post by Magnus Ihse Bursie
We've spent some considerable effort of getting rid of the /export
flags for Windows (and map files for unix), and I don't want to see
them creep back in. Note that option 3 is just option 1 in disguise.
The only single good thing about it is that it allows you to put the
compiler option next to the signature declaration in the source code.

At least in this case, the option will be near the function body
definition. It will be easier to update it if the signature changes.

If we are to use __stdcall, it seems to be the only way to export the
undecorated name.

Post by Magnus Ihse Bursie
The same goes for def files, as suggested by Ali. It's just yet
another version of option 1 in disguise/map files.

If option 2 is rejected, I'm for using option 3. If option 3 cannot be
used too, I'm for option 1.
I think we should not fall back to def files in any case. And Makefile
will have to include the reference to the def file, so it's even worse
than option 1.

Regards,
Alexey

[1] https://bugs.openjdk.java.net/browse/JDK-8201226
[2] https://bugs.openjdk.java.net/browse/JDK-8202476

Post by Magnus Ihse Bursie
/Magnus

Post by Alexey Ivanov
Regards,
Alexey

Post by Magnus Ihse Bursie
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

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

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

Magnus Ihse Bursie

2018-11-22 17:29:08 UTC

Permalink

I think we are in complete agreement. What's missing is some expert opinion from serviceability-dev if there is any problem with changing the signature. I'd preferably have that.

If no one knows, I'd say, change it, and see if we still pass the relevant tests, and if so, ship it.

/Magnus

I'm not fond of either solution 1 or 3. I really think the signature should be made correctly at the point of definition in the source code.

That is why I proposed removing JNICALL (__stdcall) from the function declaration as we had done for libzip, libjimage [1] and mlib_image [2].
Microsoft recommends using __stdcall for DLLs, at the same time it decorates the function name making it harder to export it by its plain name.
By removing JNICALL / __stdcall, we make the function use __cdecl calling convention. The difference between them is that with __stdcall the callee cleans the stack whereas with __cdecl the caller cleans the stack.
It shouldn't be a problem as long as all function declarations use the same calling convention, which, I believe, is the case here.

We've spent some considerable effort of getting rid of the /export flags for Windows (and map files for unix), and I don't want to see them creep back in. Note that option 3 is just option 1 in disguise. The only single good thing about it is that it allows you to put the compiler option next to the signature declaration in the source code.

At least in this case, the option will be near the function body definition. It will be easier to update it if the signature changes.
If we are to use __stdcall, it seems to be the only way to export the undecorated name.

The same goes for def files, as suggested by Ali. It's just yet another version of option 1 in disguise/map files.

If option 2 is rejected, I'm for using option 3. If option 3 cannot be used too, I'm for option 1.
I think we should not fall back to def files in any case. And Makefile will have to include the reference to the def file, so it's even worse than option 1.
Regards,
Alexey
[1] https://bugs.openjdk.java.net/browse/JDK-8201226
[2] https://bugs.openjdk.java.net/browse/JDK-8202476

/Magnus

Post by Alexey Ivanov
Regards,
Alexey

Post by Magnus Ihse Bursie
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

Post by Magnus Ihse Bursie
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 by https://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
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 in https://github.com/AdoptOpenJDK/openjdk-jdk11u/blob/5f01925b80ed851b133ee26fbcb07026ac04149e/src/jdk.jdwp.agent/share/native/libjdwp/transport.c#L99.
Any thoughts?
Regards,
Ali

Post by Magnus Ihse Bursie
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
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 at https://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, \

Ali İnce

2018-12-06 22:49:04 UTC

Permalink

Hi Magnus, Alexey,

I believe we wonât be able to get further opinions from serviceability-dev. Andrew Luo suggested using a similar mechanism as is used for jvm.dll by using symbol name files mapped by platform (files are under make/hotspot/symbols and interestingly windows is almost the only platform for which a symbol file doesnât exist).

Another issue, again opened against AdoptOpenJDK 32-bit builds is somehow related with the same problem - but this time it is jimage.dll depending on zip.dll expecting the ZIP_InflateFully function to be decorated with JNICALL - whereas it was removed earlier from the implementation and the runtime image just fails with access violation just because jimage source code still declared it with JNICALL (https://github.com/AdoptOpenJDK/openjdk-build/issues/763 <https://github.com/AdoptOpenJDK/openjdk-build/issues/763>).

Iâve also searched for GetProcAddress calls across the source code - and I think itâs important to work on the consistency of JNICALL usages across the whole source code.

Another noteworthy thing Iâve noticed is there are still JNICALL modifiers for example in https://github.com/AdoptOpenJDK/openjdk-jdk11u/blob/master/src/jdk.crypto.cryptoki/windows/native/libj2pkcs11/j2secmod_md.c#L49 <https://github.com/AdoptOpenJDK/openjdk-jdk11u/blob/master/src/jdk.crypto.cryptoki/windows/native/libj2pkcs11/j2secmod_md.c#L49> - which I guess will also be reported as broken since these functions are again name decorated.

If weâre still determined to remove JNICALL, what about just removing __stdcall from JNICALL declaration at https://github.com/AdoptOpenJDK/openjdk-jdk11u/blob/master/src/java.base/windows/native/include/jni_md.h#L31 <https://github.com/AdoptOpenJDK/openjdk-jdk11u/blob/master/src/java.base/windows/native/include/jni_md.h#L31> ? Wouldnât that be a more consistent move?

Regards,

Ali

Post by Magnus Ihse Bursie
I think we are in complete agreement. What's missing is some expert opinion from serviceability-dev if there is any problem with changing the signature. I'd preferably have that.
If no one knows, I'd say, change it, and see if we still pass the relevant tests, and if so, ship it.
/Magnus

I'm not fond of either solution 1 or 3. I really think the signature should be made correctly at the point of definition in the source code.

That is why I proposed removing JNICALL (__stdcall) from the function declaration as we had done for libzip, libjimage [1] and mlib_image [2].
Microsoft recommends using __stdcall for DLLs, at the same time it decorates the function name making it harder to export it by its plain name.
By removing JNICALL / __stdcall, we make the function use __cdecl calling convention. The difference between them is that with __stdcall the callee cleans the stack whereas with __cdecl the caller cleans the stack.
It shouldn't be a problem as long as all function declarations use the same calling convention, which, I believe, is the case here.

At least in this case, the option will be near the function body definition. It will be easier to update it if the signature changes.
If we are to use __stdcall, it seems to be the only way to export the undecorated name.

The same goes for def files, as suggested by Ali. It's just yet another version of option 1 in disguise/map files.

If option 2 is rejected, I'm for using option 3. If option 3 cannot be used too, I'm for option 1.
I think we should not fall back to def files in any case. And Makefile will have to include the reference to the def file, so it's even worse than option 1.
Regards,
Alexey
[1] https://bugs.openjdk.java.net/browse/JDK-8201226 <https://bugs.openjdk.java.net/browse/JDK-8201226>
[2] https://bugs.openjdk.java.net/browse/JDK-8202476 <https://bugs.openjdk.java.net/browse/JDK-8202476>

/Magnus

Post by Alexey Ivanov
Regards,
Alexey

Post by Magnus Ihse Bursie
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

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 <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 in https://github.com/AdoptOpenJDK/openjdk-jdk11u/blob/5f01925b80ed851b133ee26fbcb07026ac04149e/src/jdk.jdwp.agent/share/native/libjdwp/transport.c#L99 <https://github.com/AdoptOpenJDK/openjdk-jdk11u/blob/5f01925b80ed851b133ee26fbcb07026ac04149e/src/jdk.jdwp.agent/share/native/libjdwp/transport.c#L99>.
Any thoughts?
Regards,
Ali

Hi,
An issue (https://github.com/AdoptOpenJDK/openjdk-build/issues/709 <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 at https://github.com/AdoptOpenJDK/openjdk-jdk11u/pull/1 <https://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, \

Alexey Ivanov

2018-12-07 20:24:29 UTC

Permalink

Hi Ali,

Post by Ali Ä°nce
Hi Magnus, Alexey,
I believe we wonât be able to get further opinions from
serviceability-dev.

Unfortunately, no one has replied as of now.
Have you found any issues with jdwpTransport_OnLoad after removing JNICALL?

Post by Ali Ä°nce
Andrew Luo suggested using a similar mechanism as is used for jvm.dll
by using symbol name files mapped by platform (files are under
make/hotspot/symbols and interestingly windows is almost the only
platform for which a symbol file doesnât exist).

Andrew says the .def files are auto-generated for Windows. There's a set
of shared functions.
JVM cannot change the calling convention, jvm.dll is the public
interface to JVM.

Post by Ali Ä°nce
Another issue, again opened against AdoptOpenJDK 32-bit builds is
somehow related with the same problem - but this time it is jimage.dll
depending on zip.dll expecting the ZIP_InflateFully function to be
decorated with JNICALL - whereas it was removed earlier from the
implementation and the runtime image just fails with access violation
just because jimage source code still declared it with JNICALL
(https://github.com/AdoptOpenJDK/openjdk-build/issues/763).

The usage of ZIP_InflateFully from zip.dll by jimage.dll was overlooked
during work on https://bugs.openjdk.java.net/browse/JDK-8201226.

I can take care of it.
(I could not build 32 bit Windows. It seem to have overcome the problem
by adding --disable-warnings-as-errors option to configure.)

However, the report says the resulting image crashes in 64 bit windows
too which shouldn't be affected by JNICALL mismatch.

Post by Ali Ä°nce
Iâve also searched for GetProcAddress calls across the source code -
and I think itâs important to work on the consistency of JNICALL
usages across the whole source code.

There are many places where libraries are loaded dynamically or a
function may be unavailable on older versions of the platform.
Have you identified any other place where functions from JDK DLLs are
looked up by names?

Post by Ali Ä°nce
Another noteworthy thing Iâve noticed is there are still JNICALL
modifiers for example in
https://github.com/AdoptOpenJDK/openjdk-jdk11u/blob/master/src/jdk.crypto.cryptoki/windows/native/libj2pkcs11/j2secmod_md.c#L49Â -
which I guess will also be reported as broken since these functions
are again name decorated.

This is a JNI method. It should use JNICALL modifier. If it wasn't
found, the class sun.security.pkcs11.Secmod would fail to load. I guess
JVM handles name mangling somehow for native method implementation.

Such functions were never explicitly exported using mapfiles or /export
options on Windows, at least in the client area. For Linux and Solaris,
adding or removing a native method required editing mapfiles.

Post by Ali Ä°nce
If weâre still determined to remove JNICALL, what about just removing
__stdcall from JNICALL declaration at
https://github.com/AdoptOpenJDK/openjdk-jdk11u/blob/master/src/java.base/windows/native/include/jni_md.h#L31Â ?Â Wouldnât
that be a more consistent move?

We can't do that.
Implementation of Java native methods must use __stdcall calling convention.

Post by Ali Ä°nce
Regards,
Ali

On 22 Nov 2018, at 17:29, Magnus Ihse Bursie
I think we are in complete agreement. What's missing is some expert
opinion from serviceability-dev if there is any problem with changing
the signature. I'd preferably have that.
If no one knows, I'd say, change it, and see if we still pass the
relevant tests, and if so, ship it.
/Magnus

Post by Alexey Ivanov

Post by Magnus Ihse Bursie

I'm not fond of either solution 1 or 3. I really think the
signature should be made correctly at the point of definition in
the source code.

That is why I proposed removing JNICALL (__stdcall) from the
function declaration as we had done for libzip, libjimage [1] and
mlib_image [2].
Microsoft recommends using __stdcall for DLLs, at the same time it
decorates the function name making it harder to export it by its plain name.
By removing JNICALL / __stdcall, we make the function use __cdecl
calling convention. The difference between them is that with
__stdcall the callee cleans the stack whereas with __cdecl the
caller cleans the stack.
It shouldn't be a problem as long as all function declarations use
the same calling convention, which, I believe, is the case here.

Post by Magnus Ihse Bursie
We've spent some considerable effort of getting rid of the /export
flags for Windows (and map files for unix), and I don't want to see
them creep back in. Note that option 3 is just option 1 in
disguise. The only single good thing about it is that it allows you
to put the compiler option next to the signature declaration in the
source code.

At least in this case, the option will be near the function body
definition. It will be easier to update it if the signature changes.
If we are to use __stdcall, it seems to be the only way to export the undecorated name.

Post by Magnus Ihse Bursie
The same goes for def files, as suggested by Ali. It's just yet
another version of option 1 in disguise/map files.

If option 2 is rejected, I'm for using option 3. If option 3 cannot
be used too, I'm for option 1.
I think we should not fall back to def files in any case. And
Makefile will have to include the reference to the def file, so it's
even worse than option 1.
Regards,
Alexey
[1] https://bugs.openjdk.java.net/browse/JDK-8201226
[2] https://bugs.openjdk.java.net/browse/JDK-8202476

Post by Magnus Ihse Bursie
/Magnus

Post by Alexey Ivanov
Regards,
Alexey

Post by Magnus Ihse Bursie
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

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

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

Magnus Ihse Bursie

2018-12-10 10:45:46 UTC

Permalink

Post by Magnus Ihse Bursie
Hi Ali,

Post by Ali Ä°nce
Hi Magnus, Alexey,
I believe we wonât be able to get further opinions from
serviceability-dev.

Unfortunately, no one has replied as of now.
Have you found any issues with jdwpTransport_OnLoad after removing JNICALL?

Andrew says the .def files are auto-generated for Windows. There's a
set of shared functions.
JVM cannot change the calling convention, jvm.dll is the public
interface to JVM.

Post by Ali Ä°nce
Another issue, again opened against AdoptOpenJDK 32-bit builds is
somehow related with the same problem - but this time it is
jimage.dll depending on zip.dll expecting the ZIP_InflateFully
function to be decorated with JNICALL - whereas it was removed
earlier from the implementation and the runtime image just fails with
access violation just because jimage source code still declared it
with JNICALL (https://github.com/AdoptOpenJDK/openjdk-build/issues/763).

The usage of ZIP_InflateFully from zip.dll by jimage.dll was
overlooked during work on
https://bugs.openjdk.java.net/browse/JDK-8201226.
I can take care of it.

It might be worthwhile to scan the entire code base for JNICALL and
verify that we have no more mismatches. In general, JNICALL should be
preserved on all officially supported, exported interfaces. It need not
be present on interfaces used only internally, and my current thinking
is that it would be better if it were removed on all internal
interfaces. But at the very least, it should be consistently specificed
where exported and imported. (Any misses here is probably due to
duplicate declarations, instead of properly including header files, so
that's the correct way to resolve any problems found...)

/Magnus

Post by Magnus Ihse Bursie
(I could not build 32 bit Windows. It seem to have overcome the
problem by adding --disable-warnings-as-errors option to configure.)
However, the report says the resulting image crashes in 64 bit windows
too which shouldn't be affected by JNICALL mismatch.

Post by Ali Ä°nce
Iâve also searched for GetProcAddress calls across the source code -
and I think itâs important to work on the consistency of JNICALL
usages across the whole source code.

Post by Ali Ä°nce
Another noteworthy thing Iâve noticed is there are still JNICALL
modifiers for example in
https://github.com/AdoptOpenJDK/openjdk-jdk11u/blob/master/src/jdk.crypto.cryptoki/windows/native/libj2pkcs11/j2secmod_md.c#L49 -
which I guess will also be reported as broken since these functions
are again name decorated.

This is a JNI method. It should use JNICALL modifier. If it wasn't
found, the class sun.security.pkcs11.Secmod would fail to load. I
guess JVM handles name mangling somehow for native method implementation.
Such functions were never explicitly exported using mapfiles or
/export options on Windows, at least in the client area. For Linux and
Solaris, adding or removing a native method required editing mapfiles.

We can't do that.
Implementation of Java native methods must use __stdcall calling convention.

Post by Ali Ä°nce
Regards,
Ali

On 22 Nov 2018, at 17:29, Magnus Ihse Bursie
I think we are in complete agreement. What's missing is some expert
opinion from serviceability-dev if there is any problem with
changing the signature. I'd preferably have that.
If no one knows, I'd say, change it, and see if we still pass the
relevant tests, and if so, ship it.
/Magnus

Post by Alexey Ivanov

Post by Magnus Ihse Bursie

I'm not fond of either solution 1 or 3. I really think the
signature should be made correctly at the point of definition in
the source code.

That is why I proposed removing JNICALL (__stdcall) from the
function declaration as we had done for libzip, libjimage [1] and
mlib_image [2].
Microsoft recommends using __stdcall for DLLs, at the same time it
decorates the function name making it harder to export it by its plain name.
By removing JNICALL / __stdcall, we make the function use __cdecl
calling convention. The difference between them is that with
__stdcall the callee cleans the stack whereas with __cdecl the
caller cleans the stack.
It shouldn't be a problem as long as all function declarations use
the same calling convention, which, I believe, is the case here.

Post by Magnus Ihse Bursie
We've spent some considerable effort of getting rid of the /export
flags for Windows (and map files for unix), and I don't want to
see them creep back in. Note that option 3 is just option 1 in
disguise. The only single good thing about it is that it allows
you to put the compiler option next to the signature declaration
in the source code.

At least in this case, the option will be near the function body
definition. It will be easier to update it if the signature changes.
If we are to use __stdcall, it seems to be the only way to export
the undecorated name.

Post by Magnus Ihse Bursie
The same goes for def files, as suggested by Ali. It's just yet
another version of option 1 in disguise/map files.

If option 2 is rejected, I'm for using option 3. If option 3 cannot
be used too, I'm for option 1.
I think we should not fall back to def files in any case. And
Makefile will have to include the reference to the def file, so
it's even worse than option 1.
Regards,
Alexey
[1] https://bugs.openjdk.java.net/browse/JDK-8201226
[2] https://bugs.openjdk.java.net/browse/JDK-8202476

Post by Magnus Ihse Bursie
/Magnus

Post by Alexey Ivanov
Regards,
Alexey

Post by Magnus Ihse Bursie
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

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

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

Magnus Ihse Bursie

2018-12-10 15:11:25 UTC

Permalink

Looking at the code, it seems (to me) the "correct" thing to do is to is
to create a Windows-specific version of dbgsysBuildFunName() in
linker_md.c, and have it decorate the function name as appropriate for
32-bit windows. Note that in transport.c, you'd need to pass in the
properly on 32 vs 64 bit Windows.
Notice this approach is already used for the library name, via
dbgsysBuildLibName()
If the function is never intended to be called by Java via JNI, then it
should never have been declared JNICALL in the first place - but that
ship has sailed.
I don't think changing the decorated exported name to undecorated is a
good idea, as it obfuscates the calling convention used.

Good analysis, Simon. I agree.
I have now looked more into the situation. In fact, it looks a lot
like JDWP has been broken on Windows 32-bit for a long time, but we
have patched around the issue in the JDK by using /export. This is
https://docs.oracle.com/javase/10/docs/specs/jdwp/jdwp-transport.html.

The transport library must export an/onload/function with the
|JNIEXPORT jint JNICALL jdwpTransport_OnLoad(JavaVM *jvm,
jdwpTransportCallback *callback, jint version, jdwpTransportEnv** env);|
This function will be called by the JDWP (or other agent) when the
library is loaded.

Nothing here says anything that the function should be exported using
anything other than the normal _stdcall (implied by JNICALL) name
mangling rules. However, JDWP has not been working according to the
spec and looking for the correct symbol, and while we could have
noticed that in the JDK, we didn't, because someone (long ago) added
/export:jdwpTransport_OnLoad as a workaround to the affected
libraries, instead of fixing JDWP.

This means that we cannot change the calling convention: it must stay
as it is now.
I think JDWP has always been working according to the spec. Using
__stdcall is the recommended calling convention for Win32 and for DLLs
in particular. Usually function names are exported undecorated in DLL.
(If my memory serves me well, older Microsoft tools exported |extern
"C" __stdcall| functions undecorated.)

So then the question becomes: what does the spec mean? That the
__stdcall convention should be used, or that the function name should be
explicitly exported under a non __stdcall-convention name? Neither
behavior seems clearly written out, so this is left to an implicit
interpretation of what that "usually" means..?

I believe this â exporting the undecorated function names â allows for
interoperability between 32 bit and 64 bit in cases where you load a
DLL and dynamically look up a function in it.
There were too many /export options to export Win32 functions
undecorated for this one to be an accident rather than the intention.

How do you mean?

Now, given that we've shipped code that didn't adhere to the specs
for so long, we can probably not break that behavior. Instead, I
propose we amend dbgsysBuildFunName() so that on Windows 32-bit, it
looks for both the "jdwpTransport_OnLoad", and the symbol as mangled
by _stdcall rules. I also propose that if both symbols are present,
the _stdcall named one should take precedence, since that's according
to the spec (and the other is just a fallback to maintain backwards
compatibility).

1. Add |/export| option to the Makefile or pragma-comment to the
source file;
fallback to undecorated one.

Yes.

I think the correct solution here is 2.

I just wonder how it's possible to implement a generic
|dbgsysBuildFunName| for an arbitrary function without knowing the
size of function parameters. Could it be the reason why most DLLs
export __stdcall functions undecorated?

(I assume you mean dbgsysFindLibraryEntry; there is a dbgsysBuildFunName
as well but it appears to be dead code.)

The dbgsysFindLibraryEntry does not need to work with an arbitrary
function. It's implemented in jdk.jdwp.agent, for the sole reason of
locating the jdwpTransport_OnLoad function.

In general, I assume this to hold true. If you don't know the signature
of the function you want to call, you're screwed anyway, regardless of
calling convention.

/Magnus

Regards,
Alexey

/Magnus

-Simon Tooke

Hi Andrew,
Sorry for my belated reply.
Yes, it's also an optionâŠ however, this solution seems to be
overcomplicated to export one function or two. The calling convention
of exported functions for JVM cannot be changed, they can be called
from third-party software.
If the function is used internally-only, its calling convention can be
changed as long as all components are updated.
Thank you for the pointer to another approach used to handle name
decorations of __stdcall functions. It looks we have to work out a
common approach to this problem.
Regards,
Alexey

By the way, in hotspot we are generating a .def file dynamically
while keeping the JNICALL calling convention (for symbols such as
JNI_CreateJavaVM) I believe (just by looking through the code in
http://hg.openjdk.java.net/jdk/jdk/file/44fe5fab538a/make/hotspot/lib/JvmMapfile.gmk,
unless this code is inactive - someone who knows this area better
than me might want to comment...). Shouldn't the same approach be
taken here as well?
Thanks
Andrew
-----Original Message-----
Behalf Of Ali Ince
Sent: Monday, November 19, 2018 2:08 PM
Subject: Re: [PATCH] Windows 32-bit DLL name decoration
Hi Alexey,
I donât have an account on JBS as Iâm not an author yet, my OCA has
just been processed. Would it be possible for someone with an author
status to create an issue?
Regards,
Ali

Post by Magnus Ihse Bursie
Hi Ali,
The fix looks good to me provided it resolves your problem.
I am not a reviewer so you'll have to get OK from reviewers, likely
from build-dev and from core-libs.
Have you submitted the issue in JBS?
https://openjdk.java.net/contribute/
and also
https://openjdk.java.net/sponsor/
Regards,
Alexey

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
# 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) {

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 in
https://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
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.h
tml
Regards,
Alexey

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 at
https://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, \

Continue reading on narkive:

Search results for '[PATCH] Windows 32-bit DLL name decoration' (Questions and Answers)

replies

i need to decorate my bedroom?

started 2008-07-27 20:52:43 UTC

fashion & accessories

replies

Mother's day gifts for a 'green' mother?

started 2008-02-26 10:54:13 UTC

green living