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

[Magnus Ihse Bursie]

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