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

Baesken, Matthias matthias.baesken at sap.com
Fri Apr 13 05:48:23 UTC 2018

Hi Phil/Alexey, thanks for adding the other lists .

Is this the current version of the change : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/ ?

Yes.

Best regards, Matthias

-----Original Message----- From: Alexey Ivanov [mailto:alexey.ivanov at oracle.com] Sent: Donnerstag, 12. April 2018 23:53 To: Phil Race <philip.race at oracle.com>; Baesken, Matthias <matthias.baesken at sap.com>; Alan Bateman <Alan.Bateman at oracle.com>; Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com> Cc: build-dev at openjdk.java.net; core-libs-dev at openjdk.java.net; Doerr, Martin <martin.doerr at sap.com>; 2d-dev <2d-dev at openjdk.java.net>; hotspot-dev <hotspot-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

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.deskto p/share/native/libsplashscreen/splashscreenimpl.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.deskto p/share/native/libmlibimage/mlibcImageLookUp.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/jdkjdkntintel/support/native/java.base/java/java.lib and >>> object >>> C:/JVM/jdkjdkntintel/support/native/java.base/java/java.exp >>>> LIBCMT.lib(crt0.obj) : error LNK2019: unresolved external symbol main _>>> referenced in function tmainCRTStartup >>>> C:/JVM/jdkjdkntintel/support/native/java.base/javaobjs/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/ziputil.h >>>> --- a/src/java.base/share/native/libzip/ziputil.h      Thu Apr 05 >>>> 14:39:04 >>> 2018 -0700 >>>> +++ b/src/java.base/share/native/libzip/ziputil.h      Fri Apr 06 >>>> 15:16:40 >>> 2018 +0200 >>>> @@ -284,4 +284,7 @@ >>>> JNIEXPORT jboolean JNICALL >>>> ZIPInflateFully(void *inBuf, jlong inLen, void *outBuf, jlong >>>> outLen, char >>> **pmsg); >>>> +JNIEXPORT jint JNICALL >>>> +ZIPCRC32(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/splashscreenimpl.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/libmlibimage/ml >>> >>> ibImageConvKernelConvert.c(87) : error C2373: >>> 'j2dmlibImageConvKernelConvert' : redefinition; different type >>> modifiers >>> e:\priv\openjdk\repos\jdk\src\java.desktop\share\native\libmlibimage\ml >>> >>> ibimageproto.h(2630) : see declaration of >>> 'j2dmlibImageConvKernelConvert' >>>> (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 >>> JIMAGEFindResource(JImageFile* jimage, >>>> +extern "C" JNIEXPORT JImageLocationRef JNICALL >>> JIMAGEFindResource(JImageFile* jimage, >>>>            const char* modulename, 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 hotspot-dev mailing list