[monodroid] Update to new profiler init symbol name. #689
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jonpryor
reviewed
Jul 11, 2017
src/monodroid/jni/dylib-mono.h
Outdated
| monodroid_mono_profiler_install_thread_fptr mono_profiler_install_thread; | ||
| monodroid_mono_profiler_set_events_fptr mono_profiler_set_events; | ||
| monodroid_mono_profiler_install_fptr mono_profiler_install; | ||
| monodroid_mono_profiler_set_jit_done_callback_fptr mono_profiler_set_jit_done_callback; |
There was a problem hiding this comment.
This is potentially problematic:
/* NOTE: structure members MUST NOT CHANGE ORDER. */
The reason for that comment is because a long time ago @mhutch wanted to reuse dylib-mono.h for use with Unreal. Changing any members would constitute an ABI breakage; it was expected that any callers could check the DylibMono::version structure to see if it was >= the size they expected, and bail if it was "too small."
This only works if we only append structure members, i.e. the "version number" is constantly increasing, hence the comment.
This PR changes member ordering, which could break any external users of monodroid_dylib_mono_new()...if there were any.
Which there probably aren't.
I'm thus conflict: if there were or will be external users of monodroid_dylib_mono_new(), then this particular change in this PR isn't acceptable. Which could be very problematic if the "obsolete" symbols are ever removed from mono.
Pinging @mhutch to confirm that we don't care about ABI here. If we do need to care, we'll likely need to reconsider the public interface to better deal with "optional" DylibMono sections.
There was a problem hiding this comment.
The old symbols are not just obsolete, they're completely removed from Mono.
There was a problem hiding this comment.
I expect to resurrect MonoUE's Android support at some point but it currently has no users, so breakage doesn't matter.
jonpryor
reviewed
Jul 11, 2017
src/monodroid/jni/dylib-mono.h
Outdated
| typedef void (*monodroid_mono_profiler_install_jit_end_fptr) (MonoProfileJitResult end); | ||
| typedef void (*monodroid_mono_profiler_install_thread_fptr) (void *start_ftn, void *end_ftn); | ||
| typedef void (*monodroid_mono_profiler_set_events_fptr) (MonoProfileFlags events); | ||
| typedef MonoProfilerHandle (*monodroid_mono_profiler_install_fptr (MonoProfiler *prof); |
There was a problem hiding this comment.
This line is missing a ); it should be:
typedef MonoProfilerHandle (*monodroid_mono_profiler_install_fptr) (MonoProfiler *prof);
alexrp
changed the title
|
I've changed this PR to only contain the init symbol change. mono/mono#5248 will ensure that the old functions used by XA will continue to work. |
jonpryor
added a commit
to jonpryor/xamarin-android
that referenced
this pull request
Aug 25, 2020
Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1139578 Context: dotnet/java-interop#691 Context: https://liquid.microsoft.com/Web/Object/Read/ms.security/Requirements/Microsoft.Security.SystemsADM.10039#guide Changes: dotnet/java-interop@007b35b...08ff4db * dotnet/java-interop@08ff4db1: [java-interop, Java.Interop] Securely load native libs (dotnet#691) * dotnet/java-interop@09c68911: [generator] Clean up generated whitespace (dotnet#692) * dotnet/java-interop@d664e90e: [generator] Make warning and error messages localizable. (dotnet#689) For improved security, the guidance is that when loading `.dll` files on Windows, the default [`LoadLibrary()`][0] behavior is to be *avoided*, as the [Dynamic-Link Library Search Order][1] includes the "current working directory" and directories listed in `%PATH%`, both of which are under control of an "attacker" and not the application. Instead, [`LoadLibraryEx()`][2] should be used, providing a set of `LOAD_LIBRARY_SEARCH_*` values which constrain the set of directories that will be searched for the library (if a full path is not used) and/or the library's dependencies. Historically, Xamarin.Android hasn't directly called any `LoadLibrary()` function. Instead, xamarin-android called [**dlopen**(3)][3], then used dlfcn-win32/dlfcn-win32@ef7e412d to implement `dlopen()` in terms of `LoadLibraryEx()`. Unfortunately, dlfcn-win32/dlfcn-win32@ef7e412d calls `LoadLibraryEx()` with `LOAD_WITH_ALTERED_SEARCH_PATH`, which while a security improvement, does not go far enough for our internal requirements. Migrate away from dlfcn-win32 and instead use the new `java-interop-dlfcn.h` family of functions added in dotnet/java-interop@08ff4db1. These new functions appropriately use `LoadLibraryEx()` with a constrained set of `LOAD_LIBRARY_SEARCH_*` flags. *Non-obvious semantic note*: On Mono for all platforms, P/Invoking `__Internal` is equivalent to trying to load the path `NULL`, e.g. on macOS `dlopen(NULL, …)`. However, only Android allows a custom "`loader_func`" to intercept the attempted load of `NULL` and "do something" with it, which xamarin-android *does* intercept, "remapping" `NULL` to `libxa-internal-api.so`. On all other platforms, `loader_func` cannot intercept this invocation. …which made for an "interesting" interaction: on macOS, if we used: api_dso_handle = java_interop_lib_load (dso_path.get (), JAVA_INTEROP_LIB_LOAD_LOCALLY, nullptr); then the macOS Designer integration tests would fail! Renderer >> 4 [monodroid] Calling into managed runtime init Renderer (error) >> Renderer (error) >> Unhandled Exception: Renderer (error) >> System.EntryPointNotFoundException: java_interop_jnienv_get_java_vm assembly:<unknown assembly> type:<unknown type> member:(null) Renderer (error) >> at (wrapper managed-to-native) Java.Interop.NativeMethods.java_interop_jnienv_get_java_vm(intptr,intptr&) Renderer (error) >> at Java.Interop.JniEnvironment+References.GetJavaVM (System.IntPtr jnienv, System.IntPtr& vm) [0x00000] in <0f003a4904fd44d0a8cc6a63962ab40b>:0 Renderer (error) >> at Java.Interop.JniEnvironmentInfo.set_EnvironmentPointer (System.IntPtr value) [0x00037] in <0f003a4904fd44d0a8cc6a63962ab40b>:0 Renderer (error) >> at Java.Interop.JniEnvironmentInfo..ctor (System.IntPtr environmentPointer, Java.Interop.JniRuntime runtime) [0x00006] in <0f003a4904fd44d0a8cc6a63962ab40b>:0 Renderer (error) >> at Java.Interop.JniRuntime..ctor (Java.Interop.JniRuntime+CreationOptions options) [0x0017b] in <0f003a4904fd44d0a8cc6a63962ab40b>:0 Renderer (error) >> at Android.Runtime.AndroidRuntime..ctor (System.IntPtr jnienv, System.IntPtr vm, System.Boolean allocNewObjectSupported, System.IntPtr classLoader, System.IntPtr classLoader_loadClass, System.Boolean jniAddNativeMethodRegistrationAttributePresent) [0x00000] in /Users/builder/azdo/_work/4/s/xamarin-android/src/Mono.Android/Android.Runtime/AndroidRuntime.cs:25 In order for the macOS Designer integration tests to succeed, `libxa-internal-api.dylib` *must* be loaded as `RTLD_GLOBAL`, which is done via `JAVA_INTEROP_LIB_LOAD_GLOBALLY`. [0]: https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibrarya [1]: https://docs.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-search-order [2]: https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibraryexa [3]: https://linux.die.net/man/3/dlopen
jonpryor
added a commit
to jonpryor/xamarin-android
that referenced
this pull request
Aug 26, 2020
Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1139578 Context: dotnet/java-interop#691 Context: https://liquid.microsoft.com/Web/Object/Read/ms.security/Requirements/Microsoft.Security.SystemsADM.10039#guide Changes: dotnet/java-interop@007b35b...08ff4db * dotnet/java-interop@08ff4db1: [java-interop, Java.Interop] Securely load native libs (dotnet#691) * dotnet/java-interop@09c68911: [generator] Clean up generated whitespace (dotnet#692) * dotnet/java-interop@d664e90e: [generator] Make warning and error messages localizable. (dotnet#689) For improved security, the guidance is that when loading `.dll` files on Windows, the default [`LoadLibrary()`][0] behavior is to be *avoided*, as the [Dynamic-Link Library Search Order][1] includes the "current working directory" and directories listed in `%PATH%`, both of which are under control of an "attacker" and not the application. Instead, [`LoadLibraryEx()`][2] should be used, providing a set of `LOAD_LIBRARY_SEARCH_*` values which constrain the set of directories that will be searched for the library (if a full path is not used) and/or the library's dependencies. Historically, Xamarin.Android hasn't directly called any `LoadLibrary()` function. Instead, xamarin-android called [**dlopen**(3)][3], then used dlfcn-win32/dlfcn-win32@ef7e412d to implement `dlopen()` in terms of `LoadLibraryEx()`. Unfortunately, dlfcn-win32/dlfcn-win32@ef7e412d calls `LoadLibraryEx()` with `LOAD_WITH_ALTERED_SEARCH_PATH`, which while a security improvement, does not go far enough for our internal requirements. Migrate away from dlfcn-win32 and instead use the new `java-interop-dlfcn.h` family of functions added in dotnet/java-interop@08ff4db1. These new functions appropriately use `LoadLibraryEx()` with a constrained set of `LOAD_LIBRARY_SEARCH_*` flags. *Non-obvious semantic note*: On Mono for all platforms, P/Invoking `__Internal` is equivalent to trying to load the path `NULL`, e.g. on macOS `dlopen(NULL, …)`. However, only Android allows a custom "`loader_func`" to intercept the attempted load of `NULL` and "do something" with it, which xamarin-android *does* intercept, "remapping" `NULL` to `libxa-internal-api.so`. On all other platforms, `loader_func` cannot intercept this invocation. …which made for an "interesting" interaction: on macOS, if we used: api_dso_handle = java_interop_lib_load (dso_path.get (), JAVA_INTEROP_LIB_LOAD_LOCALLY, nullptr); then the macOS Designer integration tests would fail! Renderer >> 4 [monodroid] Calling into managed runtime init Renderer (error) >> Renderer (error) >> Unhandled Exception: Renderer (error) >> System.EntryPointNotFoundException: java_interop_jnienv_get_java_vm assembly:<unknown assembly> type:<unknown type> member:(null) Renderer (error) >> at (wrapper managed-to-native) Java.Interop.NativeMethods.java_interop_jnienv_get_java_vm(intptr,intptr&) Renderer (error) >> at Java.Interop.JniEnvironment+References.GetJavaVM (System.IntPtr jnienv, System.IntPtr& vm) [0x00000] in <0f003a4904fd44d0a8cc6a63962ab40b>:0 Renderer (error) >> at Java.Interop.JniEnvironmentInfo.set_EnvironmentPointer (System.IntPtr value) [0x00037] in <0f003a4904fd44d0a8cc6a63962ab40b>:0 Renderer (error) >> at Java.Interop.JniEnvironmentInfo..ctor (System.IntPtr environmentPointer, Java.Interop.JniRuntime runtime) [0x00006] in <0f003a4904fd44d0a8cc6a63962ab40b>:0 Renderer (error) >> at Java.Interop.JniRuntime..ctor (Java.Interop.JniRuntime+CreationOptions options) [0x0017b] in <0f003a4904fd44d0a8cc6a63962ab40b>:0 Renderer (error) >> at Android.Runtime.AndroidRuntime..ctor (System.IntPtr jnienv, System.IntPtr vm, System.Boolean allocNewObjectSupported, System.IntPtr classLoader, System.IntPtr classLoader_loadClass, System.Boolean jniAddNativeMethodRegistrationAttributePresent) [0x00000] in /Users/builder/azdo/_work/4/s/xamarin-android/src/Mono.Android/Android.Runtime/AndroidRuntime.cs:25 In order for the macOS Designer integration tests to succeed, `libxa-internal-api.dylib` *must* be loaded as `RTLD_GLOBAL`, which is done via `JAVA_INTEROP_LIB_LOAD_GLOBALLY`. [0]: https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibrarya [1]: https://docs.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-search-order [2]: https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibraryexa [3]: https://linux.die.net/man/3/dlopen
jonpryor
added a commit
to jonpryor/xamarin-android
that referenced
this pull request
Aug 26, 2020
Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1139578 Context: dotnet/java-interop#691 Context: https://liquid.microsoft.com/Web/Object/Read/ms.security/Requirements/Microsoft.Security.SystemsADM.10039#guide Changes: dotnet/java-interop@007b35b...dcaf794 * dotnet/java-interop@dcaf794d: [generator] Fix error and warning codes values (dotnet#697) * dotnet/java-interop@08ff4db1: [java-interop, Java.Interop] Securely load native libs (dotnet#691) * dotnet/java-interop@09c68911: [generator] Clean up generated whitespace (dotnet#692) * dotnet/java-interop@d664e90e: [generator] Make warning and error messages localizable. (dotnet#689) For improved security, the guidance is that when loading `.dll` files on Windows, the default [`LoadLibrary()`][0] behavior is to be *avoided*, as the [Dynamic-Link Library Search Order][1] includes the "current working directory" and directories listed in `%PATH%`, both of which are under control of an "attacker" and not the application. Instead, [`LoadLibraryEx()`][2] should be used, providing a set of `LOAD_LIBRARY_SEARCH_*` values which constrain the set of directories that will be searched for the library (if a full path is not used) and/or the library's dependencies. Historically, Xamarin.Android hasn't directly called any `LoadLibrary()` function. Instead, xamarin-android called [**dlopen**(3)][3], then used dlfcn-win32/dlfcn-win32@ef7e412d to implement `dlopen()` in terms of `LoadLibraryEx()`. Unfortunately, dlfcn-win32/dlfcn-win32@ef7e412d calls `LoadLibraryEx()` with `LOAD_WITH_ALTERED_SEARCH_PATH`, which while a security improvement, does not go far enough for our internal requirements. Migrate away from dlfcn-win32 and instead use the new `java-interop-dlfcn.h` family of functions added in dotnet/java-interop@08ff4db1. These new functions appropriately use `LoadLibraryEx()` with a constrained set of `LOAD_LIBRARY_SEARCH_*` flags. *Non-obvious semantic note*: On Mono for all platforms, P/Invoking `__Internal` is equivalent to trying to load the path `NULL`, e.g. on macOS `dlopen(NULL, …)`. However, only Android allows a custom "`loader_func`" to intercept the attempted load of `NULL` and "do something" with it, which xamarin-android *does* intercept, "remapping" `NULL` to `libxa-internal-api.so`. On all other platforms, `loader_func` cannot intercept this invocation. …which made for an "interesting" interaction: on macOS, if we used: api_dso_handle = java_interop_lib_load (dso_path.get (), JAVA_INTEROP_LIB_LOAD_LOCALLY, nullptr); then the macOS Designer integration tests would fail! Renderer >> 4 [monodroid] Calling into managed runtime init Renderer (error) >> Renderer (error) >> Unhandled Exception: Renderer (error) >> System.EntryPointNotFoundException: java_interop_jnienv_get_java_vm assembly:<unknown assembly> type:<unknown type> member:(null) Renderer (error) >> at (wrapper managed-to-native) Java.Interop.NativeMethods.java_interop_jnienv_get_java_vm(intptr,intptr&) Renderer (error) >> at Java.Interop.JniEnvironment+References.GetJavaVM (System.IntPtr jnienv, System.IntPtr& vm) [0x00000] in <0f003a4904fd44d0a8cc6a63962ab40b>:0 Renderer (error) >> at Java.Interop.JniEnvironmentInfo.set_EnvironmentPointer (System.IntPtr value) [0x00037] in <0f003a4904fd44d0a8cc6a63962ab40b>:0 Renderer (error) >> at Java.Interop.JniEnvironmentInfo..ctor (System.IntPtr environmentPointer, Java.Interop.JniRuntime runtime) [0x00006] in <0f003a4904fd44d0a8cc6a63962ab40b>:0 Renderer (error) >> at Java.Interop.JniRuntime..ctor (Java.Interop.JniRuntime+CreationOptions options) [0x0017b] in <0f003a4904fd44d0a8cc6a63962ab40b>:0 Renderer (error) >> at Android.Runtime.AndroidRuntime..ctor (System.IntPtr jnienv, System.IntPtr vm, System.Boolean allocNewObjectSupported, System.IntPtr classLoader, System.IntPtr classLoader_loadClass, System.Boolean jniAddNativeMethodRegistrationAttributePresent) [0x00000] in /Users/builder/azdo/_work/4/s/xamarin-android/src/Mono.Android/Android.Runtime/AndroidRuntime.cs:25 In order for the macOS Designer integration tests to succeed, `libxa-internal-api.dylib` *must* be loaded as `RTLD_GLOBAL`, which is done via `JAVA_INTEROP_LIB_LOAD_GLOBALLY`. [0]: https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibrarya [1]: https://docs.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-search-order [2]: https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibraryexa [3]: https://linux.die.net/man/3/dlopen
jonpryor
added a commit
that referenced
this pull request
Aug 27, 2020
Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1139578 Context: dotnet/java-interop#691 Context: https://liquid.microsoft.com/Web/Object/Read/ms.security/Requirements/Microsoft.Security.SystemsADM.10039#guide Changes: dotnet/java-interop@007b35b...dcaf794 * dotnet/java-interop@dcaf794d: [generator] Fix error and warning codes values (#697) * dotnet/java-interop@08ff4db1: [java-interop, Java.Interop] Securely load native libs (#691) * dotnet/java-interop@09c68911: [generator] Clean up generated whitespace (#692) * dotnet/java-interop@d664e90e: [generator] Make warning and error messages localizable. (#689) For improved security, the guidance is that when loading `.dll` files on Windows, the default [`LoadLibrary()`][0] behavior is to be *avoided*, as the [Dynamic-Link Library Search Order][1] includes the "current working directory" and directories listed in `%PATH%`, both of which are under control of an "attacker" and not the application. Instead, [`LoadLibraryEx()`][2] should be used, providing a set of `LOAD_LIBRARY_SEARCH_*` values which constrain the set of directories that will be searched for the library (if a full path is not used) and/or the library's dependencies. Historically, Xamarin.Android hasn't directly called any `LoadLibrary()` function. Instead, xamarin-android called [**dlopen**(3)][3], then used dlfcn-win32/dlfcn-win32@ef7e412d to implement `dlopen()` in terms of `LoadLibraryEx()`. Unfortunately, dlfcn-win32/dlfcn-win32@ef7e412d calls `LoadLibraryEx()` with `LOAD_WITH_ALTERED_SEARCH_PATH`, which while a security improvement, does not go far enough for our internal requirements. Migrate away from dlfcn-win32 and instead use the new `java-interop-dlfcn.h` family of functions added in dotnet/java-interop@08ff4db1. These new functions appropriately use `LoadLibraryEx()` with a constrained set of `LOAD_LIBRARY_SEARCH_*` flags. *Non-obvious semantic note*: On Mono for all platforms, P/Invoking `__Internal` is equivalent to trying to load the path `NULL`, e.g. on macOS `dlopen(NULL, …)`. However, only Android allows a custom "`loader_func`" to intercept the attempted load of `NULL` and "do something" with it, which xamarin-android *does* intercept, "remapping" `NULL` to `libxa-internal-api.so`. On all other platforms, `loader_func` cannot intercept this invocation. …which made for an "interesting" interaction: on macOS, if we used: api_dso_handle = java_interop_lib_load (dso_path.get (), JAVA_INTEROP_LIB_LOAD_LOCALLY, nullptr); then the macOS Designer integration tests would fail! Renderer >> 4 [monodroid] Calling into managed runtime init Renderer (error) >> Renderer (error) >> Unhandled Exception: Renderer (error) >> System.EntryPointNotFoundException: java_interop_jnienv_get_java_vm assembly:<unknown assembly> type:<unknown type> member:(null) Renderer (error) >> at (wrapper managed-to-native) Java.Interop.NativeMethods.java_interop_jnienv_get_java_vm(intptr,intptr&) Renderer (error) >> at Java.Interop.JniEnvironment+References.GetJavaVM (System.IntPtr jnienv, System.IntPtr& vm) [0x00000] in <0f003a4904fd44d0a8cc6a63962ab40b>:0 Renderer (error) >> at Java.Interop.JniEnvironmentInfo.set_EnvironmentPointer (System.IntPtr value) [0x00037] in <0f003a4904fd44d0a8cc6a63962ab40b>:0 Renderer (error) >> at Java.Interop.JniEnvironmentInfo..ctor (System.IntPtr environmentPointer, Java.Interop.JniRuntime runtime) [0x00006] in <0f003a4904fd44d0a8cc6a63962ab40b>:0 Renderer (error) >> at Java.Interop.JniRuntime..ctor (Java.Interop.JniRuntime+CreationOptions options) [0x0017b] in <0f003a4904fd44d0a8cc6a63962ab40b>:0 Renderer (error) >> at Android.Runtime.AndroidRuntime..ctor (System.IntPtr jnienv, System.IntPtr vm, System.Boolean allocNewObjectSupported, System.IntPtr classLoader, System.IntPtr classLoader_loadClass, System.Boolean jniAddNativeMethodRegistrationAttributePresent) [0x00000] in /Users/builder/azdo/_work/4/s/xamarin-android/src/Mono.Android/Android.Runtime/AndroidRuntime.cs:25 In order for the macOS Designer integration tests to succeed, `libxa-internal-api.dylib` *must* be loaded as `RTLD_GLOBAL`, which is done via `JAVA_INTEROP_LIB_LOAD_GLOBALLY`. [0]: https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibrarya [1]: https://docs.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-search-order [2]: https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibraryexa [3]: https://linux.die.net/man/3/dlopen
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Do not merge yet. This is intended for the next monthly integration branch, not master.