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 > >>>> > >
