Re: [OpenJDK 2D-Dev] 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/impleme

[Baesken, Matthias]

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 ; Baesken, Matthias
> ; Alan Bateman ;
> Magnus Ihse Bursie 
> Cc: build-...@openjdk.java.net; core-libs-...@openjdk.java.net; Doerr,
> Martin ; 2d-dev <2d-dev@openjdk.java.net>;
> hotspot-dev 
> 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 ; Magnus Ihse
> Bursie
> >>> 
> >>> Cc: build-...@openjdk.java.net; Doerr, Martin
> ;
> >>> 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
> :
> >>> 

Re: [OpenJDK 2D-Dev] Proposal: Unicode Variation Selector

[Steven R Loomis]

the webrev also looks OK to me, I did not check the math yet.
Steven R. Loomis
IBM Global Foundations Technology Team
Technical Lead, ICU for C/C++
Phone: 1-720-342-4930  https://ibm.biz/srloomis 
E-mail: srloo...@us.ibm.com
 
 
- Original message -From: Phil Race To: Toshio 5 Nakamura , 2d-dev@openjdk.java.net, Steven R Loomis , "Steven R. Loomis" Cc:Subject: Re: [OpenJDK 2D-Dev] Proposal: Unicode Variation SelectorDate: Wed, Apr 11, 2018 8:46 AM  I recommend you ask Steven Loomis to sponsor this - andprobably the input method support, but particularly this one.-phil. 
On 04/08/2018 11:46 PM, Toshio 5 Nakamura wrote:
HelloIBM would like to propose Unicode Variation Selector[1] capability to AWTand Swing components.(This proposal was posted to i18n-dev first, and I got a suggestion to discuss in 2d-dev.)This proposal changed the following files:src/java.desktop/share/classes/sun/font/CMap.javasrc/java.desktop/share/classes/sun/font/CharToGlyphMapper.javasrc/java.desktop/share/classes/sun/font/CompositeGlyphMapper.javasrc/java.desktop/share/classes/sun/font/Font2D.javasrc/java.desktop/share/classes/sun/font/FontRunIterator.javasrc/java.desktop/share/classes/sun/font/FontUtilities.javasrc/java.desktop/share/classes/sun/font/TrueTypeGlyphMapper.javasrc/java.desktop/share/native/common/font/sunfontids.hsrc/java.desktop/share/native/libfontmanager/hb-jdk-font.ccsrc/java.desktop/share/native/libfontmanager/sunFont.csrc/java.desktop/share/classes/javax/swing/text/DefaultEditorKit.java542 lines will be changed.There are three parts. 1) Adding CMap format 14 support2) Adding CharsToGlyphs functions support Variation Selector Sequences3) Swing text component's DEL and BS key operations changeHow would I go about obtaining a sponsor?[1] http://www.unicode.org/versions/Unicode10.0.0/ch23.pdf Chapter 23.4 Variation SelectorsBest regards,Toshio NakamuraIBM Japan
 

Re: [OpenJDK 2D-Dev] Proposal: Unicode Variation Selector

[Steven R Loomis]

I can sponsor this.
Steven R. Loomis
IBM Global Foundations Technology Team
Technical Lead, ICU for C/C++
Phone: 1-720-342-4930  https://ibm.biz/srloomis 
E-mail: srloo...@us.ibm.com
 
 
- Original message -From: Phil Race To: Toshio 5 Nakamura , 2d-dev@openjdk.java.net, Steven R Loomis , "Steven R. Loomis" Cc:Subject: Re: [OpenJDK 2D-Dev] Proposal: Unicode Variation SelectorDate: Wed, Apr 11, 2018 8:46 AM  I recommend you ask Steven Loomis to sponsor this - andprobably the input method support, but particularly this one.-phil. 
On 04/08/2018 11:46 PM, Toshio 5 Nakamura wrote:
HelloIBM would like to propose Unicode Variation Selector[1] capability to AWTand Swing components.(This proposal was posted to i18n-dev first, and I got a suggestion to discuss in 2d-dev.)This proposal changed the following files:src/java.desktop/share/classes/sun/font/CMap.javasrc/java.desktop/share/classes/sun/font/CharToGlyphMapper.javasrc/java.desktop/share/classes/sun/font/CompositeGlyphMapper.javasrc/java.desktop/share/classes/sun/font/Font2D.javasrc/java.desktop/share/classes/sun/font/FontRunIterator.javasrc/java.desktop/share/classes/sun/font/FontUtilities.javasrc/java.desktop/share/classes/sun/font/TrueTypeGlyphMapper.javasrc/java.desktop/share/native/common/font/sunfontids.hsrc/java.desktop/share/native/libfontmanager/hb-jdk-font.ccsrc/java.desktop/share/native/libfontmanager/sunFont.csrc/java.desktop/share/classes/javax/swing/text/DefaultEditorKit.java542 lines will be changed.There are three parts. 1) Adding CMap format 14 support2) Adding CharsToGlyphs functions support Variation Selector Sequences3) Swing text component's DEL and BS key operations changeHow would I go about obtaining a sponsor?[1] http://www.unicode.org/versions/Unicode10.0.0/ch23.pdf Chapter 23.4 Variation SelectorsBest regards,Toshio NakamuraIBM Japan
 

Re: [OpenJDK 2D-Dev] [11] RFR for JDK-8201433: Fix potential crash in BufImg_SetupICM

[Alexey Ivanov]

Hi Sergey,

Thank you for your review.

Please take a look at the updated webrev:
http://cr.openjdk.java.net/~aivanov/8201433/jdk11/webrev.01/

On 12/04/2018 22:33, Sergey Bylokhov wrote:

Hi, Alexey.
Since the test requires 1g of memory, should we use this tag?:
 * @requires os.maxMemory >= 1g
Otherwise the test may fail on start if amount of memory is not 
sufficient.


It makes sense to add it so that the test is skipped if there's not 
enough of memory.
However, I think the actual requirement is >= 2G as it needs 1G for Java 
heap and it needs native memory for images and for other purposes.


The test exhausts the available native memory to make malloc fail. The 
amount of used memory reaches close to 2G before it fails.



Regards,
Alexey



On 12/04/2018 08:25, Alexey Ivanov wrote:

Hi,

Please review the fix for jdk11:

bug: https://bugs.openjdk.java.net/browse/JDK-8201433
webrev: http://cr.openjdk.java.net/~aivanov/8201433/jdk11/webrev.00/


When the JVM is run with limited amount of memory, initCubemap 
function called from BufImg_SetupICM can return NULL. It can lead to 
a crash. It usually happens when native memory is exhausted and 
malloc fails. In this state, JVM itself is not stable.


Adding NULL-check can prevent the crash in this particular place. 
However, if the app continues, since native memory is exhausted and 
malloc fails, JVM can still crash in another place.



Thank you in advance.

Regards,
Alexey

Re: [OpenJDK 2D-Dev] 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/impleme

[Alexey Ivanov]

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 ; Magnus Ihse Bursie

Cc: build-...@openjdk.java.net; Doerr, Martin ;
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 09:54
To: 'Magnus Ihse 

Re: [OpenJDK 2D-Dev] 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/impleme

[Alexey Ivanov]

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' ; Magnus Ihse Bursie

Cc: build-dev ; Doerr, Martin

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 ; Magnus Ihse Bursie

Cc: build-dev ; Doerr, Martin

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 ; Magnus Ihse

Bursie


Cc: build-dev ; Doerr, Martin

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 ; Magnus Ihse

Bursie


Cc: build-dev ; Doerr, Martin

Subject: Re: 8201226 missing JNIEXPORT / JNICALL at some places in

function

declarations/implementations - was : RE: missing JNIEXPORT /

Re: [OpenJDK 2D-Dev] [11] RFR for JDK-8201433: Fix potential crash in BufImg_SetupICM

[Sergey Bylokhov]

Hi, Alexey.
Since the test requires 1g of memory, should we use this tag?:
 * @requires os.maxMemory >= 1g
Otherwise the test may fail on start if amount of memory is not sufficient.

On 12/04/2018 08:25, Alexey Ivanov wrote:

Hi,

Please review the fix for jdk11:

bug: https://bugs.openjdk.java.net/browse/JDK-8201433
webrev: http://cr.openjdk.java.net/~aivanov/8201433/jdk11/webrev.00/


When the JVM is run with limited amount of memory, initCubemap function 
called from BufImg_SetupICM can return NULL. It can lead to a crash. It 
usually happens when native memory is exhausted and malloc fails. In 
this state, JVM itself is not stable.


Adding NULL-check can prevent the crash in this particular place. 
However, if the app continues, since native memory is exhausted and 
malloc fails, JVM can still crash in another place.



Thank you in advance.

Regards,
Alexey



--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/impleme

[Phil Race]

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 ; Magnus Ihse Bursie

Cc: build-...@openjdk.java.net; Doerr, Martin ;
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' 
Cc: build-...@openjdk.java.net; Doerr, Martin 
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 / 

Re: [OpenJDK 2D-Dev] 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/impleme

[Phil Race]

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' ; Magnus Ihse Bursie

Cc: build-dev ; Doerr, Martin

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 ; Magnus Ihse Bursie

Cc: build-dev ; Doerr, Martin

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 ; Magnus Ihse

Bursie


Cc: build-dev ; Doerr, Martin

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 ; Magnus Ihse

Bursie


Cc: build-dev ; Doerr, Martin

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/

-   

Re: [OpenJDK 2D-Dev] [11] RFR for JDK-8201433: Fix potential crash in BufImg_SetupICM

[Phil Race]

+1

-phil.

On 04/12/2018 08:25 AM, Alexey Ivanov wrote:

Hi,

Please review the fix for jdk11:

bug: https://bugs.openjdk.java.net/browse/JDK-8201433
webrev: http://cr.openjdk.java.net/~aivanov/8201433/jdk11/webrev.00/


When the JVM is run with limited amount of memory, initCubemap 
function called from BufImg_SetupICM can return NULL. It can lead to a 
crash. It usually happens when native memory is exhausted and malloc 
fails. In this state, JVM itself is not stable.


Adding NULL-check can prevent the crash in this particular place. 
However, if the app continues, since native memory is exhausted and 
malloc fails, JVM can still crash in another place.



Thank you in advance.

Regards,
Alexey

[OpenJDK 2D-Dev] [11] RFR for JDK-8201433: Fix potential crash in BufImg_SetupICM

[Alexey Ivanov]

Hi,

Please review the fix for jdk11:

bug: https://bugs.openjdk.java.net/browse/JDK-8201433
webrev: http://cr.openjdk.java.net/~aivanov/8201433/jdk11/webrev.00/


When the JVM is run with limited amount of memory, initCubemap function 
called from BufImg_SetupICM can return NULL. It can lead to a crash. It 
usually happens when native memory is exhausted and malloc fails. In 
this state, JVM itself is not stable.


Adding NULL-check can prevent the crash in this particular place. 
However, if the app continues, since native memory is exhausted and 
malloc fails, JVM can still crash in another place.



Thank you in advance.

Regards,
Alexey