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]

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 at 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.ivanov at oracle.com]
>>> Sent: Donnerstag, 12. April 2018 23:53
>>> To: Phil Race <philip.race at oracle.com>; Baesken, Matthias
>>> <matthias.baesken at sap.com>; Alan Bateman <Alan.Bateman at oracle.com>;
>>> Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com>
>>> Cc: build-dev at openjdk.java.net; core-libs-dev at openjdk.java.net; Doerr,
>>> Martin <martin.doerr at sap.com>; 2d-dev <2d-dev at openjdk.java.net>;
>>> hotspot-dev <hotspot-dev at openjdk.java.net>
>>> Subject: Re: 8201226 missing JNIEXPORT / JNICALL at some places in 
>>> function
>>> declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at
>>> some places in function declarations/implementations
>>>
>>>
>>> On 12/04/2018 21:42, Phil Race wrote:
>>>> How can JNIEXPORT be different between 32 bit & 64 bit ?
>>>> I'm sure you saw compilation errors but I don't get why it failed for
>>>> 32 only.
>>>>
>>>> JNICALL (_stdcall) may be unnecessary on 64 bit Windows but that 
>>>> doesn't
>>>> explain why the 32 bit compiler would complain about inconsistent
>>>> application
>>>> of __declspec(dllexport) - ie JNIEXPORT.
>>>>
>>>> Or is that part (adding JNIEXPORT) pure clean up and the compilation
>>>> errors were all down to JNICALL ?
>>> Adding missing JNIEXPORT is for cleanup only.
>>>
>>> The compiler complained about mismatched JNICALL / non-JNICALL
>>> declarations as the macro changes calling convention from the default
>>> __cdecl  to __stdcall on 32 bit Windows.
>>>
>>> Another issue is that __stdcall decorates the functions: prefixes with
>>> underscore and postfixes with @ + size of parameters. Because of the
>>> decorations, classLoader.cpp can't lookup the required functions by 
>>> name
>>> from zip.dll and jimage.dll. The functions are exported but with
>>> different name.
>>>
>>> I hope this information adds more details to the picture.
>>>
>>>> I was a bit puzzled at the removals I saw here :
>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/src/java.deskto
>>> p/share/native/libsplashscreen/splashscreen_impl.h.udiff.html
>>>> .. I needed to look at the whole file to realise that you were
>>>> removing a duplicate
>>>> declaration.
>>> That was tricky. I could have been mentioned in the review.
>>>
>>>
>>> Regards,
>>> Alexey
>>>
>>>> -phil.
>>>>
>>>> On 04/12/2018 04:04 AM, Baesken, Matthias wrote:
>>>>> Hi  Alan , this is the up to date webrev .
>>>>> However we want to add   Alexey   Ivanov  as additional author .
>>>>>
>>>>>> As I read it, this changes the calling convention of these 
>>>>>> functions on
>>>>>> 32-bit Windows but it will have no impact on 64-bit Windows (as
>>>>>> __stdcall is ignored) or other platforms, is that correct?
>>>>>>
>>>>> The  change adds  JNIEXPORT   at some places  where it is ben
>>>>> forgotten , for example :
>>>>>
>>>>>
>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/src/java.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.Bateman at oracle.com]
>>>>>> Sent: Donnerstag, 12. April 2018 12:54
>>>>>> To: Baesken, Matthias <matthias.baesken at sap.com>; Magnus Ihse 
>>>>>> Bursie <magnus.ihse.bursie at oracle.com>
>>>>>> Cc: build-dev at openjdk.java.net; Doerr, Martin 
>>>>>> <martin.doerr at sap.com>; core-libs-dev at openjdk.java.net
>>>>>> Subject: Re: 8201226 missing JNIEXPORT / JNICALL at some places in
>>>>>> function
>>>>>> declarations/implementations - was : RE: missing JNIEXPORT / 
>>>>>> JNICALL at
>>>>>> some places in function declarations/implementations
>>>>>>
>>>>>> Adding core-libs-dev as this is change code in libjava, libzip,
>>>>>> libjimage, ...
>>>>>>
>>>>>> Can you confirm that this is the up to date webrev:
>>>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/
>>>>>>
>>>>>> As I read it, this changes the calling convention of these 
>>>>>> functions on
>>>>>> 32-bit Windows but it will have no impact on 64-bit Windows (as
>>>>>> __stdcall is ignored) or other platforms, is that correct?
>>>>>>
>>>>>> -Alan
>>>>>>
>>>>>>
>>>>>> <SNIP>
>