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

Daniel D. Daugherty daniel.daugherty at oracle.com
Wed Dec 19 15:05:42 UTC 2018

On 12/19/18 9:23 AM, Alexey Ivanov wrote:

Any volunteers from core-libs and serviceability to review?

How about former Serviceability Team members? :-) Alan B. and I both used to be on the Serviceability Team... And Alan B. is a current member of Core Libs...

webrev: http://cr.openjdk.java.net/~aivanov/8214122/webrev.01/

src/jdk.jdwp.agent/share/native/libjdwp/transport.c     No comments.

The code appears to do what is described in this (very long) review thread. I don't believe we (Oracle) have any Win32 test setups for the jdk/jdk repo so this isn't a change that someone from Oracle can do additional testing on.

Thumbs up!

Dan

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 jdwpTransportOnLoad is exported as jdwpTransportOnLoad 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, jdwpTransportOnLoad at 16, first; if not found, look up the undecorated name, jdwpTransportOnLoad.

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 jdwpTransportOnLoad 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 = (jdwpTransportOnLoadt) dbgsysFindLibraryEntry(handle, "jdwpTransportOnLoad at 16"); #else onLoad = (jdwpTransportOnLoadt) dbgsysFindLibraryEntry(handle, "jdwpTransportOnLoad"); #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 |jdwpTransportOnLoad at 16| for Win32 with fallback to undecorated one. Yes. I think the correct solution here is 2.

More information about the build-dev mailing list