java.interop icon indicating copy to clipboard operation
java.interop copied to clipboard

Wrong calling convention used on Windows

Open radekdoulik opened this issue 6 years ago • 2 comments
trafficstars

We should make sure we use the right calling convention on different platforms.

Recently I have run into that when trying to use Java.Runtime.Environment.dll on Windows. Turned out we don't specify right calling convention when interfacing with JVM and with own native functions as well (java_interop_gc_bridge_*).

That leads to crashes without usable stacktrace information.

radekdoulik avatar Dec 12 '18 17:12 radekdoulik

We should make sure we use the right calling convention on different platforms.

Should we?

We need to make sure that we use the right calling convention: the C# P/Invoke declaration must match what's in the C library.

The thing is, this is already done (mostly): the default calling convention on Windows differs between C# and C. The default P/Invoke calling convention is WinAPI -- stdcall on Windows, cdecl elsewhere -- while the default calling convention for C is cdecl everywhere. This is why our C# declarations explicitly specify cdecl:

https://github.com/xamarin/java.interop/blob/b57d7703ee1af11bc16db689c6282e50d76ef121/src/Java.Runtime.Environment/Java.Interop/MonoRuntimeObjectReferenceManager.cs#L207

Not specifying CallingConvention=CallingConvention.Cdecl instead requires the use of JNICALL within our C code, meaning this:

JI_API jint
java_interop_jnienv_get_version (JNIEnv *env);

would need to become:

JI_API jint JI_CALLCONV
java_interop_jnienv_get_version (JNIEnv *env);

where JI_CALLCONV is __stdcall on Windows.

Such a change would become a significantly larger change, and I don't see any actual need for it.

All we need is for the C# and C sides to be consistent with each other. They already are.

jonpryor avatar Dec 13 '18 15:12 jonpryor

I don't think we need to add the CallingConvention=CallingConvention.Cdecl as the default works OK.

These for example work without specifying Cdecl explicitly:

                [DllImport (JavaInteropLib, CharSet=CharSet.Ansi)]
                internal static extern int java_interop_jvm_load (string path);

                [DllImport (JavaInteropLib, CharSet=CharSet.Ansi)]
                internal static extern int java_interop_jvm_create (out IntPtr javavm, out IntPtr jnienv, ref JavaVMInitArgs args);

The Cdecl removal is not important part of that issue though. The wrong calling convention used to call JNI API is.

radekdoulik avatar Dec 13 '18 16:12 radekdoulik