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) - 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 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 > >>> > >>> 9 apr. 2018 kl. 12:46 skrev Baesken, Matthias > <matthias.baes...@sap.com>: > >>> > >>>>> I think adding prototype of ZIP_CRC32 to zip_util.h is unnecessary as > >>>>> this function is correctly decorated in CRC32.c and is exported. > >>>>> > >>>> Hi Alexey, you are correct on this one . The added declaration does > not > >>> help with the "Corrupted ZIP library" error . > >>>> This one can be fixed by bringing back the exports in the makefile > >>> make/lib/CoreLibraries.gmk (same for some JIMAGE functions) : > >>>> > >>>> --- a/make/lib/CoreLibraries.gmk Sun Apr 08 17:01:20 2018 +0800 > >>>> +++ b/make/lib/CoreLibraries.gmk Mon Apr 09 12:44:08 2018 +0200 > >>>> @@ -191,6 +191,9 @@ > >>>> DISABLED_WARNINGS_gcc := implicit-fallthrough, \ > >>>> LDFLAGS := $(LDFLAGS_JDKLIB) \ > >>>> $(call SET_SHARED_LIBRARY_ORIGIN), \ > >>>> + LDFLAGS_windows := -export:ZIP_Open -export:ZIP_Close - > >>> export:ZIP_FindEntry \ > >>>> + -export:ZIP_ReadEntry -export:ZIP_GetNextEntry \ > >>>> + -export:ZIP_InflateFully -export:ZIP_CRC32 - > export:ZIP_FreeEntry, \ > >>>> LIBS_unix := -ljvm -ljava $(LIBZ_LIBS), \ > >>>> LIBS_windows := jvm.lib $(WIN_JAVA_LIB), \ > >>>> )) > >>>> @@ -221,6 +224,10 @@ > >>>> CFLAGS_unix := -UDEBUG, \ > >>>> LDFLAGS := $(LDFLAGS_JDKLIB) $(LDFLAGS_CXX_JDK) \ > >>>> $(call SET_SHARED_LIBRARY_ORIGIN), \ > >>>> + LDFLAGS_windows := -export:JIMAGE_Open - > export:JIMAGE_Close \ > >>>> + -export:JIMAGE_PackageToModule \ > >>>> + -export:JIMAGE_FindResource -export:JIMAGE_GetResource \ > >>>> + -export:JIMAGE_ResourceIterator - > export:JIMAGE_ResourcePath, \ > >>>> LIBS_unix := -ljvm -ldl $(LIBCXX), \ > >>>> LIBS_macosx := -lc++, \ > >>>> LIBS_windows := jvm.lib, \ > >>>> > >>>> > >>>> I wonder why the 64bit windows build can live without the exports , > any > >>> ideas ? > >>>> > >>>> Best regards , Matthias > >>>> > >>>> > >>>>> -----Original Message----- > >>>>> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] > >>>>> Sent: Samstag, 7. April 2018 00:14 > >>>>> 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 Magnus, Matthias, > >>>>> > >>>>> I tried to build 32 bit Windows but it fails to run for me with > >>>>> Corrupted ZIP library: > >>>>> c:\work\jdk-dev\build\windows-x86-normal-server- > >>> release\jdk\bin\zip.dll > >>>>> The problem is that zip.dll exports all symbols as C++ rather than C. > >>>>> For example, ZIP_CRC32 is exported as _ZIP_CRC32@12, and > >>> classLoader.cpp > >>>>> cannot find the function. > >>>>> > >>>>> > >>>>> I think adding prototype of ZIP_CRC32 to zip_util.h is unnecessary as > >>>>> this function is correctly decorated in CRC32.c and is exported. > >>>>> > >>>>> Regards, > >>>>> Alexey > >>>>> > >>>>>> On 06/04/2018 17:33, Magnus Ihse Bursie wrote: > >>>>>> I think it's reasonable to update the existing webrev. > >>>>>> > >>>>>> /Magnus > >>>>>> > >>>>>>> 6 apr. 2018 kl. 15:20 skrev Baesken, Matthias > >>>>> <matthias.baes...@sap.com>: > >>>>>>> Hello, I just noticed 2 additonal issues regarding > >>>>>>> mapfile-removal : > >>>>>>> > >>>>>>> The follow up change that removed mapfiles for exes as well > leads > >>>>> on Win32 bit to this link error : > >>>>>>> Creating library > >>>>> C:/JVM/jdk_jdk_ntintel/support/native/java.base/java/java.lib and > >>> object > >>>>> C:/JVM/jdk_jdk_ntintel/support/native/java.base/java/java.exp > >>>>>>> LIBCMT.lib(crt0.obj) : error LNK2019: unresolved external symbol > _main > >>>>> referenced in function ___tmainCRTStartup > >>>>>>> > C:/JVM/jdk_jdk_ntintel/support/native/java.base/java_objs/java.exe > >>> : > >>>>> fatal error LNK1120: 1 unresolved externals > >>>>>>> > >>>>>>> Looks like we cannot have JNICALL in main.c : > >>>>>>> > >>>>>>> diff -r 4f6887eade94 src/java.base/share/native/launcher/main.c > >>>>>>> --- a/src/java.base/share/native/launcher/main.c Thu Apr 05 > >>> 14:39:04 > >>>>> 2018 -0700 > >>>>>>> +++ b/src/java.base/share/native/launcher/main.c Fri Apr 06 > >>> 15:16:40 > >>>>> 2018 +0200 > >>>>>>> @@ -93,7 +93,7 @@ > >>>>>>> __initenv = _environ; > >>>>>>> > >>>>>>> #else /* JAVAW */ > >>>>>>> -JNIEXPORT int JNICALL > >>>>>>> +JNIEXPORT int > >>>>>>> main(int argc, char **argv) > >>>>>>> { > >>>>>>> int margc; > >>>>>>> > >>>>>>> > >>>>>>> Zip-library has runtime issues when symbols are resolved in > >>>>> src/hotspot/share/classfile/classLoader.cpp. > >>>>>>> I added the declaration of the missing symbol, this seems to fix it . > >>>>>>> > >>>>>>> > >>>>>>> diff -r 4f6887eade94 src/java.base/share/native/libzip/zip_util.h > >>>>>>> --- a/src/java.base/share/native/libzip/zip_util.h Thu Apr 05 > 14:39:04 > >>>>> 2018 -0700 > >>>>>>> +++ b/src/java.base/share/native/libzip/zip_util.h Fri Apr 06 > 15:16:40 > >>>>> 2018 +0200 > >>>>>>> @@ -284,4 +284,7 @@ > >>>>>>> JNIEXPORT jboolean JNICALL > >>>>>>> ZIP_InflateFully(void *inBuf, jlong inLen, void *outBuf, jlong > outLen, > >>> char > >>>>> **pmsg); > >>>>>>> +JNIEXPORT jint JNICALL > >>>>>>> +ZIP_CRC32(jint crc, const jbyte *buf, jint len); > >>>>>>> + > >>>>>>> > >>>>>>> > >>>>>>> Should I prepare another bug, or add this to the existing one and > >>> post > >>>>> a second webrev ? > >>>>>>> Best regards, Matthias > >>>>>>> > >>>>>>> > >>>>>>> From: Baesken, Matthias > >>>>>>> Sent: Freitag, 6. April 2018 09:54 > >>>>>>> To: 'Magnus Ihse Bursie' <magnus.ihse.bur...@oracle.com> > >>>>>>> Cc: build-dev@openjdk.java.net; Doerr, Martin > >>> <martin.do...@sap.com> > >>>>>>> Subject: RFR: 8201226 missing JNIEXPORT / JNICALL at some places > in > >>>>> function declarations/implementations - was : RE: missing JNIEXPORT > / > >>>>> JNICALL at some places in function declarations/implementations > >>>>>>> Hello, please review : > >>>>>>> > >>>>>>> Bug : > >>>>>>> > >>>>>>> https://bugs.openjdk.java.net/browse/JDK-8201226 > >>>>>>> > >>>>>>> change : > >>>>>>> > >>>>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8201226/ > >>>>>>> > >>>>>>> mostly I added JNIEXPORT / JNICALL at some places where the > >>> win32bit > >>>>> build missed it . > >>>>>>> A difference is > >>>>> src/java.desktop/share/native/libsplashscreen/splashscreen_impl.h > >>>>>>> Where I removed a few declarations (one decl with one without > >>>>> JNIEXPORT / JNICALL – looked a bit strange ) . > >>>>>>> Best regards , Matthias > >>>>>>> > >>>>>>> > >>>>>>> From: Magnus Ihse Bursie > [mailto:magnus.ihse.bur...@oracle.com] > >>>>>>> Sent: Donnerstag, 5. April 2018 17:45 > >>>>>>> To: Baesken, Matthias <matthias.baes...@sap.com> > >>>>>>> Cc: build-dev@openjdk.java.net; Doerr, Martin > >>> <martin.do...@sap.com> > >>>>>>> Subject: Re: missing JNIEXPORT / JNICALL at some places in > function > >>>>> declarations/implementations > >>>>>>> That's most likely a result of the new JNIEXPORT I added as part of > the > >>>>> mapfile removal. > >>>>>>> I tried to match header file and C file, but I can certainly have > missed > >>>>> cases. If I didn't get any warnings, it was hard to know what I missed. > >>>>>>> Please do submit your patch. > >>>>>>> > >>>>>>> I'm a bit surprised 32-bit Window is still buildable. :) > >>>>>>> > >>>>>>> /Magnus > >>>>>>> > >>>>>>> 5 apr. 2018 kl. 17:20 skrev Baesken, Matthias > >>>>> <matthias.baes...@sap.com>: > >>>>>>> Hello, we noticed that at a number of places in the coding , the > >>>>> JNIEXPORT and/or JNICALL modifiers do not match when one > compares > >>>>> the declaration and > >>>>>>> The implementation of functions. > >>>>>>> While this is ok on most platforms, it seems to fail on Windows 32 > bit > >>> and > >>>>> leads to errors like this one : > >>>>>>> > >>> > e:/priv/openjdk/repos/jdk/src/java.desktop/share/native/libmlib_image/ml > >>>>> ib_ImageConvKernelConvert.c(87) : error C2373: > >>>>> 'j2d_mlib_ImageConvKernelConvert' : redefinition; different type > >>> modifiers > >>> > e:\priv\openjdk\repos\jdk\src\java.desktop\share\native\libmlib_image\ml > >>>>> ib_image_proto.h(2630) : see declaration of > >>>>> 'j2d_mlib_ImageConvKernelConvert' > >>>>>>> (there are quite a few of these e.g. in mlib / splashscreen etc.) > >>>>>>> > >>>>>>> > >>>>>>> Another example is this one : > >>>>>>> > >>>>>>> diff -r 4d98473ed33e > src/java.base/share/native/libjimage/jimage.hpp > >>>>>>> --- a/src/java.base/share/native/libjimage/jimage.hpp Thu Apr 05 > >>>>> 09:55:16 2018 +0200 > >>>>>>> +++ b/src/java.base/share/native/libjimage/jimage.hpp > >>>>>>> Thu > Apr > >>>>> 05 17:07:40 2018 +0200 > >>>>>>> @@ -126,7 +126,7 @@ > >>>>>>> * JImageLocationRef location = (*JImageFindResource)(image, > >>>>>>> * "java.base", "9.0", > >>>>>>> "java/lang/String.class", > &size); > >>>>>>> */ > >>>>>>> -extern "C" JNIEXPORT JImageLocationRef > >>>>> JIMAGE_FindResource(JImageFile* jimage, > >>>>>>> +extern "C" JNIEXPORT JImageLocationRef JNICALL > >>>>> JIMAGE_FindResource(JImageFile* jimage, > >>>>>>> const char* module_name, const char* version, const char* > name, > >>>>>>> jlong* size); > >>>>>>> > >>>>>>> > >>>>>>> Is there some generic way to get the same declarations / > >>>>> impementations in the code ? > >>>>>>> Or should I just add a patch with my findings ? > >>>>>>> > >>>>>>> Best regards, Matthias > >>>>>>>
