[PATCH] Windows 32-bit DLL name decoration (original) (raw)

Ali İnce ali.ince at gmail.com
Mon Dec 10 21:05:28 UTC 2018

On 10 Dec 2018, at 15:11, Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com> wrote:

On 2018-12-10 16:02, Alexey Ivanov wrote:

On 10/12/2018 10:41, Magnus Ihse Bursie wrote:

On 2018-12-07 22:18, Simon Tooke wrote: 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 linkermd.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 parameter stack size (the bit after the @), but it could at least behave 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 the spec we're dealing with: https://docs.oracle.com/javase/10/docs/specs/jdwp/jdwp-transport.html. I quote: The transport library must export an onload function with the following prototype: JNIEXPORT jint JNICALL jdwpTransportOnLoad(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:jdwpTransportOnLoad 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..?

A couple of MSFT quotes on name decoration from https://docs.microsoft.com/en-gb/cpp/build/reference/decorated-names?view=vs-2017 says;

Normally, you don't have to know the decorated name to write code that compiles and links successfully. Decorated names are an implementation detail internal to the compiler and linker.

Name decoration is also important when linking to code written in other programming languages or using other compilers. Different compilers use different name decoration conventions. When your executable links to code written in another language, special care must be taken to match the exported and imported names and calling conventions.

And we should also keep in mind that MSFT itself doesn’t use name mangling in the core windows API.

I still see this |jdwpTransport_OnLoad| not much different than |JNI_CreateJavaVM| in the context of it’s availability and accessibility from outer world.

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 "jdwpTransportOnLoad", 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). Since removing JNICALL is not an option, there are only two options: 1. Add /export option to the Makefile or pragma-comment to the source file; 2. Lookup the decorated name jdwpTransportOnLoad at 16 for Win32 with fallback to undecorated one. Yes. I think the correct solution here is 2.

Does keeping name mangling in-place gives anyone any value? It would probably only add the overhead of making the callers compute the stack size of the parameters. And I couldn’t find any sign or history of changes but name mangling, theoretically, it could change over versions of compilers.

The pragma-comment approach could also be overly simplified as shown https://stackoverflow.com/a/41910450, which will leave us with a single line addition to the function body as;

#pragma EXPORT

which is from my point of view a simpler (and maintainable) approach than adding a decorated name lookup logic.

Regards,

Ali

_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 jdwpTransportOnLoad 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

On 12/7/2018 2:09 PM, Alexey Ivanov wrote: 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 JNICreateJavaVM) 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-bounces at openjdk.java.net> On Behalf Of Ali Ince Sent: Monday, November 19, 2018 2:08 PM To: Alexey Ivanov <alexey.ivanov at oracle.com> ; build-dev <build-dev at openjdk.java.net>; core-libs <core-libs-dev at 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.ivanov at 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.ince at 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/libdtshmem/shmemBack.c --- a/src/jdk.jdi/share/native/libdtshmem/shmemBack.c Tue Aug 14 14:28:23 2018 +0200 +++ b/src/jdk.jdi/share/native/libdtshmem/shmemBack.c Sun Nov 18 12:00:15 2018 +0000 @@ -339,7 +339,7 @@ return JDWPTRANSPORTERRORNONE; } -JNIEXPORT jint JNICALL +JNIEXPORT jint jdwpTransportOnLoad(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 *jdwpTransportOnLoadt)(JavaVM *jvm, +typedef jint (*jdwpTransportOnLoadt)(JavaVM *jvm, jdwpTransportCallback *callback, jint version, jdwpTransportEnv** env); diff -r 16d5ec7dbbb4 -r a41df44e13e1 src/jdk.jdwp.agent/share/native/libdtsocket/socketTransport.c --- a/src/jdk.jdwp.agent/share/native/libdtsocket/socketTransport.c Tue Aug 14 14:28:23 2018 +0200 +++ b/src/jdk.jdwp.agent/share/native/libdtsocket/socketTransport.c Sun Nov 18 12:00:15 2018 +0000 @@ -1019,7 +1019,7 @@ return JDWPTRANSPORTERRORNONE; } -JNIEXPORT jint JNICALL +JNIEXPORT jint jdwpTransportOnLoad(JavaVM vm, jdwpTransportCallback cbTablePtr, jint version, jdwpTransportEnv** env) {

On 16 Nov 2018, at 23:03, Alexey Ivanov <alexey.ivanov at 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 jdwpTransportOnLoad exported functions both for dtsocket.dll and dtshmem.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.ivanov at 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 dtsocket.dll and dtshmem.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 ($(OPENJDKTARGETOS), windows) _jdk.jdwp.agent:include _ _jdk.jdwp.agent:libjdwp/export, _ _LDFLAGS := $(LDFLAGSJDKLIB), _ _+ LDFLAGSwindows := -export:jdwpTransportOnLoad, _ _LIBS := $(JDKLIBLIBS), _ )) 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(eval (eval(call SetupJdkLibrary, _BUILDLIBDTSOCKET, _ _libjdwp/export, _ _LDFLAGS := $(LDFLAGSJDKLIB) _ _$(call SETSHAREDLIBRARYORIGIN), _ _+ LDFLAGSwindows := -export:jdwpTransportOnLoad, _ _LIBSlinux := -lpthread, _ _LIBSsolaris := -lnsl -lsocket, _ _LIBSwindows := $(JDKLIBLIBS) ws232.lib, _

More information about the build-dev mailing list