[PATCH] Windows 32-bit DLL name decoration

[Alexey Ivanov]

Hi Simon,

I'd like to clarify some points…
Please see my comments inline:

On 11/12/2018 17:16, Alexey Ivanov wrote:
> Hi Simon,
>
> Thank you for your comments.
>
> The updated webrev:
> http://cr.openjdk.java.net/~aivanov/8214122/webrev.01/
>
> Indeed, it looks much cleaner.
>
> Regards,
> Alexey
>
> On 11/12/2018 16:43, Simon Tooke wrote:
>> On 2018-12-11 10:05 a.m., Alexey Ivanov wrote:
>>> Hi everyone,
>>>
>>> I came up with the following patch:
>>> http://cr.openjdk.java.net/~aivanov/8214122/webrev.00/
>>>
>>> It specifically addresses the problem in JDK-8214122 where on 32 bit
>>> Windows jdwpTransport_OnLoad can exported with its plain and
>>> __stdcall-mangled name. I used conditional compilation so that for
>>> other platforms the code remains as it is now.
>>>
>>> jshell starts successfully with this fix; an IDE debugger works as 
>>> well.
>>>
>> I am not a reviewer, but this patch only works for the specific case
>> under discussion; the '@16' refers to the reserved stack size in the
>> Pascal calling convention.  So, the patch only works for 16 bytes of
>> parameters.  To be generic, the routine needs to have the stack size
>> passed in by the caller.  If a generic fix isn't desired (and I hope it
>> is), I'd prefer to see the caller simply pass the decorated or
>> undecorated name depending on the Win32/64 defines.

At this time, it's only one function. So the current approach works well 
enough, although it will not scale.

If other similar functions are added in the future, _OnUnload for 
example, we should implement approach used in Hotspot as suggested by Bob:
http://mail.openjdk.java.net/pipermail/build-dev/2018-December/024373.html

>>
>>
>>      #if defined(_WIN32) && !defined(_WIN64) onLoad =
>>      (jdwpTransport_OnLoad_t) dbgsysFindLibraryEntry(handle,
>>      "_jdwpTransport_OnLoad at 16"); #else onLoad = 
>> (jdwpTransport_OnLoad_t)
>>      dbgsysFindLibraryEntry(handle, "jdwpTransport_OnLoad"); #endif

I believe falling back to undecorated name is the right thing in the 
case where the decorated name isn't found. Only the undecorated name has 
been looked up before. If a (third-party) JDWP transport DLL exports the 
undecorated name only, it will fail to load. With fallback in place, it 
will load successfully.

Does it sound reasonable?


Regards,
Alexey

>>
>>
>> Thanks,
>> -Simon
>>
>>> Regards,
>>> Alexey
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8214122
>>>
>>> On 10/12/2018 15:11, Magnus Ihse Bursie wrote:
>>>>> 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 |_jdwpTransport_OnLoad at 16| for Win32
>>>>> with fallback to undecorated one.
>>>> Yes.
>>>>
>>>> I think the correct solution here is 2.
>