8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
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/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.deskto
> p/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 core-libs-dev
mailing list