Hi Matthias, thank you for providing the fix. I think these issues should really get fixed regardless if 32 bit is supported or not. It's not good to have inconsistent JNI declarations.
The fix looks good to me. I guess it should get pushed to the submission forest because it contains hotspot changes. Or did Oracle run the necessary tests? Best regards, Martin -----Original Message----- From: Baesken, Matthias Sent: Dienstag, 10. April 2018 12:15 To: Alexey Ivanov <alexey.iva...@oracle.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 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/ 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>
