Also, I'm not sure if I said it, but I think your original webrev looks good.
/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 >>>>>> >>> >