8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
Baesken, Matthias
matthias.baesken at sap.com
Mon Apr 9 11:38:38 UTC 2018
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.bursie at oracle.com]
> Sent: Montag, 9. April 2018 13:29
> To: Baesken, Matthias <matthias.baesken at sap.com>
> Cc: Alexey Ivanov <alexey.ivanov at oracle.com>; build-dev <build-
> dev at openjdk.java.net>; Doerr, Martin <martin.doerr at 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.baesken at 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.ivanov at oracle.com]
> >> Sent: Samstag, 7. April 2018 00:14
> >> To: Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com>; Baesken,
> >> Matthias <matthias.baesken at sap.com>
> >> Cc: build-dev <build-dev at openjdk.java.net>; Doerr, Martin
> >> <martin.doerr at 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 at 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.baesken at 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.bursie at oracle.com>
> >>>> Cc: build-dev at openjdk.java.net; Doerr, Martin
> <martin.doerr at 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.bursie at oracle.com]
> >>>> Sent: Donnerstag, 5. April 2018 17:45
> >>>> To: Baesken, Matthias <matthias.baesken at sap.com>
> >>>> Cc: build-dev at openjdk.java.net; Doerr, Martin
> <martin.doerr at 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.baesken at 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
> >>>>
> >
More information about the build-dev
mailing list