Proposal: Use new JDK_EXPORT decorator instead of JNIEXPORT

[Magnus Ihse Bursie]

On 2018-12-12 09:40, David Holmes wrote:
> On 12/12/2018 5:44 pm, Volker Simonis wrote:
>> On Tue, Dec 11, 2018 at 11:47 PM David Holmes 
>> <david.holmes at oracle.com> wrote:
>>>
>>> On 12/12/2018 12:34 am, Magnus Ihse Bursie wrote:
>>>>
>>>>
>>>> On 2018-12-11 00:23, David Holmes wrote:
>>>>> Hi Magnus,
>>>>>
>>>>> On 10/12/2018 11:19 pm, Magnus Ihse Bursie wrote:
>>>>>> I propose that we introduce a new define, available to all JDK 
>>>>>> native
>>>>>> files (Hotspot included), called JDK_EXPORT. The behavior of this
>>>>>> symbol will be very similar (as of now, in fact identical) to
>>>>>> JNIEXPORT; however, the semantics will not.
>>>>>>
>>>>>> Currently, we "mis-use" the JNIEXPORT define to mark a function for
>>>>>> exporting from the library. The problem with this is that JNIEXPORT
>>>>>> is part of the JNI interface, and is supposed to be used when C
>>>>>> programs interact with Java. And, when doing this, the function
>>>>>> should be fully decorated like this: "JNIEXPORT foo JNICALL".
>>>>>
>>>>> I've seen a lot of the emails on this issue and I don't fully
>>>>> understand what has been going wrong. But the intent is obviously the
>>>>> JNIEXPORT represents what is needed to export this function for 
>>>>> use by
>>>>> JNI, while JNICALL defines the calling convention. I agree there may
>>>>> be some mistmatch when functions are actually not intended for 
>>>>> general
>>>>> export outside the JDK but are only for internal JDK use.
>>>>>
>>>>>> We do have many such JNI exports in our native libraries, but we 
>>>>>> also
>>>>>> have a lot of other, non-JNI exports, where one native library just
>>>>>> provides an interface to other libraries. In these cases, we have
>>>>>> still used JNIEXPORT for the functionality of getting the function
>>>>>> exported, but we have not been consistent in our use of JNICALL. 
>>>>>> This
>>>>>> has caused us way too much trouble for something that should Just
>>>>>> Work<tm>.
>>>>>
>>>>> Are you suggesting that the interface between different libraries in
>>>>> the JDK should not be a JNI interface? Is this because you think the
>>>>> functions in these libraries are only for JDK internal use or ... ??
>>>>>
>>>>>> I therefore propose that we define "JDK_EXPORT", with the same
>>>>>> behavior as JNIEXPORT (that is, flagging the function for external
>>>>>> visibility in the resulting native library), but which is *not*
>>>>>> supposed to be exported to Java code using JNI, nor supposed to be
>>>>>> decorated with
>>>>>
>>>>> Just a clarification there. JNI functions are not exported to Java
>>>>> code, they are exported to native code. Java code can declare native
>>>>> methods and those native methods must be written as JNI functions, 
>>>>> but
>>>>> that's not what we are discussing. Libraries expose a JNI 
>>>>> interface (a
>>>>> set of functions in the library) that can be called by application
>>>>> native code, using JNI.
>>>> We're apparently looking at "JNI" and "exporting" from two opposite
>>>> sides here. :-) Just to make everything clear: If I have a Java class
>>>> class MyClass {
>>>>     public static void native myNativeFunc();
>>>> }
>>>>
>>>> then I have one half of the JNI function, the Java half. This must be
>>>> matched by a corresponding implementation in native, like this:
>>>> JNIEXPORT void JNICALL
>>>> Java_MyClass_myNativeFunc(void) {
>>>> // ... do stuff
>>>> }
>>>>
>>>> And this is the native half of the JNI function. Right? Let's leave
>>>> aside which side is "exporting" to the other for now. :-)
>>>>
>>>> This way of setting up native functions that can be called from 
>>>> Java is
>>>> what I refer to as JNI. And when you declare a native JNI function, 
>>>> you
>>>> *must* use both JNIEXPORT and JNICALL. Alright?
>>>>
>>>> We do have a lot of those functions in our native libraries. And they
>>>> are correct just the way they are.
>>>
>>> Yep all well and good. A function declared native in Java must have an
>>> implementation as you describe. But not all native functions exist to
>>> provide the native-half of a Java native function!
>>>
>>>> However, we also have *other* native functions, that are exported, not
>>>> as JNI functions that should be called from Java, but as normal native
>>>> library functions that should be called by other native code. Okay so
>>>> far? And *those* functions have been problematic in how we decorate
>>>
>>> But there are again two cases. Those functions exported from a library
>>> that are expected to be called from external code using the JNI
>>> interface mechanism - such as all the JNI functions and JVM TI 
>>> functions
>>> we export from the JVM - and those "exported" for access between
>>> libraries within the JDK (such as all the JVM_* functions in libjvm).
>>>
>>> I think it is only the second group that should be addressed by your
>>> JDK_EXPORT proposal - though I'm not completely clear exactly how to
>>> identify them.
>>>
>>>> them. My proposal is that we *refrain* from using JNIEXPORT for those
>>>> functions, and instead use JDK_EXPORT as name for the macro that
>>>> decorates them as exported. That way, we can clearly see that a 
>>>> function
>>>> like this:
>>>>
>>>> JDK_EXPORT void
>>>> JLI_ReadEnv(char* env);
>>>>
>>>> is correctly declared, and will be exported to other native libraries,
>>>> but not to Java.
>>>
>>> The issue is not whether it is "exported to Java"** but whether it is
>>> exported using the JNI mechanism such that other native code calls it
>>> using the JNI mechanism.
>>>
>>> ** There is no way to write a native method declaration in Java such
>>> that it would be linked to the JLI_ReadEnv function. The naming is all
>>> wrong, as is the signature.
>>>
>>
>> But that's exactly what this change is about! Remove the usage of
>> JNIEXPORT from functions which are NOT exported using the JNI
>> mechanism. What don't you like about it ?
>
> I'm just saying we need to be clear about what functions we plan on 
> changing. Taking concrete examples as I don't see JLI_Read anywhere, 
> we have:
>
> JNIEXPORT void JNICALL
> JLI_ReportErrorMessage(const char* fmt, ...) {
>
> JNIEXPORT void JNICALL
> JLI_ReportErrorMessageSys(const char* fmt, ...) {
>
> JNIEXPORT void JNICALL
> JLI_ReportExceptionDescription(JNIEnv * env) {
>
> JNIEXPORT StdArg JNICALL
> *JLI_GetStdArgs()
> {
>
> JNIEXPORT int JNICALL
> JLI_GetStdArgc() {
>     return 0;
> }
>
> which seems to define the exported interface for (part of) libJLI and 
> establishes both the export status and the expected calling 
> convention. Would these be changed?
Yes! Those are exactly the kind of functions that should change.

At this point, I intended to go out and make a full list of all 
functions that I wanted to change, but a grep for JNIEXPORT resulted in 
over 3800 hits, so I'm not doing it now. (However, if my proposal is 
eventually accepted, I will need to go through these 3800 calls and 
check them up. Fortunately, with some scripting, the search can be 
drastically limited.)

These functions you list *cannot* be called from Java. There is no 
corresponding Java native methods. All suchs methods need to follow the 
JNI calling convention, which does not *only* put requirements on the 
decoration, but also on the name 
("Java_fully_qualified_class_name_and_method") and on the argument list 
(first argument must be a JNIEnv*). For the purpose of this discussion, 
I'm calling functions that fulfil these requirements *JNI functions*.

The functions you listed above is *not* JNI function. OK? Other examples 
of non-JNI functions are e.g:

JNIEXPORT void JNICALL SplashSetScaleFactor(float); (from splashscreen)

or

JNIEXPORT jboolean JNICALL doDrawPath(DrawHandler* hnd, ...)

Contrast this with:

JNIEXPORT void JNICALL Java_sun_java2d_loops_DrawPath_DrawPath(JNIEnv 
*env, jobject self, ...)

The latter function is called, not from some other native code, but 
directly from java, since there is a 
sun.java2d.loops.DrawPath.DrawPath() method (it's apparently a native 
constructor, a bit odd, but hey... 2d...).

The former function is called from *other native libraries in the JDK*, 
from native code, using #import "java2d/loops/ProcessPath.h" and then 
calling doDrawPath().

I think there's an obvious difference between these two, and I'm getting 
increasingly frustrated that I'm unable to communicate this difference 
to you. :-(

Are you with me now, with this classification?

My proposal is that we should *only* use JNIEXPORT and JNICALL in the 
latter case, the JNI case. The former case, the non-JNI, "native lib to 
native lib" function, should use JDK_EXPORT instead of JNIEXPORT, so we 
let JNIEXPORT correctly signify that the function marked is a JNI-call.

*Also*, we should remove JNICALL as well. Since these functions are just 
ways for our native libraries to communicate with another, we can 
determine to change the calling convention at will, just as if we've 
added a new argument to a function signature. Also, for practical 
purposes, JNICALL is empty for all platforms except Windows 32.

And, just to be extremely clear, I do *not* propose we change official 
APIs. Documented, public function calls will not be affected. This 
includes e.g.

JNIEXPORT jint JNICALL JNI_CreateJavaVM(JavaVM **pvm, void **penv, void 
*args);

which will continue to look like that, even though it can not be called 
from Java. It also applies to all other functions in jni.h, and other 
documented header files.


>
>>>> Just to clarify, this has nothing to do with if this is a officially
>>>> supported API or not. In general though, I assume that most (if not
>>>> all?) of our exported functions (apart from the JNI_* stuff) is 
>>>> supposed
>>>> to be consumed by other libraries in the JDK, and is not a public API.
>>>
>>> I think it varies library by library. You may need native application
>>> code that can call directly into native JDK libraries. JLI is the Java
>>> Launcher Interface - I think it was introduced to make it easier for
>>> other launchers to be created. Native agents may need access to
>>> libmanagement or libjdwp functions. Native graphics code may need 
>>> access
>>> to the JDK graphics library. Some of these accesses may be unsupported
>>> and undocumented, but I don't think you can just cut them all off.
>>>
>>
>> Nobody wants to cut off anything. Magnus only proposes to decorate
>> these required functions with the new JDK_EXPORT macro (instead of
>> JNIEXPORT) in order to make it clear that they are not exported by
>> using the JNI mechanism (but they will still be exported, technically
>> speaking, JDK_EXPORT will even resolve to the exact same function
>> modifiers!).
>
> Maybe I've misunderstood the proposal. I thought some functions 
> presently JNIEXPORT and JNICALL would be changed to just JDK_EXPORT as 
> they are deemed not to be "JNI exported functions" as they are not the 
> native-half of any Java native method. 
If I understand you correctly, this is what I'm proposing.

> If the proposal is only to use JDK_EXPORT where JNICALL is missing 
> (and JDK_EXPORT is identical to JNIEXPORT) that seems fine as it just 
> serves as "documentation". But if JDKEXPORT differs from JNI_EXPORT 
> then you may change the ability of external code to link to libraries 
> it presently does link to.
JDK_EXPORT and JNIEXPORT will be identical. The difference is, we can in 
the future change JDK_EXPORT at will, without having to do a CCC or risk 
breaking tons of user code. For instance, in AIX, we build native 
libraries with all symbols for xlc version 12 or below, but for version 
13 doing proper symbol visibility becomes doable. In that case, the AIX 
team might find that it would be a good idea to declare JNIEXPORT as a 
way to set exported visibility on functions, to help user's developing 
native code, but we might still keep JDK_EXPORT as a no-op (and continue 
exporting all symbols) for our internal need. (Not saying any of this is 
a good idea, but it's an example of the freedom it gives us.)

I think you are trying to use  "JNICALL is missing" as a way to measure 
either:
1) the function is a JNI-function, an implementation of a Java native 
method,
or
2) the function is officially documented

Unfortunately, neither of these are true. And the presence, or absence, 
of JNICALL is at this time mostly arbitrary, for non-JNI ("lib to lib") 
functions. Which is one thing that has caused us a lot of trouble!
>
> And the change to jdk.h versus jni.h is still unclear to me given 
> jni.h defines the types that allow interaction with Java and which may 
> still be needed in a JDK_EXPORT function implementation.
>
> I think there is overloaded use of the term "JNI" function in all this
Yes, that is one part of the confusion I want to address. There is no 
need to bring in "JNI" in functions that has nothing to do with JNI.

/Magnus

>
> David
> -----
>
>>> David
>>>
>>>>
>>>> /Magnus
>>>>
>>>>
>>>>
>>>>>
>>>>>> JNICALL. All current instances of JNIEXPORT which is not pure JNI
>>>>>> native functions should be changed to use JDK_EXPORT instead.
>>>>>>
>>>>>> I further propose that this macro should reside in a new file
>>>>>> "jdk.h", placed in the new directory
>>>>>> src/java.base/share/native/include/internal. This header file path
>>>>>> will automatically be provided to all native libraries, but not
>>>>>> copied to the JDK being built. (The existence of a 
>>>>>> "include/internal"
>>>>>> directory with this behavior has been discussed before. There are
>>>>>> more files that ought to be moved there, if/when it is created.) I
>>>>>> believe in many cases the #include "jni.h" can be just modified to
>>>>>> #include "#jdk.h", since most native code will not require "jni.h"
>>>>>> unless actually doing JNI calls -- most have included this file to
>>>>>> get the JNIEXPORT macro, which would explain the pervasive use of
>>>>>> #include "jni.h" in our code base.
>>>>>
>>>>> jni.h also defines all of the types used by the JNI. Those types are
>>>>> pervsive to the native code used throughout the JDK.
>>>>>
>>>>>> Thoughts?
>>>>>
>>>>> I think we need to understand the problems on Windows that prompted
>>>>> all this. Then I think we need to look at exactly how jni.h and
>>>>> JNIEXPORT etc are being used and understand whether this is truly an
>>>>> exported interface or not.
>>>>>
>>>>> Cheers,
>>>>> David
>>>>>
>>>>>> /Magnus
>>>>>>
>>>>