[PATCH] Windows 32-bit DLL name decoration

[Magnus Ihse Bursie]

On 2018-12-12 16:34, Alexey Ivanov wrote:
>
>
> On 12/12/2018 12:58, Magnus Ihse Bursie wrote:
>>
>>
>> On 2018-12-12 12:35, Alan Bateman wrote:
>>> On 12/12/2018 11:14, Magnus Ihse Bursie wrote:
>>>> :
>>>>
>>>> I searched the code for "jdwpTransport_On" to see of there was any 
>>>> corresponding OnUnload function (or other), but could not find any.
>>> That's right, an unload wasn't needed when that SPI was originally 
>>> added but could be added in the event that some future debugger 
>>> agent need it. The SPI is a supported/documented JDK-specific 
>>> mechanism, its "spec" is hosted here:
>>>
>>> https://docs.oracle.com/en/java/javase/11/docs/specs/jdwp/jdwp-transport.html 
>>>
>> ... yet in all that time, we have not fully supported the spec on 
>> Windows 32. :-( (We never discovered this because of lack of testing, 
>> I presume, and that our internal usage empoyed a questonable 
>> workaround.)
>
> The spec does not specify whether the mangled name should be exported 
> or not (for Windows 32 bit).
>
> I looked through JNI specification and have found no mention of 
> JNIEXPORT or JNICALL.
> https://docs.oracle.com/en/java/javase/11/docs/specs/jni/index.html
>
> This example uses extern "C" modifier but neither JNIEXPORT or JNICALL:
> https://docs.oracle.com/en/java/javase/11/docs/specs/jni/design.html#native-method-arguments 
>
I'm not sure if it's worth continuing this discussion, but at the very 
least the specs have been not very clear on this. The links you cite 
point to the JNI specification, not JDWP. I don't bother go looking for 
the exact place in the JNI spec, but I assume they state that you must 
implement the header file as generated by javac (formerly javah). These 
do look like this:

* DO NOT EDIT THIS FILE - it is machine generated */
#include <jni.h>
/* Header for class sun_nio_ch_FileKey */

#ifndef _Included_sun_nio_ch_FileKey
#define _Included_sun_nio_ch_FileKey
#ifdef __cplusplus
extern "C" {
#endif
/*
  * Class:     sun_nio_ch_FileKey
  * Method:    init
  * Signature: (Ljava/io/FileDescriptor;)V
  */
JNIEXPORT void JNICALL Java_sun_nio_ch_FileKey_init
   (JNIEnv *, jobject, jobject);

So at least implicitly, they say that you need to use JNIEXPORT and 
JNICALL. And there is no /export thingy here. And no instructions in the 
specs to do a specific export; if it were, we would have needed it for 
all the hundereds of JNI functions in the JDK. So obviously the JNI code 
reads decorated names on Windows 32.

Then again, it is not clear that the spec for JNI should apply to JDWP. 
But, if they state that you must use the JNICALL, and this will, unless 
other actions are taken, such as using /export, by default create a 
decorated name, I think it's very clear that *if* this indeed was 
intended to be the formal interface, it *should* have been explicitly 
written in the spec.

/Magnus


>
>>
>>>
>>> I see this thread is touching on the functions exported by libjli. 
>>> This library is part of the launcher and shouldn't be used directly 
>>> by anything outside of the JDK. However we have to be careful 
>>> because JavaFX `javapackager` tool has been using it. There's a JEP 
>>> to bring a similar tool, jpackage in the current proposal, that will 
>>> supersede it but in the mean time we need to be careful not to break 
>>> it. A second issue is that the libjli is listed in the property list 
>>> (Info.plist) on macOS. This came from Apple in 7u4 and periodically 
>>> things show up that are using it, e.g. that recent breakage with 
>>> Eclipse that was never fully diagnosed but we think it was using 
>>> Info.plist.
>> The latest patch, as suggested, will not modify JLI, but instead fix 
>> the broken behaviour of JDWP on Windows 32.
>
> Yes, that's right.
> However, I believe the previous versions did not modify libjli.
>
> Regards,
> Alexey
>
>>
>> /Magnus
>