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

[Magnus Ihse Bursie]

> 10 apr. 2018 kl. 12:14 skrev Baesken, Matthias <matthias.baesken at 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 finally    build    jdk/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.ivanov at oracle.com]
>> Sent: Montag, 9. April 2018 17:14
>> To: Baesken, Matthias <matthias.baesken at sap.com>; Magnus Ihse Bursie
>> <magnus.ihse.bursie at oracle.com>
>> Cc: build-dev <build-dev at openjdk.java.net>; Doerr, Martin
>> <martin.doerr at 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
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: Alexey Ivanov [mailto:alexey.ivanov at oracle.com]
>>>> Sent: Montag, 9. April 2018 15:52
>>>> To: Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com>; Baesken,
>>>> Matthias <matthias.baesken at sap.com>
>>>> Cc: build-dev <build-dev at openjdk.java.net>; Doerr, Martin
>>>> <martin.doerr at 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.baesken at 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.bursie at oracle.com]
>>>>>>> Sent: Montag, 9. April 2018 13:29
>>>>>>> To: Baesken, Matthias <matthias.baesken at sap.com>
>>>>>>> Cc: Alexey Ivanov <alexey.ivanov at oracle.com>; build-dev <build-
>>>>>>> dev at openjdk.java.net>; Doerr, Martin <martin.doerr at 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
>>>>>>> 
>>>>>>> 
>> <SNIP>