8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
Magnus Ihse Bursie
magnus.ihse.bursie at oracle.com
Mon Apr 9 11:47:21 UTC 2018
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.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