[12] RFR for JDK-8214122: JDWP is broken on 32 bit Windows: transport library missing onLoad entry

[Alexey Ivanov]

Any volunteers from core-libs and serviceability to review?

Regards,
Alexey

On 12/12/2018 16:43, Magnus Ihse Bursie wrote:
>
> On 2018-12-12 17:38, Alexey Ivanov wrote:
>> Hi all,
>>
>> I have updated the summary of JDK-8214122 and the subject accordingly.
>>
>> Could you please review the following fix?
>>
>> https://bugs.openjdk.java.net/browse/JDK-8214122
>> webrev: http://cr.openjdk.java.net/~aivanov/8214122/webrev.01/
> Looks good to me.
>
> /Magnus
>>
>> *Root cause*
>> jdwpTransport_OnLoad is exported as _jdwpTransport_OnLoad at 16 on 32 
>> bit Windows according to the name decorations [1] for __stdcall [2] 
>> calling conventions.
>>
>> *Fix*
>> On 32 bit Windows, look up the decorated name, 
>> _jdwpTransport_OnLoad at 16, first; if not found, look up the 
>> undecorated name, jdwpTransport_OnLoad.
>>
>>
>> Regards,
>> Alexey
>>
>> [1] 
>> https://docs.microsoft.com/en-us/cpp/build/reference/decorated-names?view=vs-2017#FormatC
>> [2] https://docs.microsoft.com/en-ie/cpp/cpp/stdcall?view=vs-2017
>>
>> On 12/12/2018 11:19, Magnus Ihse Bursie wrote:
>>>
>>>
>>> On 2018-12-11 18: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.
>>> Yes! This seems like the correct fix.
>>>
>>> Ship it! :)
>>>
>>> /Magnus
>>>
>>>>
>>>> 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.
>>>>>
>>>>>      #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
>>>>>
>>>>>
>>>>> 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.
>>>>
>>>
>>
>