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

On 27/11/2018 18:49, Andrew Luo wrote:
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-----
From: core-libs-dev <core-libs-dev-boun...@openjdk.java.net> On Behalf Of Ali 
Ince
Sent: Monday, November 19, 2018 2:08 PM
To: Alexey Ivanov <alexey.iva...@oracle.com>; build-dev <build-dev@openjdk.java.net>; 
core-libs <core-libs-...@openjdk.java.net>
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

On 19 Nov 2018, at 12:12, Alexey Ivanov <alexey.iva...@oracle.com> wrote:

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?

You have to sign OCA to be able to contribute to OpenJDK:
https://openjdk.java.net/contribute/
and also
https://openjdk.java.net/sponsor/

Regards,
Alexey

On 18/11/2018 12: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 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

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 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 <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.h
tml

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

Reply via email to