[PATCH] Windows 32-bit DLL name decoration

[Alexey Ivanov]

On 12/12/2018 16:52, Magnus Ihse Bursie wrote:
> 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.

I absolutely agree it's not worth it.
It does not solve the problem at hand; it does not solve any problem at all.
So let's leave it here.

--
Alexey

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