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

Thu Apr 12 20:42:59 UTC 2018

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 ?

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.

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