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.bate...@oracle.com]
Sent: Donnerstag, 12. April 2018 12:54
To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie
<magnus.ihse.bur...@oracle.com>
Cc: build-...@openjdk.java.net; Doerr, Martin <martin.do...@sap.com>;
core-libs-dev@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.bur...@oracle.com>
Cc: build-...@openjdk.java.net; Doerr, Martin <martin.do...@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.bur...@oracle.com]
Sent: Donnerstag, 5. April 2018 17:45
To: Baesken, Matthias
<matthias.baes...@sap.com<mailto:matthias.baes...@sap.com>>
Cc: build-...@openjdk.java.net<mailto:build-...@openjdk.java.net>;
Doerr, Martin <martin.do...@sap.com<mailto:martin.do...@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.baes...@sap.com<mailto:matthias.baes...@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



Reply via email to