[PATCH] Windows 32-bit DLL name decoration
alexey.ivanov at oracle.com
Wed Dec 12 16:02:33 UTC 2018
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:
> Indeed, it looks much cleaner.
> 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:
>>> 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
>> 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:
>> #if defined(_WIN32) && !defined(_WIN64) onLoad =
>> (jdwpTransport_OnLoad_t) dbgsysFindLibraryEntry(handle,
>> "_jdwpTransport_OnLoad at 16"); #else onLoad =
>> 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?
>>> 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.
>>>> I think the correct solution here is 2.
More information about the core-libs-dev