8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations

Alexey Ivanov alexey.ivanov at oracle.com
Thu Apr 12 21:53:10 UTC 2018 

On 12/04/2018 21:42, Phil Race wrote:
> How can JNIEXPORT be different between 32 bit & 64 bit ?
> I'm sure you saw compilation errors but I don't get why it failed for 
> 32 only.
>
> JNICALL (_stdcall) may be unnecessary on 64 bit Windows but that doesn't
> explain why the 32 bit compiler would complain about inconsistent 
> application
> of __declspec(dllexport) - ie JNIEXPORT.
>
> Or is that part (adding JNIEXPORT) pure clean up and the compilation 
> errors were all down to JNICALL ?

Adding missing JNIEXPORT is for cleanup only.

The compiler complained about mismatched JNICALL / non-JNICALL 
declarations as the macro changes calling convention from the default 
__cdecl  to __stdcall on 32 bit Windows.

Another issue is that __stdcall decorates the functions: prefixes with 
underscore and postfixes with @ + size of parameters. Because of the 
decorations, classLoader.cpp can't lookup the required functions by name 
from zip.dll and jimage.dll. The functions are exported but with 
different name.

I hope this information adds more details to the picture.

> I was a bit puzzled at the removals I saw here :
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/src/java.desktop/share/native/libsplashscreen/splashscreen_impl.h.udiff.html 
>
>
> .. I needed to look at the whole file to realise that you were 
> removing a duplicate
> declaration.

That was tricky. I could have been mentioned in the review.


Regards,
Alexey

>
> -phil.
>
> On 04/12/2018 04:04 AM, Baesken, Matthias wrote:
>> Hi  Alan , this is the up to date webrev .
>> However we want to add   Alexey   Ivanov  as additional  author .
>>
>>> As I read it, this changes the calling convention of these functions on
>>> 32-bit Windows but it will have no impact on 64-bit Windows (as
>>> __stdcall is ignored) or other platforms, is that correct?
>>>
>> The  change adds  JNIEXPORT   at some places  where it is  ben 
>> forgotten , for example :
>>
>> http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/src/java.desktop/share/native/libmlib_image/mlib_c_ImageLookUp.c.udiff.html 
>>
>>
>> This might have  potential  impact  on other platforms   (fixes the 
>> mismatches) .
>>
>> Best regards, Matthias
>>
>>
>>
>>
>>> -----Original Message-----
>>> From: Alan Bateman [mailto:Alan.Bateman at oracle.com]
>>> Sent: Donnerstag, 12. April 2018 12:54
>>> To: Baesken, Matthias <matthias.baesken at sap.com>; Magnus Ihse Bursie
>>> <magnus.ihse.bursie at oracle.com>
>>> Cc: build-dev at openjdk.java.net; Doerr, Martin <martin.doerr at sap.com>;
>>> core-libs-dev at openjdk.java.net
>>> 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
>>>
>>> Adding core-libs-dev as this is change code in libjava, libzip,
>>> libjimage, ...
>>>
>>> Can you confirm that this is the up to date webrev:
>>>      http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/
>>>
>>> As I read it, this changes the calling convention of these functions on
>>> 32-bit Windows but it will have no impact on 64-bit Windows (as
>>> __stdcall is ignored) or other platforms, is that correct?
>>>
>>> -Alan
>>>
>>>
>>> On 06/04/2018 14:20, Baesken, Matthias wrote:
>>>> Hello, I just noticed  2  additonal issues  regarding 
>>>> mapfile-removal :
>>>>
>>>>
>>>>     1.  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;
>>>>
>>>>
>>>>
>>>>     1.  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<mailto:matthias.baesken at sap.com>>
>>>> Cc: build-dev at openjdk.java.net<mailto:build-dev at openjdk.java.net>;
>>> Doerr, Martin <martin.doerr at sap.com<mailto: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<mailto: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