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 2018-04-16 14:59, Alexey Ivanov wrote: Hi Matthias, Phil, The build of 32 bit Windows is broken because of mlib_image.dll. As JNICALL modifier has been added to function declarations, they're exported with a decorated name, for example _j2d_mlib_ImageCreate@16. The functions in this library are looked up by their name [1] and therefore none can be found. You should most likely just be able to remove the JNICALL modifiers for libmlib_image. /Magnus If you run tests in test/jdk/java/awt/image, for example test/jdk/java/awt/image/mlib/MlibOpsTest.java, some of them fail because ImagingLib is not available. I'm working on a patch to fix it. Regards, Alexey [1] http://hg.openjdk.java.net/jdk/jdk/file/bc1c7e41e285/src/java.desktop/windows/native/libawt/windows/awt_Mlib.cpp#l60 On 13/04/2018 06:48, Baesken, Matthias wrote: 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.iva...@oracle.com] Sent: Donnerstag, 12. April 2018 23:53 To: Phil Race <philip.r...@oracle.com>; Baesken, Matthias <matthias.baes...@sap.com>; Alan Bateman <alan.bate...@oracle.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev@openjdk.java.net; core-libs-...@openjdk.java.net; Doerr, Martin <martin.do...@sap.com>; 2d-dev <2d-...@openjdk.java.net>; hotspot-dev <hotspot-...@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.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-dev@openjdk.java.net; Doerr, Martin <martin.do...@sap.com>; core-libs-...@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
Re: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
Hi Matthias, Phil, The build of 32 bit Windows is broken because of mlib_image.dll. As JNICALL modifier has been added to function declarations, they're exported with a decorated name, for example _j2d_mlib_ImageCreate@16. The functions in this library are looked up by their name [1] and therefore none can be found. If you run tests in test/jdk/java/awt/image, for example test/jdk/java/awt/image/mlib/MlibOpsTest.java, some of them fail because ImagingLib is not available. I'm working on a patch to fix it. Regards, Alexey [1] http://hg.openjdk.java.net/jdk/jdk/file/bc1c7e41e285/src/java.desktop/windows/native/libawt/windows/awt_Mlib.cpp#l60 On 13/04/2018 06:48, Baesken, Matthias wrote: 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.iva...@oracle.com] Sent: Donnerstag, 12. April 2018 23:53 To: Phil Race <philip.r...@oracle.com>; Baesken, Matthias <matthias.baes...@sap.com>; Alan Bateman <alan.bate...@oracle.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev@openjdk.java.net; core-libs-...@openjdk.java.net; Doerr, Martin <martin.do...@sap.com>; 2d-dev <2d-...@openjdk.java.net>; hotspot-dev <hotspot-...@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.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-dev@openjdk.java.net; Doerr, Martin <martin.do...@sap.com>; core-libs-...@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
RE: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
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.iva...@oracle.com] > Sent: Donnerstag, 12. April 2018 23:53 > To: Phil Race <philip.r...@oracle.com>; Baesken, Matthias > <matthias.baes...@sap.com>; Alan Bateman <alan.bate...@oracle.com>; > Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> > Cc: build-dev@openjdk.java.net; core-libs-...@openjdk.java.net; Doerr, > Martin <martin.do...@sap.com>; 2d-dev <2d-...@openjdk.java.net>; > hotspot-dev <hotspot-...@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.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-dev@openjdk.java.net; Doerr, Martin > <martin.do...@sap.com>; > >>> core-libs-...@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-
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.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-dev@openjdk.java.net; Doerr, Martin <martin.do...@sap.com>;
core-libs-...@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
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 19:38, Phil Race wrote: I was just directed to this look at this change. I don't know why it is being reviewed exclusively on build-dev since no build files are being changed. My bad! I tried to engage core-libs when the patch was ready but I completely overlooked the fact that 2d libs were also affected by this change and I didn't cc to 2d-dev. 50% of it should have been sent to 2d-dev and the rest on core-libs + hotspot .. Is this the current version of the change : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/ ? Yes, it's the latest version to the best of my knowledge. Regards, Alexey -phil. On 04/12/2018 12:49 AM, Baesken, Matthias wrote: Hi, could someone please sponsor the change now ? And could someone please check what happened to the submit-repo ? Yesterday I pushed to the submit repo to check my change , but no response so far . Maybe the submit repo is not working currently , not sure about it . Best regards , Matthias -Original Message- From: Baesken, Matthias Sent: Mittwoch, 11. April 2018 11:20 To: 'Alexey Ivanov' <alexey.iva...@oracle.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 Was main() exported via map files? Seems main was exported , I can find it in jdk10 in e.g. : make/mapfiles/launchers/mapfile-sparcv9 make/mapfiles/launchers/mapfile-x86_64 Best regards, Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Mittwoch, 11. April 2018 11:11 To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 11/04/2018 08:44, Baesken, Matthias wrote: JIMAGE_FindResource doesn't have JNICALL modifier now, does it? Hi Alexey, yes that's true . Please remove JNIEXPORT from main(): src/java.base/share/native/launcher/main.c src/jdk.pack/share/native/unpack200/main.cpp I would prefer to keep it for now . I notice some comments in our SAPJVM code base about needing JNIEXPORT for main for Solaris (we were running in SAPJVM without mapfiles in the past already). Maybe that’s related to src/java.base/unix/native/libjli/java_md_solinux.c where main is dlsym-ed : fptr = (int (*)())dlsym(RTLD_DEFAULT, "main"); but I am not sure about this. So I better keep the JNIEXPORT for the main functions, could be removed in another cleanup if really needed. OK. Let them stay then. Was main() exported via map files? The change looks good to me. Regards, Alexey You can reference both yourself and me as Contributed-by: mbaesken, aivanov when pushing the changeset if you don't mind. Sure . Best regards, Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Dienstag, 10. April 2018 21:34 To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 Hi Matthias, On 10/04/2018 11:14, Baesken, Matthias wrote: Hello, I had to do another small adjustment to make jimage.hpp/cpp match. Please review : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/ JIMAGE_FindResource doesn't have JNICALL modifier now, does it? I've successfully built 32 bit Windows with your patch. Please remove JNIEXPORT from main(): src/java.base/share/native/launcher/main.c src/jdk.pack/share/native/unpack200/main.cpp With the latest webrev I could finally build jdk/jdk successfully on both win32bit and win64 bit. Thanks again to Alexey to provide the incorporated patch . You can reference both yourself and me as Contributed-by: mbaesken, aivanov when pushing the changeset if you don't mind. Regards, Alexey Best regards, Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Montag, 9. April 2018 17:14 To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> Subject: Re: 8201226 missing J
Re: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
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.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-dev@openjdk.java.net; Doerr, Martin <martin.do...@sap.com>;
core-libs-...@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.cThu Apr 05 14:39:04
2018 -0700
+++ b/src/java.base/share/native/launcher/main.cFri 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 andpost 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-dev@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
Re: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
I was just directed to this look at this change. I don't know why it is being reviewed exclusively on build-dev since no build files are being changed. 50% of it should have been sent to 2d-dev and the rest on core-libs + hotspot .. Is this the current version of the change : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/ ? -phil. On 04/12/2018 12:49 AM, Baesken, Matthias wrote: Hi, could someone please sponsor the change now ? And could someone please check what happened to the submit-repo ? Yesterday I pushed to the submit repo to check my change , but no response so far . Maybe the submit repo is not working currently , not sure about it . Best regards , Matthias -Original Message- From: Baesken, Matthias Sent: Mittwoch, 11. April 2018 11:20 To: 'Alexey Ivanov' <alexey.iva...@oracle.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 Was main() exported via map files? Seems main was exported , I can find it in jdk10 in e.g. : make/mapfiles/launchers/mapfile-sparcv9 make/mapfiles/launchers/mapfile-x86_64 Best regards, Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Mittwoch, 11. April 2018 11:11 To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 11/04/2018 08:44, Baesken, Matthias wrote: JIMAGE_FindResource doesn't have JNICALL modifier now, does it? Hi Alexey, yes that's true . Please remove JNIEXPORT from main(): src/java.base/share/native/launcher/main.c src/jdk.pack/share/native/unpack200/main.cpp I would prefer to keep it for now . I notice some comments in our SAPJVM code base about needing JNIEXPORT for main for Solaris (we were running in SAPJVM without mapfiles in the past already). Maybe that’s related to src/java.base/unix/native/libjli/java_md_solinux.c where main is dlsym-ed : fptr = (int (*)())dlsym(RTLD_DEFAULT, "main"); but I am not sure about this. So I better keep the JNIEXPORT for the main functions, could be removed in another cleanup if really needed. OK. Let them stay then. Was main() exported via map files? The change looks good to me. Regards, Alexey You can reference both yourself and me as Contributed-by: mbaesken, aivanov when pushing the changeset if you don't mind. Sure . Best regards, Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Dienstag, 10. April 2018 21:34 To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 Hi Matthias, On 10/04/2018 11:14, Baesken, Matthias wrote: Hello, I had to do another small adjustment to make jimage.hpp/cpp match. Please review : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/ JIMAGE_FindResource doesn't have JNICALL modifier now, does it? I've successfully built 32 bit Windows with your patch. Please remove JNIEXPORT from main(): src/java.base/share/native/launcher/main.c src/jdk.pack/share/native/unpack200/main.cpp With the latest webrev I could finally build jdk/jdk successfully on both win32bit and win64 bit. Thanks again to Alexey to provide the incorporated patch . You can reference both yourself and me as Contributed-by: mbaesken, aivanov when pushing the changeset if you don't mind. Regards, Alexey Best regards, Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Montag, 9. April 2018 17:14 To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 Hi Matthias, On 09/04/2018 15:38, Baesken, Matthias wrote: Hi Alexey,thanks for the diff provided by you, and for t
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 Apr 12, 2018, at 11:54 21AM, Iris Clark <iris.cl...@oracle.com> wrote: > > Hi. > > I believe that the internal page Christian references is for the test system. > > If you want to know whether the push arrived in the repository, you could > subscribe to jdk-submit-chan...@openjdk.java.net. The archive of recent push > notifications is public: > > > http://mail.openjdk.java.net/pipermail/jdk-submit-changes/2018-April/thread.html > > I wonder if the test system could be enhanced to send a brief notification > when a job is queued? I’ve opened an enhancement for adding a notification once the job has been handed off to our build and test farm. Thanks, Christian > > Thanks, > Iris > > -Original Message- > From: Christian Tornqvist > Sent: Thursday, April 12, 2018 6:12 AM > To: Baesken, Matthias <matthias.baes...@sap.com> > Cc: core-libs-...@openjdk.java.net; Alexey Ivanov <alexey.iva...@oracle.com>; > Doerr, Martin <martin.do...@sap.com>; build-dev <build-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 > > > >> On Apr 12, 2018, at 9:07 48AM, Baesken, Matthias <matthias.baes...@sap.com> >> wrote: >> >>> Your submit job ran without failures, we were doing maintenance on >>> the jdk- submit repo yesterday and had turned off notifications. >>> Sorry for the inconvenience. >> >> Hi Christian , Thanks for the information about the submit job success. >> >> Is there a way to check (e.g. webpage) that a submit job has "arrived" >> and is queued for build/test ? > > Unfortunately that webpage is only available internally at this point, we > could look into sending an email notification that the job has been started > if that would help? > > Thanks, > Christian > >> Would have been helpful in this situation . >> >> Best regards, Matthias >> >> >>> -Original Message- >>> From: Christian Tornqvist [mailto:christian.tornqv...@oracle.com] >>> Sent: Donnerstag, 12. April 2018 14:58 >>> To: Baesken, Matthias <matthias.baes...@sap.com> >>> Cc: Alexey Ivanov <alexey.iva...@oracle.com>; Magnus Ihse Bursie >>> <magnus.ihse.bur...@oracle.com>; build-dev >> d...@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> >>> 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 >>> >>> Hi Matthias, >>> >>> >>>> On Apr 12, 2018, at 3:49 35AM, Baesken, Matthias >>> <matthias.baes...@sap.com> wrote: >>>> >>>> Hi, could someone please sponsor the change now ? >>>> >>>> And could someone please check what happened to the submit-repo ? >>>> Yesterday I pushed to the submit repo to check my change , but no >>> response so far . >>>> Maybe the submit repo is not working currently , not sure about it . >>> >>> Your submit job ran without failures, we were doing maintenance on >>> the jdk- submit repo yesterday and had turned off notifications. >>> Sorry for the inconvenience. >>> >>> Thanks, >>> Christian >>>> >>>> >>>> Best regards , Matthias >>>> >>>> >>>> >>>> >>>>> -Original Message- >>>>> From: Baesken, Matthias >>>>> Sent: Mittwoch, 11. April 2018 11:20 >>>>> To: 'Alexey Ivanov' <alexey.iva...@oracle.com>; Magnus Ihse Bursie >>>>> <magnus.ihse.bur...@oracle.com> >>>>> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin >>>>> <martin.do...@sap.com> >>>>> 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 >>>>> >>>>>> >>>>>> Was main() exported via map files? >>>>>> >>>>> >>>>> Seems main was exported , I can find it in jdk10 in e.g. : >>>>> >>>>&g
RE: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
Hi. I believe that the internal page Christian references is for the test system. If you want to know whether the push arrived in the repository, you could subscribe to jdk-submit-chan...@openjdk.java.net. The archive of recent push notifications is public: http://mail.openjdk.java.net/pipermail/jdk-submit-changes/2018-April/thread.html I wonder if the test system could be enhanced to send a brief notification when a job is queued? Thanks, Iris -Original Message- From: Christian Tornqvist Sent: Thursday, April 12, 2018 6:12 AM To: Baesken, Matthias <matthias.baes...@sap.com> Cc: core-libs-...@openjdk.java.net; Alexey Ivanov <alexey.iva...@oracle.com>; Doerr, Martin <martin.do...@sap.com>; build-dev <build-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 > On Apr 12, 2018, at 9:07 48AM, Baesken, Matthias <matthias.baes...@sap.com> > wrote: > >> Your submit job ran without failures, we were doing maintenance on >> the jdk- submit repo yesterday and had turned off notifications. >> Sorry for the inconvenience. > > Hi Christian , Thanks for the information about the submit job success. > > Is there a way to check (e.g. webpage) that a submit job has "arrived" and > is queued for build/test ? Unfortunately that webpage is only available internally at this point, we could look into sending an email notification that the job has been started if that would help? Thanks, Christian > Would have been helpful in this situation . > > Best regards, Matthias > > >> -Original Message- >> From: Christian Tornqvist [mailto:christian.tornqv...@oracle.com] >> Sent: Donnerstag, 12. April 2018 14:58 >> To: Baesken, Matthias <matthias.baes...@sap.com> >> Cc: Alexey Ivanov <alexey.iva...@oracle.com>; Magnus Ihse Bursie >> <magnus.ihse.bur...@oracle.com>; build-dev > d...@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> >> 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 >> >> Hi Matthias, >> >> >>> On Apr 12, 2018, at 3:49 35AM, Baesken, Matthias >> <matthias.baes...@sap.com> wrote: >>> >>> Hi, could someone please sponsor the change now ? >>> >>> And could someone please check what happened to the submit-repo ? >>> Yesterday I pushed to the submit repo to check my change , but no >> response so far . >>> Maybe the submit repo is not working currently , not sure about it . >> >> Your submit job ran without failures, we were doing maintenance on >> the jdk- submit repo yesterday and had turned off notifications. >> Sorry for the inconvenience. >> >> Thanks, >> Christian >>> >>> >>> Best regards , Matthias >>> >>> >>> >>> >>>> -Original Message----- >>>> From: Baesken, Matthias >>>> Sent: Mittwoch, 11. April 2018 11:20 >>>> To: 'Alexey Ivanov' <alexey.iva...@oracle.com>; Magnus Ihse Bursie >>>> <magnus.ihse.bur...@oracle.com> >>>> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin >>>> <martin.do...@sap.com> >>>> 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 >>>> >>>>> >>>>> Was main() exported via map files? >>>>> >>>> >>>> Seems main was exported , I can find it in jdk10 in e.g. : >>>> >>>> make/mapfiles/launchers/mapfile-sparcv9 >>>> make/mapfiles/launchers/mapfile-x86_64 >>>> >>>> >>>> Best regards, Matthias >>>> >>>> >>>>> -Original Message- >>>>> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] >>>>> Sent: Mittwoch, 11. April 2018 11:11 >>>>> To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse >> Bursie >>>>> <magnus.ihse.bur...@oracle.com> >>>>> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin >>>>> <martin.do...@sap.com> >&g
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 Apr 12, 2018, at 9:07 48AM, Baesken, Matthias <matthias.baes...@sap.com> > wrote: > >> Your submit job ran without failures, we were doing maintenance on the jdk- >> submit repo yesterday and had turned off notifications. Sorry for the >> inconvenience. > > Hi Christian , Thanks for the information about the submit job success. > > Is there a way to check (e.g. webpage) that a submit job has "arrived" and > is queued for build/test ? Unfortunately that webpage is only available internally at this point, we could look into sending an email notification that the job has been started if that would help? Thanks, Christian > Would have been helpful in this situation . > > Best regards, Matthias > > >> -Original Message- >> From: Christian Tornqvist [mailto:christian.tornqv...@oracle.com] >> Sent: Donnerstag, 12. April 2018 14:58 >> To: Baesken, Matthias <matthias.baes...@sap.com> >> Cc: Alexey Ivanov <alexey.iva...@oracle.com>; Magnus Ihse Bursie >> <magnus.ihse.bur...@oracle.com>; build-dev > d...@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> >> 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 >> >> Hi Matthias, >> >> >>> On Apr 12, 2018, at 3:49 35AM, Baesken, Matthias >> <matthias.baes...@sap.com> wrote: >>> >>> Hi, could someone please sponsor the change now ? >>> >>> And could someone please check what happened to the submit-repo ? >>> Yesterday I pushed to the submit repo to check my change , but no >> response so far . >>> Maybe the submit repo is not working currently , not sure about it . >> >> Your submit job ran without failures, we were doing maintenance on the jdk- >> submit repo yesterday and had turned off notifications. Sorry for the >> inconvenience. >> >> Thanks, >> Christian >>> >>> >>> Best regards , Matthias >>> >>> >>> >>> >>>> -----Original Message- >>>> From: Baesken, Matthias >>>> Sent: Mittwoch, 11. April 2018 11:20 >>>> To: 'Alexey Ivanov' <alexey.iva...@oracle.com>; Magnus Ihse Bursie >>>> <magnus.ihse.bur...@oracle.com> >>>> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin >>>> <martin.do...@sap.com> >>>> 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 >>>> >>>>> >>>>> Was main() exported via map files? >>>>> >>>> >>>> Seems main was exported , I can find it in jdk10 in e.g. : >>>> >>>> make/mapfiles/launchers/mapfile-sparcv9 >>>> make/mapfiles/launchers/mapfile-x86_64 >>>> >>>> >>>> Best regards, Matthias >>>> >>>> >>>>> -Original Message- >>>>> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] >>>>> Sent: Mittwoch, 11. April 2018 11:11 >>>>> To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse >> Bursie >>>>> <magnus.ihse.bur...@oracle.com> >>>>> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin >>>>> <martin.do...@sap.com> >>>>> 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 11/04/2018 08:44, Baesken, Matthias wrote: >>>>>>> JIMAGE_FindResource doesn't have JNICALL modifier now, does it? >>>>>> Hi Alexey, yes that's true . >>>>>> >>>>>>> Please remove JNIEXPORT from main(): >>>>>>> src/java.base/share/native/launcher/main.c >>>>>>> src/jdk.pack/share/native/unpack200/main.cpp >>>>>> I would prefer to keep it for now . >>>>>> I notice some comments in our SAPJVM code base about needing >>>>> JNIEXPORT for main for Solaris (we were running in S
RE: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
> Your submit job ran without failures, we were doing maintenance on the jdk- > submit repo yesterday and had turned off notifications. Sorry for the > inconvenience. Hi Christian , Thanks for the information about the submit job success. Is there a way to check (e.g. webpage) that a submit job has "arrived" and is queued for build/test ? Would have been helpful in this situation . Best regards, Matthias > -Original Message- > From: Christian Tornqvist [mailto:christian.tornqv...@oracle.com] > Sent: Donnerstag, 12. April 2018 14:58 > To: Baesken, Matthias <matthias.baes...@sap.com> > Cc: Alexey Ivanov <alexey.iva...@oracle.com>; Magnus Ihse Bursie > <magnus.ihse.bur...@oracle.com>; build-dev d...@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> > 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 > > Hi Matthias, > > > > On Apr 12, 2018, at 3:49 35AM, Baesken, Matthias > <matthias.baes...@sap.com> wrote: > > > > Hi, could someone please sponsor the change now ? > > > > And could someone please check what happened to the submit-repo ? > > Yesterday I pushed to the submit repo to check my change , but no > response so far . > > Maybe the submit repo is not working currently , not sure about it . > > Your submit job ran without failures, we were doing maintenance on the jdk- > submit repo yesterday and had turned off notifications. Sorry for the > inconvenience. > > Thanks, > Christian > > > > > > Best regards , Matthias > > > > > > > > > >> -Original Message- > >> From: Baesken, Matthias > >> Sent: Mittwoch, 11. April 2018 11:20 > >> To: 'Alexey Ivanov' <alexey.iva...@oracle.com>; Magnus Ihse Bursie > >> <magnus.ihse.bur...@oracle.com> > >> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin > >> <martin.do...@sap.com> > >> 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 > >> > >>> > >>> Was main() exported via map files? > >>> > >> > >> Seems main was exported , I can find it in jdk10 in e.g. : > >> > >> make/mapfiles/launchers/mapfile-sparcv9 > >> make/mapfiles/launchers/mapfile-x86_64 > >> > >> > >> Best regards, Matthias > >> > >> > >>> -----Original Message- > >>> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] > >>> Sent: Mittwoch, 11. April 2018 11:11 > >>> To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse > Bursie > >>> <magnus.ihse.bur...@oracle.com> > >>> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin > >>> <martin.do...@sap.com> > >>> 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 11/04/2018 08:44, Baesken, Matthias wrote: > >>>>> JIMAGE_FindResource doesn't have JNICALL modifier now, does it? > >>>> Hi Alexey, yes that's true . > >>>> > >>>>> Please remove JNIEXPORT from main(): > >>>>> src/java.base/share/native/launcher/main.c > >>>>> src/jdk.pack/share/native/unpack200/main.cpp > >>>> I would prefer to keep it for now . > >>>> I notice some comments in our SAPJVM code base about needing > >>> JNIEXPORT for main for Solaris (we were running in SAPJVM without > >>> mapfiles in the past already). > >>>> Maybe that’s related to > >>>> > >>>> src/java.base/unix/native/libjli/java_md_solinux.c > >>>> > >>>> where main is dlsym-ed : fptr = (int (*)())dlsym(RTLD_DEFAULT, > "main"); > >>>> but I am not sure about this. > >>>> So I better keep the JNIEXPORT for the main functions, could be > >>> removed in another cleanup if really needed. > >>> > >>> OK. Let them stay then. > >>> Was main() exported via map files? > &g
Re: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
Hi Matthias, > On Apr 12, 2018, at 3:49 35AM, Baesken, Matthias <matthias.baes...@sap.com> > wrote: > > Hi, could someone please sponsor the change now ? > > And could someone please check what happened to the submit-repo ? > Yesterday I pushed to the submit repo to check my change , but no > response so far . > Maybe the submit repo is not working currently , not sure about it . Your submit job ran without failures, we were doing maintenance on the jdk-submit repo yesterday and had turned off notifications. Sorry for the inconvenience. Thanks, Christian > > > Best regards , Matthias > > > > >> -Original Message- >> From: Baesken, Matthias >> Sent: Mittwoch, 11. April 2018 11:20 >> To: 'Alexey Ivanov' <alexey.iva...@oracle.com>; Magnus Ihse Bursie >> <magnus.ihse.bur...@oracle.com> >> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin >> <martin.do...@sap.com> >> 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 >> >>> >>> Was main() exported via map files? >>> >> >> Seems main was exported , I can find it in jdk10 in e.g. : >> >> make/mapfiles/launchers/mapfile-sparcv9 >> make/mapfiles/launchers/mapfile-x86_64 >> >> >> Best regards, Matthias >> >> >>> -Original Message- >>> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] >>> Sent: Mittwoch, 11. April 2018 11:11 >>> To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie >>> <magnus.ihse.bur...@oracle.com> >>> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin >>> <martin.do...@sap.com> >>> 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 11/04/2018 08:44, Baesken, Matthias wrote: >>>>> JIMAGE_FindResource doesn't have JNICALL modifier now, does it? >>>> Hi Alexey, yes that's true . >>>> >>>>> Please remove JNIEXPORT from main(): >>>>> src/java.base/share/native/launcher/main.c >>>>> src/jdk.pack/share/native/unpack200/main.cpp >>>> I would prefer to keep it for now . >>>> I notice some comments in our SAPJVM code base about needing >>> JNIEXPORT for main for Solaris (we were running in SAPJVM without >>> mapfiles in the past already). >>>> Maybe that’s related to >>>> >>>> src/java.base/unix/native/libjli/java_md_solinux.c >>>> >>>> where main is dlsym-ed : fptr = (int (*)())dlsym(RTLD_DEFAULT, "main"); >>>> but I am not sure about this. >>>> So I better keep the JNIEXPORT for the main functions, could be >>> removed in another cleanup if really needed. >>> >>> OK. Let them stay then. >>> Was main() exported via map files? >>> >>> >>> The change looks good to me. >>> >>> Regards, >>> Alexey >>> >>>> >>>>> You can reference both yourself and me as >>>>> Contributed-by: mbaesken, aivanov >>>>> when pushing the changeset if you don't mind. >>>>> >>>> Sure . >>>> >>>> Best regards, Matthias >>>> >>>> >>>>> -Original Message- >>>>> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] >>>>> Sent: Dienstag, 10. April 2018 21:34 >>>>> To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse >>> Bursie >>>>> <magnus.ihse.bur...@oracle.com> >>>>> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin >>>>> <martin.do...@sap.com> >>>>> 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 >>>>> >>>>> Hi Matthias, >>>>> >>>>> On 10/04/2018 11:14, Baesken, Matthias wrote: >>>>>> Hello, I had to do another small adjustment to m
Re: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
Hi Matthias, On 12/04/2018 11:12, Alexey Ivanov wrote: Hi Matthias, On 12/04/2018 08:49, Baesken, Matthias wrote: Hi, could someone please sponsor the change now ? According to OpenJDK Census page [1], you have committer rights to JDK project. And could someone please check what happened to the submit-repo ? Yesterday I pushed to the submit repo to check my change , but no response so far . Maybe the submit repo is not working currently , not sure about it . I can your push to submit repo on 10 Apr 2018 08:38:56 +0200. However, I'm not sure where the build and test job is run. I couldn't find it. Anyway, I've just submitted a build job with your changeset and am running 32 bit Windows build. The build job is successful: no failures found. My local 32 bit Windows build is also fine. I successfully ran SwingSet2 and Java2Demo with it. I guess we're waiting for approval from core-libs. Regards, Alexey Regards, Alexey [1] http://openjdk.java.net/census#mbaesken Best regards , Matthias
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.cThu Apr 05 14:39:04
2018 -0700
+++ b/src/java.base/share/native/launcher/main.cFri 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 andpost
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-dev@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-dev@openjdk.java.net<mailto:build-dev@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/mlib_ImageConvKernelConvert.c(87)
: error C2373: 'j2d_mlib_ImageConvKernelConvert' : redefinition; different
type modifiers
e:\priv\openjdk\repos\jdk\src\java.desktop\share\native\libmlib_image\mlib_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/ji
Re: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
Hi Matthias, On 12/04/2018 08:49, Baesken, Matthias wrote: Hi, could someone please sponsor the change now ? According to OpenJDK Census page [1], you have committer rights to JDK project. And could someone please check what happened to the submit-repo ? Yesterday I pushed to the submit repo to check my change , but no response so far . Maybe the submit repo is not working currently , not sure about it . I can your push to submit repo on 10 Apr 2018 08:38:56 +0200. However, I'm not sure where the build and test job is run. I couldn't find it. Anyway, I've just submitted a build job with your changeset and am running 32 bit Windows build. Regards, Alexey [1] http://openjdk.java.net/census#mbaesken Best regards , Matthias -Original Message- From: Baesken, Matthias Sent: Mittwoch, 11. April 2018 11:20 To: 'Alexey Ivanov' <alexey.iva...@oracle.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 Was main() exported via map files? Seems main was exported , I can find it in jdk10 in e.g. : make/mapfiles/launchers/mapfile-sparcv9 make/mapfiles/launchers/mapfile-x86_64 Best regards, Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Mittwoch, 11. April 2018 11:11 To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 11/04/2018 08:44, Baesken, Matthias wrote: JIMAGE_FindResource doesn't have JNICALL modifier now, does it? Hi Alexey, yes that's true . Please remove JNIEXPORT from main(): src/java.base/share/native/launcher/main.c src/jdk.pack/share/native/unpack200/main.cpp I would prefer to keep it for now . I notice some comments in our SAPJVM code base about needing JNIEXPORT for main for Solaris (we were running in SAPJVM without mapfiles in the past already). Maybe that’s related to src/java.base/unix/native/libjli/java_md_solinux.c where main is dlsym-ed : fptr = (int (*)())dlsym(RTLD_DEFAULT, "main"); but I am not sure about this. So I better keep the JNIEXPORT for the main functions, could be removed in another cleanup if really needed. OK. Let them stay then. Was main() exported via map files? The change looks good to me. Regards, Alexey You can reference both yourself and me as Contributed-by: mbaesken, aivanov when pushing the changeset if you don't mind. Sure . Best regards, Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Dienstag, 10. April 2018 21:34 To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 Hi Matthias, On 10/04/2018 11:14, Baesken, Matthias wrote: Hello, I had to do another small adjustment to make jimage.hpp/cpp match. Please review : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/ JIMAGE_FindResource doesn't have JNICALL modifier now, does it? I've successfully built 32 bit Windows with your patch. Please remove JNIEXPORT from main(): src/java.base/share/native/launcher/main.c src/jdk.pack/share/native/unpack200/main.cpp With the latest webrev I could finally build jdk/jdk successfully on both win32bit and win64 bit. Thanks again to Alexey to provide the incorporated patch . You can reference both yourself and me as Contributed-by: mbaesken, aivanov when pushing the changeset if you don't mind. Regards, Alexey Best regards, Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Montag, 9. April 2018 17:14 To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 Hi Matthias, On 09/04/2018 15:38, Baesken, Matthias wrote: Hi Alexey,
RE: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
Hi, could someone please sponsor the change now ? And could someone please check what happened to the submit-repo ? Yesterday I pushed to the submit repo to check my change , but no response so far . Maybe the submit repo is not working currently , not sure about it . Best regards , Matthias > -Original Message- > From: Baesken, Matthias > Sent: Mittwoch, 11. April 2018 11:20 > To: 'Alexey Ivanov' <alexey.iva...@oracle.com>; Magnus Ihse Bursie > <magnus.ihse.bur...@oracle.com> > Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin > <martin.do...@sap.com> > 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 > > > > > Was main() exported via map files? > > > > Seems main was exported , I can find it in jdk10 in e.g. : > > make/mapfiles/launchers/mapfile-sparcv9 > make/mapfiles/launchers/mapfile-x86_64 > > > Best regards, Matthias > > > > -Original Message- > > From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] > > Sent: Mittwoch, 11. April 2018 11:11 > > To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie > > <magnus.ihse.bur...@oracle.com> > > Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin > > <martin.do...@sap.com> > > 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 11/04/2018 08:44, Baesken, Matthias wrote: > > >> JIMAGE_FindResource doesn't have JNICALL modifier now, does it? > > > Hi Alexey, yes that's true . > > > > > >> Please remove JNIEXPORT from main(): > > >> src/java.base/share/native/launcher/main.c > > >> src/jdk.pack/share/native/unpack200/main.cpp > > > I would prefer to keep it for now . > > > I notice some comments in our SAPJVM code base about needing > > JNIEXPORT for main for Solaris (we were running in SAPJVM without > > mapfiles in the past already). > > > Maybe that’s related to > > > > > > src/java.base/unix/native/libjli/java_md_solinux.c > > > > > > where main is dlsym-ed : fptr = (int (*)())dlsym(RTLD_DEFAULT, "main"); > > > but I am not sure about this. > > > So I better keep the JNIEXPORT for the main functions, could be > > removed in another cleanup if really needed. > > > > OK. Let them stay then. > > Was main() exported via map files? > > > > > > The change looks good to me. > > > > Regards, > > Alexey > > > > > > > >> You can reference both yourself and me as > > >> Contributed-by: mbaesken, aivanov > > >> when pushing the changeset if you don't mind. > > >> > > > Sure . > > > > > > Best regards, Matthias > > > > > > > > >> -Original Message- > > >> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] > > >> Sent: Dienstag, 10. April 2018 21:34 > > >> To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse > > Bursie > > >> <magnus.ihse.bur...@oracle.com> > > >> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin > > >> <martin.do...@sap.com> > > >> 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 > > >> > > >> Hi Matthias, > > >> > > >> On 10/04/2018 11:14, Baesken, Matthias wrote: > > >>> Hello, I had to do another small adjustment to make jimage.hpp/cpp > > match. Please review : > > >>> > > >>> http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/ > > >> JIMAGE_FindResource doesn't have JNICALL modifier now, does it? > > >> > > >> I've successfully built 32 bit Windows with your patch. > > >> > > >> > > >> Please remove JNIEXPORT from main(): > > >> src/java.base/share/native/launcher/main.c > > >> src/jdk.pack/share/native/unpack200/main.cpp > > >> > > >>> With the latest webrev I could finally build jdk/jdk succe
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 11/04/2018 11:38, Magnus Ihse Bursie wrote: 11 apr. 2018 kl. 11:10 skrev Alexey Ivanov <alexey.iva...@oracle.com>: On 11/04/2018 08:44, Baesken, Matthias wrote: JIMAGE_FindResource doesn't have JNICALL modifier now, does it? Hi Alexey, yes that's true . Please remove JNIEXPORT from main(): src/java.base/share/native/launcher/main.c src/jdk.pack/share/native/unpack200/main.cpp I would prefer to keep it for now . I notice some comments in our SAPJVM code base about needing JNIEXPORT for main for Solaris (we were running in SAPJVM without mapfiles in the past already). Maybe that’s related to src/java.base/unix/native/libjli/java_md_solinux.c where main is dlsym-ed : fptr = (int (*)())dlsym(RTLD_DEFAULT, "main"); but I am not sure about this. So I better keep the JNIEXPORT for the main functions, could be removed in another cleanup if really needed. OK. Let them stay then. Was main() exported via map files? Yes. You cannot remove the export of main! Since we hide symbols by default even for executables, it would not be possible to execute if the main function is not visible. Actually it works without JNIEXPORT modifier at main function declared in src/java.base/share/native/launcher/main.c src/jdk.pack/share/native/unpack200/main.cpp at least for all 64 bit platforms built at Oracle, all tier3 tests pass. I wouldn't have suggested removing JNIEXPORT if it hadn't. Usually main is not exported. Yet it was exported via map files and the code snippet provided by Matthias hints something may break if main is not exported. Regards, Alexey /Magnus The change looks good to me. Regards, Alexey You can reference both yourself and me as Contributed-by: mbaesken, aivanov when pushing the changeset if you don't mind. Sure . Best regards, Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Dienstag, 10. April 2018 21:34 To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 Hi Matthias, On 10/04/2018 11:14, Baesken, Matthias wrote: Hello, I had to do another small adjustment to make jimage.hpp/cpp match. Please review : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/ JIMAGE_FindResource doesn't have JNICALL modifier now, does it? I've successfully built 32 bit Windows with your patch. Please remove JNIEXPORT from main(): src/java.base/share/native/launcher/main.c src/jdk.pack/share/native/unpack200/main.cpp With the latest webrev I could finally build jdk/jdk successfully on both win32bit and win64 bit. Thanks again to Alexey to provide the incorporated patch . You can reference both yourself and me as Contributed-by: mbaesken, aivanov when pushing the changeset if you don't mind. Regards, Alexey Best regards, Matthias
Re: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
I guess we also need to get an approval from core-libs (cc'd) as libzip and libjimage are modified. Regards, Alexey On 11/04/2018 10:56, Alexey Ivanov wrote: The change looks good to me. On 11/04/2018 10:20, Baesken, Matthias wrote: Was main() exported via map files? Seems main was exported , I can find it in jdk10 in e.g. : make/mapfiles/launchers/mapfile-sparcv9 make/mapfiles/launchers/mapfile-x86_64 Thank you for confirming it. Then JNIEXPORT is better left untouched to keep main() exported. Regards, Alexey Best regards, Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Mittwoch, 11. April 2018 11:11 To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 11/04/2018 08:44, Baesken, Matthias wrote: JIMAGE_FindResource doesn't have JNICALL modifier now, does it? Hi Alexey, yes that's true . Please remove JNIEXPORT from main(): src/java.base/share/native/launcher/main.c src/jdk.pack/share/native/unpack200/main.cpp I would prefer to keep it for now . I notice some comments in our SAPJVM code base about needing JNIEXPORT for main for Solaris (we were running in SAPJVM without mapfiles in the past already). Maybe that’s related to src/java.base/unix/native/libjli/java_md_solinux.c where main is dlsym-ed : fptr = (int (*)())dlsym(RTLD_DEFAULT, "main"); but I am not sure about this. So I better keep the JNIEXPORT for the main functions, could be removed in another cleanup if really needed. OK. Let them stay then. Was main() exported via map files? The change looks good to me. Regards, Alexey You can reference both yourself and me as Contributed-by: mbaesken, aivanov when pushing the changeset if you don't mind. Sure . Best regards, Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Dienstag, 10. April 2018 21:34 To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 Hi Matthias, On 10/04/2018 11:14, Baesken, Matthias wrote: Hello, I had to do another small adjustment to make jimage.hpp/cpp match. Please review : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/ JIMAGE_FindResource doesn't have JNICALL modifier now, does it? I've successfully built 32 bit Windows with your patch. Please remove JNIEXPORT from main(): src/java.base/share/native/launcher/main.c src/jdk.pack/share/native/unpack200/main.cpp With the latest webrev I could finally build jdk/jdk successfully on both win32bit and win64 bit. Thanks again to Alexey to provide the incorporated patch . You can reference both yourself and me as Contributed-by: mbaesken, aivanov when pushing the changeset if you don't mind. Regards, Alexey Best regards, Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Montag, 9. April 2018 17:14 To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 Hi Matthias, On 09/04/2018 15:38, Baesken, Matthias wrote: Hi Alexey, thanks for the diff provided by you, and for the explanations . I created a second webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.1/ - it adds the diff provided by you (hope that’s fine with you) Yes, that's fine with me. There could be only one author ;) - changes 2 launchers src/java.base/share/native/launcher/main.c and src/jdk.pack/share/native/unpack200/main.cpp where we face similar issues after mapfile removal for exes I'd rather remove both JNIEXPORT and JNICALL from main(). It wasn't exported, and it shouldn't be. Regards, Alexey Best regards , Matthias
Re: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
The change looks good to me. On 11/04/2018 10:20, Baesken, Matthias wrote: Was main() exported via map files? Seems main was exported , I can find it in jdk10 in e.g. : make/mapfiles/launchers/mapfile-sparcv9 make/mapfiles/launchers/mapfile-x86_64 Thank you for confirming it. Then JNIEXPORT is better left untouched to keep main() exported. Regards, Alexey Best regards, Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Mittwoch, 11. April 2018 11:11 To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 11/04/2018 08:44, Baesken, Matthias wrote: JIMAGE_FindResource doesn't have JNICALL modifier now, does it? Hi Alexey, yes that's true . Please remove JNIEXPORT from main(): src/java.base/share/native/launcher/main.c src/jdk.pack/share/native/unpack200/main.cpp I would prefer to keep it for now . I notice some comments in our SAPJVM code base about needing JNIEXPORT for main for Solaris (we were running in SAPJVM without mapfiles in the past already). Maybe that’s related to src/java.base/unix/native/libjli/java_md_solinux.c where main is dlsym-ed : fptr = (int (*)())dlsym(RTLD_DEFAULT, "main"); but I am not sure about this. So I better keep the JNIEXPORT for the main functions, could be removed in another cleanup if really needed. OK. Let them stay then. Was main() exported via map files? The change looks good to me. Regards, Alexey You can reference both yourself and me as Contributed-by: mbaesken, aivanov when pushing the changeset if you don't mind. Sure . Best regards, Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Dienstag, 10. April 2018 21:34 To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 Hi Matthias, On 10/04/2018 11:14, Baesken, Matthias wrote: Hello, I had to do another small adjustment to make jimage.hpp/cpp match. Please review : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/ JIMAGE_FindResource doesn't have JNICALL modifier now, does it? I've successfully built 32 bit Windows with your patch. Please remove JNIEXPORT from main(): src/java.base/share/native/launcher/main.c src/jdk.pack/share/native/unpack200/main.cpp With the latest webrev I could finally build jdk/jdk successfully on both win32bit and win64 bit. Thanks again to Alexey to provide the incorporated patch . You can reference both yourself and me as Contributed-by: mbaesken, aivanov when pushing the changeset if you don't mind. Regards, Alexey Best regards, Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Montag, 9. April 2018 17:14 To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 Hi Matthias, On 09/04/2018 15:38, Baesken, Matthias wrote: Hi Alexey,thanks for the diff provided by you, and for the explanations . I created a second webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.1/ - it adds the diff provided by you(hope that’s fine with you) Yes, that's fine with me. There could be only one author ;) -changes 2 launcherssrc/java.base/share/native/launcher/main.c and src/jdk.pack/share/native/unpack200/main.cppwhere we face similar issues after mapfile removal for exes I'd rather remove both JNIEXPORT and JNICALL from main(). It wasn't exported, and it shouldn't be. Regards, Alexey Best regards , Matthias
RE: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
> > Was main() exported via map files? > Seems main was exported , I can find it in jdk10 in e.g. : make/mapfiles/launchers/mapfile-sparcv9 make/mapfiles/launchers/mapfile-x86_64 Best regards, Matthias > -Original Message- > From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] > Sent: Mittwoch, 11. April 2018 11:11 > To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie > <magnus.ihse.bur...@oracle.com> > Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin > <martin.do...@sap.com> > 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 11/04/2018 08:44, Baesken, Matthias wrote: > >> JIMAGE_FindResource doesn't have JNICALL modifier now, does it? > > Hi Alexey, yes that's true . > > > >> Please remove JNIEXPORT from main(): > >> src/java.base/share/native/launcher/main.c > >> src/jdk.pack/share/native/unpack200/main.cpp > > I would prefer to keep it for now . > > I notice some comments in our SAPJVM code base about needing > JNIEXPORT for main for Solaris (we were running in SAPJVM without > mapfiles in the past already). > > Maybe that’s related to > > > > src/java.base/unix/native/libjli/java_md_solinux.c > > > > where main is dlsym-ed : fptr = (int (*)())dlsym(RTLD_DEFAULT, "main"); > > but I am not sure about this. > > So I better keep the JNIEXPORT for the main functions, could be > removed in another cleanup if really needed. > > OK. Let them stay then. > Was main() exported via map files? > > > The change looks good to me. > > Regards, > Alexey > > > > >> You can reference both yourself and me as > >> Contributed-by: mbaesken, aivanov > >> when pushing the changeset if you don't mind. > >> > > Sure . > > > > Best regards, Matthias > > > > > >> -Original Message- > >> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] > >> Sent: Dienstag, 10. April 2018 21:34 > >> To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse > Bursie > >> <magnus.ihse.bur...@oracle.com> > >> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin > >> <martin.do...@sap.com> > >> 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 > >> > >> Hi Matthias, > >> > >> On 10/04/2018 11:14, Baesken, Matthias wrote: > >>> Hello, I had to do another small adjustment to make jimage.hpp/cpp > match. Please review : > >>> > >>> http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/ > >> JIMAGE_FindResource doesn't have JNICALL modifier now, does it? > >> > >> I've successfully built 32 bit Windows with your patch. > >> > >> > >> Please remove JNIEXPORT from main(): > >> src/java.base/share/native/launcher/main.c > >> src/jdk.pack/share/native/unpack200/main.cpp > >> > >>> With the latest webrev I could finally build jdk/jdk successfully on both > win32bit and win64 bit. > >>> > >>> Thanks again to Alexey to provide the incorporated patch . > >> You can reference both yourself and me as > >> Contributed-by: mbaesken, aivanov > >> when pushing the changeset if you don't mind. > >> > >> > >> Regards, > >> Alexey > >> > >>> > >>> Best regards, Matthias > >>> > >>> > >>> > >>>> -Original Message- > >>>> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] > >>>> Sent: Montag, 9. April 2018 17:14 > >>>> To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse > >> Bursie > >>>> <magnus.ihse.bur...@oracle.com> > >>>> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin > >>>> <martin.do...@sap.com> > >>>> 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 > >>>> > >>>> Hi Matthias, > >>>> > >>>> On 09/04/2018 15:38, Baesken, Matthias wrote: > >>>>> Hi Alexey,thanks for the diff provided by you, and for the > >> explanations > >>>> . > >>>>> I created a second webrev : > >>>>> > >>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.1/ > >>>>> > >>>>> - it adds the diff provided by you(hope that’s fine with you) > >>>> Yes, that's fine with me. > >>>> There could be only one author ;) > >>>> > >>>>> -changes 2 launcherssrc/java.base/share/native/launcher/main.c > >> and > >>>> src/jdk.pack/share/native/unpack200/main.cppwhere we face > similar > >>>> issues after mapfile removal for exes > >>>> > >>>> I'd rather remove both JNIEXPORT and JNICALL from main(). > >>>> It wasn't exported, and it shouldn't be. > >>>> > >>>> Regards, > >>>> Alexey > >>>> > >>>>> Best regards , Matthias
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 11/04/2018 08:44, Baesken, Matthias wrote: JIMAGE_FindResource doesn't have JNICALL modifier now, does it? Hi Alexey, yes that's true . Please remove JNIEXPORT from main(): src/java.base/share/native/launcher/main.c src/jdk.pack/share/native/unpack200/main.cpp I would prefer to keep it for now . I notice some comments in our SAPJVM code base about needing JNIEXPORT for main for Solaris (we were running in SAPJVM without mapfiles in the past already). Maybe that’s related to src/java.base/unix/native/libjli/java_md_solinux.c where main is dlsym-ed : fptr = (int (*)())dlsym(RTLD_DEFAULT, "main"); but I am not sure about this. So I better keep the JNIEXPORT for the main functions, could be removed in another cleanup if really needed. OK. Let them stay then. Was main() exported via map files? The change looks good to me. Regards, Alexey You can reference both yourself and me as Contributed-by: mbaesken, aivanov when pushing the changeset if you don't mind. Sure . Best regards, Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Dienstag, 10. April 2018 21:34 To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 Hi Matthias, On 10/04/2018 11:14, Baesken, Matthias wrote: Hello, I had to do another small adjustment to make jimage.hpp/cpp match. Please review : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/ JIMAGE_FindResource doesn't have JNICALL modifier now, does it? I've successfully built 32 bit Windows with your patch. Please remove JNIEXPORT from main(): src/java.base/share/native/launcher/main.c src/jdk.pack/share/native/unpack200/main.cpp With the latest webrev I could finally build jdk/jdk successfully on both win32bit and win64 bit. Thanks again to Alexey to provide the incorporated patch . You can reference both yourself and me as Contributed-by: mbaesken, aivanov when pushing the changeset if you don't mind. Regards, Alexey Best regards, Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Montag, 9. April 2018 17:14 To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 Hi Matthias, On 09/04/2018 15:38, Baesken, Matthias wrote: Hi Alexey,thanks for the diff provided by you, and for the explanations . I created a second webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.1/ - it adds the diff provided by you(hope that’s fine with you) Yes, that's fine with me. There could be only one author ;) -changes 2 launcherssrc/java.base/share/native/launcher/main.c and src/jdk.pack/share/native/unpack200/main.cppwhere we face similar issues after mapfile removal for exes I'd rather remove both JNIEXPORT and JNICALL from main(). It wasn't exported, and it shouldn't be. Regards, Alexey Best regards , Matthias
RE: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
> > JIMAGE_FindResource doesn't have JNICALL modifier now, does it? > Hi Alexey, yes that's true . > Please remove JNIEXPORT from main(): > src/java.base/share/native/launcher/main.c > src/jdk.pack/share/native/unpack200/main.cpp I would prefer to keep it for now . I notice some comments in our SAPJVM code base about needing JNIEXPORT for main for Solaris (we were running in SAPJVM without mapfiles in the past already). Maybe that’s related to src/java.base/unix/native/libjli/java_md_solinux.c where main is dlsym-ed : fptr = (int (*)())dlsym(RTLD_DEFAULT, "main"); but I am not sure about this. So I better keep the JNIEXPORT for the main functions, could be removed in another cleanup if really needed. > You can reference both yourself and me as > Contributed-by: mbaesken, aivanov > when pushing the changeset if you don't mind. > Sure . Best regards, Matthias > -Original Message- > From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] > Sent: Dienstag, 10. April 2018 21:34 > To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie > <magnus.ihse.bur...@oracle.com> > Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin > <martin.do...@sap.com> > 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 > > Hi Matthias, > > On 10/04/2018 11:14, Baesken, Matthias wrote: > > Hello, I had to do another small adjustment to make jimage.hpp/cpp > match.Please review : > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/ > > JIMAGE_FindResource doesn't have JNICALL modifier now, does it? > > I've successfully built 32 bit Windows with your patch. > > > Please remove JNIEXPORT from main(): > src/java.base/share/native/launcher/main.c > src/jdk.pack/share/native/unpack200/main.cpp > > > With the latest webrev I could finallybuildjdk/jdk successfully > > on both > win32bit and win64 bit . > > > > > > > > Thanks again to Alexey to provide the incorporated patch . > > You can reference both yourself and me as > Contributed-by: mbaesken, aivanov > when pushing the changeset if you don't mind. > > > Regards, > Alexey > > > > > > > Best regards, Matthias > > > > > > > >> -Original Message- > >> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] > >> Sent: Montag, 9. April 2018 17:14 > >> To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse > Bursie > >> <magnus.ihse.bur...@oracle.com> > >> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin > >> <martin.do...@sap.com> > >> 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 > >> > >> Hi Matthias, > >> > >> On 09/04/2018 15:38, Baesken, Matthias wrote: > >>> Hi Alexey,thanks for the diff provided by you, and for the > explanations > >> . > >>> I created a second webrev : > >>> > >>> http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.1/ > >>> > >>> - it adds the diff provided by you(hope that’s fine with you) > >> Yes, that's fine with me. > >> There could be only one author ;) > >> > >>> -changes 2 launcherssrc/java.base/share/native/launcher/main.c > and > >> src/jdk.pack/share/native/unpack200/main.cppwhere we face similar > >> issues after mapfile removal for exes > >> > >> I'd rather remove both JNIEXPORT and JNICALL from main(). > >> It wasn't exported, and it shouldn't be. > >> > >> Regards, > >> Alexey > >> > >>> Best regards , Matthias
Re: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
Hi Matthias, On 10/04/2018 11:14, Baesken, Matthias wrote: Hello, I had to do another small adjustment to make jimage.hpp/cpp match. Please review : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/ JIMAGE_FindResource doesn't have JNICALL modifier now, does it? I've successfully built 32 bit Windows with your patch. Please remove JNIEXPORT from main(): src/java.base/share/native/launcher/main.c src/jdk.pack/share/native/unpack200/main.cpp With the latest webrev I could finallybuildjdk/jdk successfully on both win32bit and win64 bit . Thanks again to Alexey to provide the incorporated patch . You can reference both yourself and me as Contributed-by: mbaesken, aivanov when pushing the changeset if you don't mind. Regards, Alexey Best regards, Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Montag, 9. April 2018 17:14 To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 Hi Matthias, On 09/04/2018 15:38, Baesken, Matthias wrote: Hi Alexey,thanks for the diff provided by you, and for the explanations . I created a second webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.1/ - it adds the diff provided by you(hope that’s fine with you) Yes, that's fine with me. There could be only one author ;) -changes 2 launcherssrc/java.base/share/native/launcher/main.cand src/jdk.pack/share/native/unpack200/main.cppwhere we face similar issues after mapfile removal for exes I'd rather remove both JNIEXPORT and JNICALL from main(). It wasn't exported, and it shouldn't be. Regards, Alexey Best regards , Matthias
Re: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
> 10 apr. 2018 kl. 12:14 skrev Baesken, Matthias <matthias.baes...@sap.com>: > > Hello, I had to do another small adjustment to make jimage.hpp/cpp match. >Please review : > > http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/ Looks good to me. /Magnus > > With the latest webrev I could finallybuildjdk/jdk successfully on > both win32bit and win64 bit . > > > > Thanks again to Alexey to provide the incorporated patch . > > > Best regards, Matthias > > > >> -Original Message- >> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] >> Sent: Montag, 9. April 2018 17:14 >> To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie >> <magnus.ihse.bur...@oracle.com> >> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin >> <martin.do...@sap.com> >> 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 >> >> Hi Matthias, >> >>> On 09/04/2018 15:38, Baesken, Matthias wrote: >>> Hi Alexey,thanks for the diff provided by you, and for the >>> explanations >> . >>> >>> I created a second webrev : >>> >>> http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.1/ >>> >>> - it adds the diff provided by you(hope that’s fine with you) >> >> Yes, that's fine with me. >> There could be only one author ;) >> >>> -changes 2 launcherssrc/java.base/share/native/launcher/main.c >>> and >> src/jdk.pack/share/native/unpack200/main.cppwhere we face similar >> issues after mapfile removal for exes >> >> I'd rather remove both JNIEXPORT and JNICALL from main(). >> It wasn't exported, and it shouldn't be. >> >> Regards, >> Alexey >> >>> >>> >>> >>> Best regards , Matthias >>> >>> >>>> -----Original Message- >>>> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] >>>> Sent: Montag, 9. April 2018 15:52 >>>> To: Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com>; Baesken, >>>> Matthias <matthias.baes...@sap.com> >>>> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin >>>> <martin.do...@sap.com> >>>> 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 >>>> >>>> Hi Matthias, Magnus, >>>> >>>> The problem is with JNICALL added to the functions. JNICALL expands to >>>> __stdcall [1] on Windows. On 64 bit, the modifier has no effect and is >>>> ignored. On 32 bit, the modifier changes the way parameters are pass and >>>> the function name is decorated with an underscore and with @ + the size >>>> of arguments. >>>> >>>> If I remove JNICALL modifier from the exported functions, they're >>>> exported with plain name as in source code (plus, __cdecl [2] calling >>>> convention is used.) >>>> >>>> zip.dll and jimage.dll are affected by this. It's because the exported >>>> functions are looked up by name rather than using a header file with >>>> JNIIMPORT. See >>>> >> http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla >>>> ssLoader.cpp#l1155 >>>> >> http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla >>>> ssLoader.cpp#l1194 >>>> >>>> JNICALL modifier must also be removed from type declarations for >>>> functions from zip.dll: >>>> >> http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla >>>> ssLoader.cpp#l81 >>>> >>>> I'm attaching the patch to zip and jimage as well as classLoader.cpp >>>> which takes these changes into account. It does not replace Matthias' >>>> patch but complements it. >>>> >>>> >>>> Alternatively, if keeping JNICALL / __stdcall, it's possible to use >>>> pragma comments [3][4] to export undecorated names. But this is >> compiler >>>> specific and may require if's for other platforms. >>>> >>>> >
RE: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
Hi Matthias, thank you for providing the fix. I think these issues should really get fixed regardless if 32 bit is supported or not. It's not good to have inconsistent JNI declarations. The fix looks good to me. I guess it should get pushed to the submission forest because it contains hotspot changes. Or did Oracle run the necessary tests? Best regards, Martin -Original Message- From: Baesken, Matthias Sent: Dienstag, 10. April 2018 12:15 To: Alexey Ivanov <alexey.iva...@oracle.com>; Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 Hello, I had to do another small adjustment to make jimage.hpp/cpp match. Please review : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/ With the latest webrev I could finallybuildjdk/jdk successfully on both win32bit and win64 bit . Thanks again to Alexey to provide the incorporated patch . Best regards, Matthias > -Original Message- > From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] > Sent: Montag, 9. April 2018 17:14 > To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie > <magnus.ihse.bur...@oracle.com> > Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin > <martin.do...@sap.com> > 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 > > Hi Matthias, > > On 09/04/2018 15:38, Baesken, Matthias wrote: > > Hi Alexey,thanks for the diff provided by you, and for the > > explanations > . > > > > I created a second webrev : > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.1/ > > > > - it adds the diff provided by you(hope that’s fine with you) > > Yes, that's fine with me. > There could be only one author ;) > > > -changes 2 launcherssrc/java.base/share/native/launcher/main.c > > and > src/jdk.pack/share/native/unpack200/main.cppwhere we face similar > issues after mapfile removal for exes > > I'd rather remove both JNIEXPORT and JNICALL from main(). > It wasn't exported, and it shouldn't be. > > Regards, > Alexey > > > > > > > > > Best regards , Matthias > > > > > >> -Original Message- > >> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] > >> Sent: Montag, 9. April 2018 15:52 > >> To: Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com>; Baesken, > >> Matthias <matthias.baes...@sap.com> > >> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin > >> <martin.do...@sap.com> > >> 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 > >> > >> Hi Matthias, Magnus, > >> > >> The problem is with JNICALL added to the functions. JNICALL expands to > >> __stdcall [1] on Windows. On 64 bit, the modifier has no effect and is > >> ignored. On 32 bit, the modifier changes the way parameters are pass and > >> the function name is decorated with an underscore and with @ + the size > >> of arguments. > >> > >> If I remove JNICALL modifier from the exported functions, they're > >> exported with plain name as in source code (plus, __cdecl [2] calling > >> convention is used.) > >> > >> zip.dll and jimage.dll are affected by this. It's because the exported > >> functions are looked up by name rather than using a header file with > >> JNIIMPORT. See > >> > http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla > >> ssLoader.cpp#l1155 > >> > http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla > >> ssLoader.cpp#l1194 > >> > >> JNICALL modifier must also be removed from type declarations for > >> functions from zip.dll: > >> > http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla > >> ssLoader.cpp#l81 > >> > >> I'm attaching the patch to zip and jimage as well as classLoader.cpp > >> which takes these changes into account. It does not replace Matthias' > >&g
RE: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
Hello, I had to do another small adjustment to make jimage.hpp/cpp match. Please review : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/ With the latest webrev I could finallybuildjdk/jdk successfully on both win32bit and win64 bit . Thanks again to Alexey to provide the incorporated patch . Best regards, Matthias > -Original Message- > From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] > Sent: Montag, 9. April 2018 17:14 > To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie > <magnus.ihse.bur...@oracle.com> > Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin > <martin.do...@sap.com> > 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 > > Hi Matthias, > > On 09/04/2018 15:38, Baesken, Matthias wrote: > > Hi Alexey,thanks for the diff provided by you, and for the > > explanations > . > > > > I created a second webrev : > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.1/ > > > > - it adds the diff provided by you(hope that’s fine with you) > > Yes, that's fine with me. > There could be only one author ;) > > > -changes 2 launcherssrc/java.base/share/native/launcher/main.c > > and > src/jdk.pack/share/native/unpack200/main.cppwhere we face similar > issues after mapfile removal for exes > > I'd rather remove both JNIEXPORT and JNICALL from main(). > It wasn't exported, and it shouldn't be. > > Regards, > Alexey > > > > > > > > > Best regards , Matthias > > > > > >> -Original Message- > >> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] > >> Sent: Montag, 9. April 2018 15:52 > >> To: Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com>; Baesken, > >> Matthias <matthias.baes...@sap.com> > >> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin > >> <martin.do...@sap.com> > >> 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 > >> > >> Hi Matthias, Magnus, > >> > >> The problem is with JNICALL added to the functions. JNICALL expands to > >> __stdcall [1] on Windows. On 64 bit, the modifier has no effect and is > >> ignored. On 32 bit, the modifier changes the way parameters are pass and > >> the function name is decorated with an underscore and with @ + the size > >> of arguments. > >> > >> If I remove JNICALL modifier from the exported functions, they're > >> exported with plain name as in source code (plus, __cdecl [2] calling > >> convention is used.) > >> > >> zip.dll and jimage.dll are affected by this. It's because the exported > >> functions are looked up by name rather than using a header file with > >> JNIIMPORT. See > >> > http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla > >> ssLoader.cpp#l1155 > >> > http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla > >> ssLoader.cpp#l1194 > >> > >> JNICALL modifier must also be removed from type declarations for > >> functions from zip.dll: > >> > http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla > >> ssLoader.cpp#l81 > >> > >> I'm attaching the patch to zip and jimage as well as classLoader.cpp > >> which takes these changes into account. It does not replace Matthias' > >> patch but complements it. > >> > >> > >> Alternatively, if keeping JNICALL / __stdcall, it's possible to use > >> pragma comments [3][4] to export undecorated names. But this is > compiler > >> specific and may require if's for other platforms. > >> > >> > >> Regards, > >> Alexey > >> > >> [1] https://msdn.microsoft.com/en-us/library/zxk0tw93.aspx > >> [2] https://msdn.microsoft.com/en-us/library/zkwh89ks.aspx > >> [3] https://docs.microsoft.com/en-ie/cpp/build/reference/exports > >> [4] https://docs.microsoft.com/en-ie/cpp/preprocessor/comment-c-cpp > >> > >> On 09/04/2018 12:42, Magnus Ihse Bursie wrote: > >>> Those were added by my patch that removed the map files. > >>> > >>&
Re: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
Hi Matthias, On 09/04/2018 15:38, Baesken, Matthias wrote: Hi Alexey,thanks for the diff provided by you, and for the explanations . I created a second webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.1/ - it adds the diff provided by you(hope that’s fine with you) Yes, that's fine with me. There could be only one author ;) -changes 2 launcherssrc/java.base/share/native/launcher/main.cand src/jdk.pack/share/native/unpack200/main.cppwhere we face similar issues after mapfile removal for exes I'd rather remove both JNIEXPORT and JNICALL from main(). It wasn't exported, and it shouldn't be. Regards, Alexey Best regards , Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Montag, 9. April 2018 15:52 To: Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com>; Baesken, Matthias <matthias.baes...@sap.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> 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 Hi Matthias, Magnus, The problem is with JNICALL added to the functions. JNICALL expands to __stdcall [1] on Windows. On 64 bit, the modifier has no effect and is ignored. On 32 bit, the modifier changes the way parameters are pass and the function name is decorated with an underscore and with @ + the size of arguments. If I remove JNICALL modifier from the exported functions, they're exported with plain name as in source code (plus, __cdecl [2] calling convention is used.) zip.dll and jimage.dll are affected by this. It's because the exported functions are looked up by name rather than using a header file with JNIIMPORT. See http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla ssLoader.cpp#l1155 http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla ssLoader.cpp#l1194 JNICALL modifier must also be removed from type declarations for functions from zip.dll: http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla ssLoader.cpp#l81 I'm attaching the patch to zip and jimage as well as classLoader.cpp which takes these changes into account. It does not replace Matthias' patch but complements it. Alternatively, if keeping JNICALL / __stdcall, it's possible to use pragma comments [3][4] to export undecorated names. But this is compiler specific and may require if's for other platforms. Regards, Alexey [1] https://msdn.microsoft.com/en-us/library/zxk0tw93.aspx [2] https://msdn.microsoft.com/en-us/library/zkwh89ks.aspx [3] https://docs.microsoft.com/en-ie/cpp/build/reference/exports [4] https://docs.microsoft.com/en-ie/cpp/preprocessor/comment-c-cpp On 09/04/2018 12:42, Magnus Ihse Bursie wrote: Those were added by my patch that removed the map files. /Magnus 9 apr. 2018 kl. 13:38 skrev Baesken, Matthias <matthias.baes...@sap.com>: I did not add JNICALL decorations to any libzip functions , please see my webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226/ , the problem here is the added JNICALL decoration, which affects only win32 (and incorrectly, that is). so I wonder which added JNICALL decoration you are refering to . Best regards, Matthias -Original Message- From: Magnus Ihse Bursie [mailto:magnus.ihse.bur...@oracle.com] Sent: Montag, 9. April 2018 13:29 To: Baesken, Matthias <matthias.baes...@sap.com> Cc: Alexey Ivanov <alexey.iva...@oracle.com>; build-dev ; Doerr, Martin <martin.do...@sap.com> 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 Reinstating the -export command line options is not the way forward here. As I understood it from private conversation with Alexey, the problem here is the added JNICALL decoration, which affects only win32 (and incorrectly, that is). /Magnus
RE: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
Hi Alexey,thanks for the diff provided by you, and for the explanations . I created a second webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.1/ - it adds the diff provided by you(hope that’s fine with you) -changes 2 launcherssrc/java.base/share/native/launcher/main.cand src/jdk.pack/share/native/unpack200/main.cppwhere we face similar issues after mapfile removal for exes Best regards , Matthias > -Original Message- > From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] > Sent: Montag, 9. April 2018 15:52 > To: Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com>; Baesken, > Matthias <matthias.baes...@sap.com> > Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin > <martin.do...@sap.com> > 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 > > Hi Matthias, Magnus, > > The problem is with JNICALL added to the functions. JNICALL expands to > __stdcall [1] on Windows. On 64 bit, the modifier has no effect and is > ignored. On 32 bit, the modifier changes the way parameters are pass and > the function name is decorated with an underscore and with @ + the size > of arguments. > > If I remove JNICALL modifier from the exported functions, they're > exported with plain name as in source code (plus, __cdecl [2] calling > convention is used.) > > zip.dll and jimage.dll are affected by this. It's because the exported > functions are looked up by name rather than using a header file with > JNIIMPORT. See > http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla > ssLoader.cpp#l1155 > http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla > ssLoader.cpp#l1194 > > JNICALL modifier must also be removed from type declarations for > functions from zip.dll: > http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla > ssLoader.cpp#l81 > > I'm attaching the patch to zip and jimage as well as classLoader.cpp > which takes these changes into account. It does not replace Matthias' > patch but complements it. > > > Alternatively, if keeping JNICALL / __stdcall, it's possible to use > pragma comments [3][4] to export undecorated names. But this is compiler > specific and may require if's for other platforms. > > > Regards, > Alexey > > [1] https://msdn.microsoft.com/en-us/library/zxk0tw93.aspx > [2] https://msdn.microsoft.com/en-us/library/zkwh89ks.aspx > [3] https://docs.microsoft.com/en-ie/cpp/build/reference/exports > [4] https://docs.microsoft.com/en-ie/cpp/preprocessor/comment-c-cpp > > On 09/04/2018 12:42, Magnus Ihse Bursie wrote: > > Those were added by my patch that removed the map files. > > > > /Magnus > > > >> 9 apr. 2018 kl. 13:38 skrev Baesken, Matthias > <matthias.baes...@sap.com>: > >> > >> I did not add JNICALL decorations to any libzip functions , please > >> see > my webrev : > >> > >> http://cr.openjdk.java.net/~mbaesken/webrevs/8201226/ > >> > >>> , the problem here > >>> is the added JNICALL decoration, which affects only win32 (and > incorrectly, > >>> that is). > >> so I wonder which added JNICALL decoration you are refering to . > >> > >> Best regards, Matthias > >> > >> > >>> -Original Message- > >>> From: Magnus Ihse Bursie [mailto:magnus.ihse.bur...@oracle.com] > >>> Sent: Montag, 9. April 2018 13:29 > >>> To: Baesken, Matthias <matthias.baes...@sap.com> > >>> Cc: Alexey Ivanov <alexey.iva...@oracle.com>; build-dev >>> d...@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> > >>> 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 > >>> > >>> Reinstating the -export command line options is not the way forward > here. > >>> As I understood it from private conversation with Alexey, the problem > here > >>> is the added JNICALL decoration, which affects only win32 (and > incorrectly, > >>> that is). > >>> > >>> /Magnus > >>> > >>> 9 apr. 2018 kl. 12:46 skrev Baesken, Matthias > <matthias.baes...@sap.com>: > >>> > >>>>> I think adding prototype of ZIP_CRC32
Re: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
Hi Matthias, Magnus, The problem is with JNICALL added to the functions. JNICALL expands to __stdcall [1] on Windows. On 64 bit, the modifier has no effect and is ignored. On 32 bit, the modifier changes the way parameters are pass and the function name is decorated with an underscore and with @ + the size of arguments. If I remove JNICALL modifier from the exported functions, they're exported with plain name as in source code (plus, __cdecl [2] calling convention is used.) zip.dll and jimage.dll are affected by this. It's because the exported functions are looked up by name rather than using a header file with JNIIMPORT. See http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/classLoader.cpp#l1155 http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/classLoader.cpp#l1194 JNICALL modifier must also be removed from type declarations for functions from zip.dll: http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/classLoader.cpp#l81 I'm attaching the patch to zip and jimage as well as classLoader.cpp which takes these changes into account. It does not replace Matthias' patch but complements it. Alternatively, if keeping JNICALL / __stdcall, it's possible to use pragma comments [3][4] to export undecorated names. But this is compiler specific and may require if's for other platforms. Regards, Alexey [1] https://msdn.microsoft.com/en-us/library/zxk0tw93.aspx [2] https://msdn.microsoft.com/en-us/library/zkwh89ks.aspx [3] https://docs.microsoft.com/en-ie/cpp/build/reference/exports [4] https://docs.microsoft.com/en-ie/cpp/preprocessor/comment-c-cpp On 09/04/2018 12:42, Magnus Ihse Bursie wrote: Those were added by my patch that removed the map files. /Magnus 9 apr. 2018 kl. 13:38 skrev Baesken, Matthias <matthias.baes...@sap.com>: I did not add JNICALL decorations to any libzip functions , please see my webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226/ , the problem here is the added JNICALL decoration, which affects only win32 (and incorrectly, that is). so I wonder which added JNICALL decoration you are refering to . Best regards, Matthias -Original Message- From: Magnus Ihse Bursie [mailto:magnus.ihse.bur...@oracle.com] Sent: Montag, 9. April 2018 13:29 To: Baesken, Matthias <matthias.baes...@sap.com> Cc: Alexey Ivanov <alexey.iva...@oracle.com>; build-dev ; Doerr, Martin <martin.do...@sap.com> 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 Reinstating the -export command line options is not the way forward here. As I understood it from private conversation with Alexey, the problem here is the added JNICALL decoration, which affects only win32 (and incorrectly, that is). /Magnus 9 apr. 2018 kl. 12:46 skrev Baesken, Matthias <matthias.baes...@sap.com>: I think adding prototype of ZIP_CRC32 to zip_util.h is unnecessary as this function is correctly decorated in CRC32.c and is exported. Hi Alexey, you are correct on this one . The added declaration does not help with the "Corrupted ZIP library" error . This one can be fixed by bringing back the exports in the makefile make/lib/CoreLibraries.gmk(same for some JIMAGE functions) : --- a/make/lib/CoreLibraries.gmk Sun Apr 08 17:01:20 2018 +0800 +++ b/make/lib/CoreLibraries.gmk Mon Apr 09 12:44:08 2018 +0200 @@ -191,6 +191,9 @@ DISABLED_WARNINGS_gcc := implicit-fallthrough, \ LDFLAGS := $(LDFLAGS_JDKLIB) \ $(call SET_SHARED_LIBRARY_ORIGIN), \ +LDFLAGS_windows := -export:ZIP_Open -export:ZIP_Close - export:ZIP_FindEntry \ +-export:ZIP_ReadEntry -export:ZIP_GetNextEntry \ +-export:ZIP_InflateFully -export:ZIP_CRC32 -export:ZIP_FreeEntry, \ LIBS_unix := -ljvm -ljava $(LIBZ_LIBS), \ LIBS_windows := jvm.lib $(WIN_JAVA_LIB), \ )) @@ -221,6 +224,10 @@ CFLAGS_unix := -UDEBUG, \ LDFLAGS := $(LDFLAGS_JDKLIB) $(LDFLAGS_CXX_JDK) \ $(call SET_SHARED_LIBRARY_ORIGIN), \ +LDFLAGS_windows := -export:JIMAGE_Open -export:JIMAGE_Close \ +-export:JIMAGE_PackageToModule \ +-export:JIMAGE_FindResource -export:JIMAGE_GetResource \ +-export:JIMAGE_ResourceIterator -export:JIMAGE_ResourcePath, \ LIBS_unix := -ljvm -ldl $(LIBCXX), \ LIBS_macosx := -lc++, \ LIBS_windows := jvm.lib, \ I wonder why the 64bit windows build can live without the exports , any ideas ? Best regards , Matthias -Original Message- From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] Sent: Samstag, 7. April 2018 00:14 To: Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com>; Baesken, Matthias <matthias.baes...@sap.com> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com>
Re: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
Also, I'm not sure if I said it, but I think your original webrev looks good. /Magnus > 9 apr. 2018 kl. 13:38 skrev Baesken, Matthias <matthias.baes...@sap.com>: > > I did not add JNICALL decorations to any libzip functions , please see > my webrev : > > http://cr.openjdk.java.net/~mbaesken/webrevs/8201226/ > >> , the problem here >> is the added JNICALL decoration, which affects only win32 (and incorrectly, >> that is). > > so I wonder which added JNICALL decoration you are refering to . > > Best regards, Matthias > > >> -Original Message- >> From: Magnus Ihse Bursie [mailto:magnus.ihse.bur...@oracle.com] >> Sent: Montag, 9. April 2018 13:29 >> To: Baesken, Matthias <matthias.baes...@sap.com> >> Cc: Alexey Ivanov <alexey.iva...@oracle.com>; build-dev > d...@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> >> 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 >> >> Reinstating the -export command line options is not the way forward here. >> As I understood it from private conversation with Alexey, the problem here >> is the added JNICALL decoration, which affects only win32 (and incorrectly, >> that is). >> >> /Magnus >> >> 9 apr. 2018 kl. 12:46 skrev Baesken, Matthias <matthias.baes...@sap.com>: >> >>>> >>>> I think adding prototype of ZIP_CRC32 to zip_util.h is unnecessary as >>>> this function is correctly decorated in CRC32.c and is exported. >>>> >>> >>> Hi Alexey, you are correct on this one . The added declaration does >>> not >> help with the "Corrupted ZIP library" error . >>> This one can be fixed by bringing back the exports in the makefile >> make/lib/CoreLibraries.gmk(same for some JIMAGE functions) : >>> >>> >>> --- a/make/lib/CoreLibraries.gmk Sun Apr 08 17:01:20 2018 +0800 >>> +++ b/make/lib/CoreLibraries.gmk Mon Apr 09 12:44:08 2018 +0200 >>> @@ -191,6 +191,9 @@ >>>DISABLED_WARNINGS_gcc := implicit-fallthrough, \ >>>LDFLAGS := $(LDFLAGS_JDKLIB) \ >>>$(call SET_SHARED_LIBRARY_ORIGIN), \ >>> +LDFLAGS_windows := -export:ZIP_Open -export:ZIP_Close - >> export:ZIP_FindEntry \ >>> +-export:ZIP_ReadEntry -export:ZIP_GetNextEntry \ >>> +-export:ZIP_InflateFully -export:ZIP_CRC32 -export:ZIP_FreeEntry, \ >>>LIBS_unix := -ljvm -ljava $(LIBZ_LIBS), \ >>>LIBS_windows := jvm.lib $(WIN_JAVA_LIB), \ >>> )) >>> @@ -221,6 +224,10 @@ >>>CFLAGS_unix := -UDEBUG, \ >>>LDFLAGS := $(LDFLAGS_JDKLIB) $(LDFLAGS_CXX_JDK) \ >>>$(call SET_SHARED_LIBRARY_ORIGIN), \ >>> +LDFLAGS_windows := -export:JIMAGE_Open -export:JIMAGE_Close \ >>> +-export:JIMAGE_PackageToModule \ >>> +-export:JIMAGE_FindResource -export:JIMAGE_GetResource \ >>> +-export:JIMAGE_ResourceIterator -export:JIMAGE_ResourcePath, \ >>>LIBS_unix := -ljvm -ldl $(LIBCXX), \ >>>LIBS_macosx := -lc++, \ >>>LIBS_windows := jvm.lib, \ >>> >>> >>> I wonder why the 64bit windows build can live without the exports , any >> ideas ? >>> >>> >>> Best regards , Matthias >>> >>> >>>> -Original Message- >>>> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] >>>> Sent: Samstag, 7. April 2018 00:14 >>>> To: Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com>; Baesken, >>>> Matthias <matthias.baes...@sap.com> >>>> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin >>>> <martin.do...@sap.com> >>>> 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 >>>> >>>> Hi Magnus, Matthias, >>>> >>>> I tried to build 32 bit Windows but it fails to run for me with >>>> Corrupted ZIP library: >>>> c:\work\jdk-dev\build\windows-x86-normal-server- >> release\jdk\bin\zip.dll >>>> >>>> The problem is that zip.dll exports all symbols as C++ rather than C. >>>> For example, ZIP_CRC32 is exported as _ZIP_C
Re: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
Those were added by my patch that removed the map files. /Magnus > 9 apr. 2018 kl. 13:38 skrev Baesken, Matthias <matthias.baes...@sap.com>: > > I did not add JNICALL decorations to any libzip functions , please see > my webrev : > > http://cr.openjdk.java.net/~mbaesken/webrevs/8201226/ > >> , the problem here >> is the added JNICALL decoration, which affects only win32 (and incorrectly, >> that is). > > so I wonder which added JNICALL decoration you are refering to . > > Best regards, Matthias > > >> -Original Message- >> From: Magnus Ihse Bursie [mailto:magnus.ihse.bur...@oracle.com] >> Sent: Montag, 9. April 2018 13:29 >> To: Baesken, Matthias <matthias.baes...@sap.com> >> Cc: Alexey Ivanov <alexey.iva...@oracle.com>; build-dev > d...@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> >> 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 >> >> Reinstating the -export command line options is not the way forward here. >> As I understood it from private conversation with Alexey, the problem here >> is the added JNICALL decoration, which affects only win32 (and incorrectly, >> that is). >> >> /Magnus >> >> 9 apr. 2018 kl. 12:46 skrev Baesken, Matthias <matthias.baes...@sap.com>: >> >>>> >>>> I think adding prototype of ZIP_CRC32 to zip_util.h is unnecessary as >>>> this function is correctly decorated in CRC32.c and is exported. >>>> >>> >>> Hi Alexey, you are correct on this one . The added declaration does >>> not >> help with the "Corrupted ZIP library" error . >>> This one can be fixed by bringing back the exports in the makefile >> make/lib/CoreLibraries.gmk(same for some JIMAGE functions) : >>> >>> >>> --- a/make/lib/CoreLibraries.gmk Sun Apr 08 17:01:20 2018 +0800 >>> +++ b/make/lib/CoreLibraries.gmk Mon Apr 09 12:44:08 2018 +0200 >>> @@ -191,6 +191,9 @@ >>>DISABLED_WARNINGS_gcc := implicit-fallthrough, \ >>>LDFLAGS := $(LDFLAGS_JDKLIB) \ >>>$(call SET_SHARED_LIBRARY_ORIGIN), \ >>> +LDFLAGS_windows := -export:ZIP_Open -export:ZIP_Close - >> export:ZIP_FindEntry \ >>> +-export:ZIP_ReadEntry -export:ZIP_GetNextEntry \ >>> +-export:ZIP_InflateFully -export:ZIP_CRC32 -export:ZIP_FreeEntry, \ >>>LIBS_unix := -ljvm -ljava $(LIBZ_LIBS), \ >>>LIBS_windows := jvm.lib $(WIN_JAVA_LIB), \ >>> )) >>> @@ -221,6 +224,10 @@ >>>CFLAGS_unix := -UDEBUG, \ >>>LDFLAGS := $(LDFLAGS_JDKLIB) $(LDFLAGS_CXX_JDK) \ >>>$(call SET_SHARED_LIBRARY_ORIGIN), \ >>> +LDFLAGS_windows := -export:JIMAGE_Open -export:JIMAGE_Close \ >>> +-export:JIMAGE_PackageToModule \ >>> +-export:JIMAGE_FindResource -export:JIMAGE_GetResource \ >>> +-export:JIMAGE_ResourceIterator -export:JIMAGE_ResourcePath, \ >>>LIBS_unix := -ljvm -ldl $(LIBCXX), \ >>>LIBS_macosx := -lc++, \ >>>LIBS_windows := jvm.lib, \ >>> >>> >>> I wonder why the 64bit windows build can live without the exports , any >> ideas ? >>> >>> >>> Best regards , Matthias >>> >>> >>>> -Original Message- >>>> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] >>>> Sent: Samstag, 7. April 2018 00:14 >>>> To: Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com>; Baesken, >>>> Matthias <matthias.baes...@sap.com> >>>> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin >>>> <martin.do...@sap.com> >>>> 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 >>>> >>>> Hi Magnus, Matthias, >>>> >>>> I tried to build 32 bit Windows but it fails to run for me with >>>> Corrupted ZIP library: >>>> c:\work\jdk-dev\build\windows-x86-normal-server- >> release\jdk\bin\zip.dll >>>> >>>> The problem is that zip.dll exports all symbols as C++ rather than C. >>>> For example, ZIP_CRC32 is exported as _ZIP_CRC32@12, and &
RE: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
I did not add JNICALL decorations to any libzip functions , please see my webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8201226/ > , the problem here > is the added JNICALL decoration, which affects only win32 (and incorrectly, > that is). so I wonder which added JNICALL decoration you are refering to . Best regards, Matthias > -Original Message- > From: Magnus Ihse Bursie [mailto:magnus.ihse.bur...@oracle.com] > Sent: Montag, 9. April 2018 13:29 > To: Baesken, Matthias <matthias.baes...@sap.com> > Cc: Alexey Ivanov <alexey.iva...@oracle.com>; build-dev d...@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com> > 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 > > Reinstating the -export command line options is not the way forward here. > As I understood it from private conversation with Alexey, the problem here > is the added JNICALL decoration, which affects only win32 (and incorrectly, > that is). > > /Magnus > > 9 apr. 2018 kl. 12:46 skrev Baesken, Matthias <matthias.baes...@sap.com>: > > >> > >> I think adding prototype of ZIP_CRC32 to zip_util.h is unnecessary as > >> this function is correctly decorated in CRC32.c and is exported. > >> > > > > Hi Alexey, you are correct on this one . The added declaration does > > not > help with the "Corrupted ZIP library" error . > > This one can be fixed by bringing back the exports in the makefile > make/lib/CoreLibraries.gmk(same for some JIMAGE functions) : > > > > > > --- a/make/lib/CoreLibraries.gmk Sun Apr 08 17:01:20 2018 +0800 > > +++ b/make/lib/CoreLibraries.gmk Mon Apr 09 12:44:08 2018 +0200 > > @@ -191,6 +191,9 @@ > > DISABLED_WARNINGS_gcc := implicit-fallthrough, \ > > LDFLAGS := $(LDFLAGS_JDKLIB) \ > > $(call SET_SHARED_LIBRARY_ORIGIN), \ > > +LDFLAGS_windows := -export:ZIP_Open -export:ZIP_Close - > export:ZIP_FindEntry \ > > +-export:ZIP_ReadEntry -export:ZIP_GetNextEntry \ > > +-export:ZIP_InflateFully -export:ZIP_CRC32 -export:ZIP_FreeEntry, \ > > LIBS_unix := -ljvm -ljava $(LIBZ_LIBS), \ > > LIBS_windows := jvm.lib $(WIN_JAVA_LIB), \ > > )) > > @@ -221,6 +224,10 @@ > > CFLAGS_unix := -UDEBUG, \ > > LDFLAGS := $(LDFLAGS_JDKLIB) $(LDFLAGS_CXX_JDK) \ > > $(call SET_SHARED_LIBRARY_ORIGIN), \ > > +LDFLAGS_windows := -export:JIMAGE_Open -export:JIMAGE_Close \ > > +-export:JIMAGE_PackageToModule \ > > +-export:JIMAGE_FindResource -export:JIMAGE_GetResource \ > > +-export:JIMAGE_ResourceIterator -export:JIMAGE_ResourcePath, \ > > LIBS_unix := -ljvm -ldl $(LIBCXX), \ > > LIBS_macosx := -lc++, \ > > LIBS_windows := jvm.lib, \ > > > > > > I wonder why the 64bit windows build can live without the exports , any > ideas ? > > > > > > Best regards , Matthias > > > > > >> -----Original Message----- > >> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] > >> Sent: Samstag, 7. April 2018 00:14 > >> To: Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com>; Baesken, > >> Matthias <matthias.baes...@sap.com> > >> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin > >> <martin.do...@sap.com> > >> 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 > >> > >> Hi Magnus, Matthias, > >> > >> I tried to build 32 bit Windows but it fails to run for me with > >> Corrupted ZIP library: > >> c:\work\jdk-dev\build\windows-x86-normal-server- > release\jdk\bin\zip.dll > >> > >> The problem is that zip.dll exports all symbols as C++ rather than C. > >> For example, ZIP_CRC32 is exported as _ZIP_CRC32@12, and > classLoader.cpp > >> cannot find the function. > >> > >> > >> I think adding prototype of ZIP_CRC32 to zip_util.h is unnecessary as > >> this function is correctly decorated in CRC32.c and is exported. > >> > >> Regards, > >> Alexey > >> > >>> On 06/04/2018 17:33, Magnus Ihse Bursie wrote: > >>> I think it's reasonable to update the existing webrev. > >>> > >>> /Magnus > >>&
Re: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
Reinstating the -export command line options is not the way forward here. As I understood it from private conversation with Alexey, the problem here is the added JNICALL decoration, which affects only win32 (and incorrectly, that is). /Magnus 9 apr. 2018 kl. 12:46 skrev Baesken, Matthias <matthias.baes...@sap.com>: >> >> I think adding prototype of ZIP_CRC32 to zip_util.h is unnecessary as >> this function is correctly decorated in CRC32.c and is exported. >> > > Hi Alexey, you are correct on this one . The added declaration does > not help with the "Corrupted ZIP library" error . > This one can be fixed by bringing back the exports in the makefile > make/lib/CoreLibraries.gmk(same for some JIMAGE functions) : > > > --- a/make/lib/CoreLibraries.gmk Sun Apr 08 17:01:20 2018 +0800 > +++ b/make/lib/CoreLibraries.gmk Mon Apr 09 12:44:08 2018 +0200 > @@ -191,6 +191,9 @@ > DISABLED_WARNINGS_gcc := implicit-fallthrough, \ > LDFLAGS := $(LDFLAGS_JDKLIB) \ > $(call SET_SHARED_LIBRARY_ORIGIN), \ > +LDFLAGS_windows := -export:ZIP_Open -export:ZIP_Close > -export:ZIP_FindEntry \ > +-export:ZIP_ReadEntry -export:ZIP_GetNextEntry \ > +-export:ZIP_InflateFully -export:ZIP_CRC32 -export:ZIP_FreeEntry, \ > LIBS_unix := -ljvm -ljava $(LIBZ_LIBS), \ > LIBS_windows := jvm.lib $(WIN_JAVA_LIB), \ > )) > @@ -221,6 +224,10 @@ > CFLAGS_unix := -UDEBUG, \ > LDFLAGS := $(LDFLAGS_JDKLIB) $(LDFLAGS_CXX_JDK) \ > $(call SET_SHARED_LIBRARY_ORIGIN), \ > +LDFLAGS_windows := -export:JIMAGE_Open -export:JIMAGE_Close \ > +-export:JIMAGE_PackageToModule \ > +-export:JIMAGE_FindResource -export:JIMAGE_GetResource \ > +-export:JIMAGE_ResourceIterator -export:JIMAGE_ResourcePath, \ > LIBS_unix := -ljvm -ldl $(LIBCXX), \ > LIBS_macosx := -lc++, \ > LIBS_windows := jvm.lib, \ > > > I wonder why the 64bit windows build can live without the exports , any > ideas ? > > > Best regards , Matthias > > >> -Original Message- >> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] >> Sent: Samstag, 7. April 2018 00:14 >> To: Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com>; Baesken, >> Matthias <matthias.baes...@sap.com> >> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin >> <martin.do...@sap.com> >> 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 >> >> Hi Magnus, Matthias, >> >> I tried to build 32 bit Windows but it fails to run for me with >> Corrupted ZIP library: >> c:\work\jdk-dev\build\windows-x86-normal-server-release\jdk\bin\zip.dll >> >> The problem is that zip.dll exports all symbols as C++ rather than C. >> For example, ZIP_CRC32 is exported as _ZIP_CRC32@12, and classLoader.cpp >> cannot find the function. >> >> >> I think adding prototype of ZIP_CRC32 to zip_util.h is unnecessary as >> this function is correctly decorated in CRC32.c and is exported. >> >> Regards, >> Alexey >> >>> On 06/04/2018 17:33, Magnus Ihse Bursie wrote: >>> I think it's reasonable to update the existing webrev. >>> >>> /Magnus >>> >>>> 6 apr. 2018 kl. 15:20 skrev Baesken, Matthias >> <matthias.baes...@sap.com>: >>>> >>>> Hello, I just noticed 2 additonal issues regarding mapfile-removal : >>>> >>>> The follow up change that removed mapfiles for exes as wellleads >> 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.cThu Apr 05 14:39:04 >> 2018 -0700 >>>> +++ b/src/java.base/share/native/launcher/main.cFri Apr 06 15:16:40 >> 2018 +0200 >>>> @@ -93,7 +93
RE: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
>
> I think adding prototype of ZIP_CRC32 to zip_util.h is unnecessary as
> this function is correctly decorated in CRC32.c and is exported.
>
Hi Alexey, you are correct on this one . The added declaration does not
help with the "Corrupted ZIP library" error .
This one can be fixed by bringing back the exports in the makefile
make/lib/CoreLibraries.gmk(same for some JIMAGE functions) :
--- a/make/lib/CoreLibraries.gmk Sun Apr 08 17:01:20 2018 +0800
+++ b/make/lib/CoreLibraries.gmk Mon Apr 09 12:44:08 2018 +0200
@@ -191,6 +191,9 @@
DISABLED_WARNINGS_gcc := implicit-fallthrough, \
LDFLAGS := $(LDFLAGS_JDKLIB) \
$(call SET_SHARED_LIBRARY_ORIGIN), \
+LDFLAGS_windows := -export:ZIP_Open -export:ZIP_Close
-export:ZIP_FindEntry \
+-export:ZIP_ReadEntry -export:ZIP_GetNextEntry \
+-export:ZIP_InflateFully -export:ZIP_CRC32 -export:ZIP_FreeEntry, \
LIBS_unix := -ljvm -ljava $(LIBZ_LIBS), \
LIBS_windows := jvm.lib $(WIN_JAVA_LIB), \
))
@@ -221,6 +224,10 @@
CFLAGS_unix := -UDEBUG, \
LDFLAGS := $(LDFLAGS_JDKLIB) $(LDFLAGS_CXX_JDK) \
$(call SET_SHARED_LIBRARY_ORIGIN), \
+LDFLAGS_windows := -export:JIMAGE_Open -export:JIMAGE_Close \
+-export:JIMAGE_PackageToModule \
+-export:JIMAGE_FindResource -export:JIMAGE_GetResource \
+-export:JIMAGE_ResourceIterator -export:JIMAGE_ResourcePath, \
LIBS_unix := -ljvm -ldl $(LIBCXX), \
LIBS_macosx := -lc++, \
LIBS_windows := jvm.lib, \
I wonder why the 64bit windows build can live without the exports , any
ideas ?
Best regards , Matthias
> -Original Message-
> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com]
> Sent: Samstag, 7. April 2018 00:14
> To: Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com>; Baesken,
> Matthias <matthias.baes...@sap.com>
> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin
> <martin.do...@sap.com>
> 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
>
> Hi Magnus, Matthias,
>
> I tried to build 32 bit Windows but it fails to run for me with
> Corrupted ZIP library:
> c:\work\jdk-dev\build\windows-x86-normal-server-release\jdk\bin\zip.dll
>
> The problem is that zip.dll exports all symbols as C++ rather than C.
> For example, ZIP_CRC32 is exported as _ZIP_CRC32@12, and classLoader.cpp
> cannot find the function.
>
>
> I think adding prototype of ZIP_CRC32 to zip_util.h is unnecessary as
> this function is correctly decorated in CRC32.c and is exported.
>
> Regards,
> Alexey
>
> On 06/04/2018 17:33, Magnus Ihse Bursie wrote:
> > I think it's reasonable to update the existing webrev.
> >
> > /Magnus
> >
> >> 6 apr. 2018 kl. 15:20 skrev Baesken, Matthias
> <matthias.baes...@sap.com>:
> >>
> >> Hello, I just noticed 2 additonal issues regarding mapfile-removal :
> >>
> >> The follow up change that removed mapfiles for exes as wellleads
> 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.cThu Apr 05 14:39:04
> 2018 -0700
> >> +++ b/src/java.base/share/native/launcher/main.cFri 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;
> >>
> >>
> >> 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
Re: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
Hi Magnus, Matthias,
I tried to build 32 bit Windows but it fails to run for me with
Corrupted ZIP library:
c:\work\jdk-dev\build\windows-x86-normal-server-release\jdk\bin\zip.dll
The problem is that zip.dll exports all symbols as C++ rather than C.
For example, ZIP_CRC32 is exported as _ZIP_CRC32@12, and classLoader.cpp
cannot find the function.
I think adding prototype of ZIP_CRC32 to zip_util.h is unnecessary as
this function is correctly decorated in CRC32.c and is exported.
Regards,
Alexey
On 06/04/2018 17:33, Magnus Ihse Bursie wrote:
I think it's reasonable to update the existing webrev.
/Magnus
6 apr. 2018 kl. 15:20 skrev Baesken, Matthias <matthias.baes...@sap.com>:
Hello, I just noticed 2 additonal issues regarding mapfile-removal :
The follow up change that removed mapfiles for exes as wellleads 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.cThu Apr 05 14:39:04
2018 -0700
+++ b/src/java.base/share/native/launcher/main.cFri 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;
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 andpost 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-dev@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>
Cc: build-dev@openjdk.java.net; Doerr, Martin <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>:
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/mlib_ImageConvKernelConvert.c(87) : error C2373: 'j2d_mlib_ImageConvKernelConvert' : redefinition; different type modifiers
e:\priv\openjdk\repos\jdk\src\java.desktop\share\native\libmlib_image\mlib_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/libji
Re: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
I think it's reasonable to update the existing webrev.
/Magnus
> 6 apr. 2018 kl. 15:20 skrev Baesken, Matthias <matthias.baes...@sap.com>:
>
> Hello, I just noticed 2 additonal issues regarding mapfile-removal :
>
> The follow up change that removed mapfiles for exes as wellleads
> 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.cThu Apr 05 14:39:04
> 2018 -0700
> +++ b/src/java.base/share/native/launcher/main.cFri 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;
>
>
> 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-dev@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>
> Cc: build-dev@openjdk.java.net; Doerr, Martin <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>:
>
> 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/mlib_ImageConvKernelConvert.c(87)
> : error C2373: 'j2d_mlib_ImageConvKernelConvert' : redefinition; different
> type modifiers
>
> e:\priv\openjdk\repos\jdk\src\java.desktop\share\native\libmlib_image\mlib_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 :
>
&
RE: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations
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.cThu Apr 05 14:39:04
2018 -0700
+++ b/src/java.base/share/native/launcher/main.cFri 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 andpost
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-dev@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-dev@openjdk.java.net<mailto:build-dev@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/mlib_ImageConvKernelConvert.c(87)
: error C2373: 'j2d_mlib_ImageConvKernelConvert' : redefinition; different
type modifiers
e:\priv\openjdk\repos\jdk\src\java.desktop\share\native\libmlib_image\mlib_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", );
*/
-extern "C" JNIEXPORT JImageLocationRef JIMAGE_FindResource(JImageFile* jimage,
+extern "C" JNIEXPORT JImageLocationRef JNICALL JIMAGE_FindResource(JImageFile*
jimage,
const
