> 10 apr. 2018 kl. 12:14 skrev Baesken, Matthias <matthias.baes...@sap.com>: > > Hello, I had to do another small adjustment to make jimage.hpp/cpp match. > Please review : > > http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/
Looks good to me. /Magnus > > With the latest webrev I could finally build jdk/jdk successfully on > both win32bit and win64 bit . > > > > Thanks again to Alexey to provide the incorporated patch . > > > Best regards, Matthias > > > >> -----Original Message----- >> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] >> Sent: Montag, 9. April 2018 17:14 >> To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie >> <magnus.ihse.bur...@oracle.com> >> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin >> <martin.do...@sap.com> >> Subject: Re: 8201226 missing JNIEXPORT / JNICALL at some places in function >> declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at >> some places in function declarations/implementations >> >> Hi Matthias, >> >>> On 09/04/2018 15:38, Baesken, Matthias wrote: >>> Hi Alexey, thanks for the diff provided by you, and for the >>> explanations >> . >>> >>> I created a second webrev : >>> >>> http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.1/ >>> >>> - it adds the diff provided by you (hope that’s fine with you) >> >> Yes, that's fine with me. >> There could be only one author ;) >> >>> - changes 2 launchers src/java.base/share/native/launcher/main.c >>> and >> src/jdk.pack/share/native/unpack200/main.cpp where we face similar >> issues after mapfile removal for exes >> >> I'd rather remove both JNIEXPORT and JNICALL from main(). >> It wasn't exported, and it shouldn't be. >> >> Regards, >> Alexey >> >>> >>> >>> >>> Best regards , Matthias >>> >>> >>>> -----Original Message----- >>>> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] >>>> Sent: Montag, 9. April 2018 15:52 >>>> To: Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com>; Baesken, >>>> Matthias <matthias.baes...@sap.com> >>>> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin >>>> <martin.do...@sap.com> >>>> Subject: Re: 8201226 missing JNIEXPORT / JNICALL at some places in >> function >>>> declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at >>>> some places in function declarations/implementations >>>> >>>> Hi Matthias, Magnus, >>>> >>>> The problem is with JNICALL added to the functions. JNICALL expands to >>>> __stdcall [1] on Windows. On 64 bit, the modifier has no effect and is >>>> ignored. On 32 bit, the modifier changes the way parameters are pass and >>>> the function name is decorated with an underscore and with @ + the size >>>> of arguments. >>>> >>>> If I remove JNICALL modifier from the exported functions, they're >>>> exported with plain name as in source code (plus, __cdecl [2] calling >>>> convention is used.) >>>> >>>> zip.dll and jimage.dll are affected by this. It's because the exported >>>> functions are looked up by name rather than using a header file with >>>> JNIIMPORT. See >>>> >> http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla >>>> ssLoader.cpp#l1155 >>>> >> http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla >>>> ssLoader.cpp#l1194 >>>> >>>> JNICALL modifier must also be removed from type declarations for >>>> functions from zip.dll: >>>> >> http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla >>>> ssLoader.cpp#l81 >>>> >>>> I'm attaching the patch to zip and jimage as well as classLoader.cpp >>>> which takes these changes into account. It does not replace Matthias' >>>> patch but complements it. >>>> >>>> >>>> Alternatively, if keeping JNICALL / __stdcall, it's possible to use >>>> pragma comments [3][4] to export undecorated names. But this is >> compiler >>>> specific and may require if's for other platforms. >>>> >>>> >>>> Regards, >>>> Alexey >>>> >>>> [1] https://msdn.microsoft.com/en-us/library/zxk0tw93.aspx >>>> [2] https://msdn.microsoft.com/en-us/library/zkwh89ks.aspx >>>> [3] https://docs.microsoft.com/en-ie/cpp/build/reference/exports >>>> [4] https://docs.microsoft.com/en-ie/cpp/preprocessor/comment-c-cpp >>>> >>>>> On 09/04/2018 12:42, Magnus Ihse Bursie wrote: >>>>> Those were added by my patch that removed the map files. >>>>> >>>>> /Magnus >>>>> >>>>>> 9 apr. 2018 kl. 13:38 skrev Baesken, Matthias >>>> <matthias.baes...@sap.com>: >>>>>> I did not add JNICALL decorations to any libzip functions , please >>>>>> see >>>> my webrev : >>>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8201226/ >>>>>> >>>>>>> , the problem here >>>>>>> is the added JNICALL decoration, which affects only win32 (and >>>> incorrectly, >>>>>>> that is). >>>>>> so I wonder which added JNICALL decoration you are refering to . >>>>>> >>>>>> Best regards, Matthias >>>>>> >>>>>> >>>>>>> -----Original Message----- >>>>>>> From: Magnus Ihse Bursie [mailto:magnus.ihse.bur...@oracle.com] >>>>>>> Sent: Montag, 9. April 2018 13:29 >>>>>>> To: Baesken, Matthias <matthias.baes...@sap.com> >>>>>>> Cc: Alexey Ivanov <alexey.iva...@oracle.com>; build-dev <build- >>>>>>> d...@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> >>>>>>> Subject: Re: 8201226 missing JNIEXPORT / JNICALL at some places in >>>> function >>>>>>> declarations/implementations - was : RE: missing JNIEXPORT / JNICALL >> at >>>>>>> some places in function declarations/implementations >>>>>>> >>>>>>> Reinstating the -export command line options is not the way forward >>>> here. >>>>>>> As I understood it from private conversation with Alexey, the problem >>>> here >>>>>>> is the added JNICALL decoration, which affects only win32 (and >>>> incorrectly, >>>>>>> that is). >>>>>>> >>>>>>> /Magnus >>>>>>> >>>>>>> >> <SNIP>
