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 14:38:01 UTC 2018
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.ivanov at oracle.com]
> Sent: Montag, 9. April 2018 15:52
> 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 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.baesken at 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.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