From 286683224ea9c38be3028dd03bf47b73f4da45f4 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Thu, 12 Aug 2021 19:29:16 +0000 Subject: [PATCH] [monodroid] Use more standard C++ features, performance improvements (#6159) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C++ features: * Use C++20 concepts to improve compile-time correctness verification * Use `std::vector` for dynamic heap buffers * Use `std::array` for static stack buffers * Use `std::unique_ptr` instead of the home-grown equivalent (also for C-allocated memory) * Use C++20 structure designators Optimizations: * Add string routines (`ends_with()` and `find_last()`) optimized for the stack string types (derived from `string_base`) * Count assemblies at build time and pre-allocate enough memory (in the form of a `std::vector` instance) to store information about the bundled assemblies, thus avoiding 2 allocations per assembly (one `realloc()` and one `malloc()`) making the bundled assemblies loading algorithm closer to O(1) instead of the previous O(n*2). * Use character arrays instead of "plain" string literals to optimize some string operations. These changes improve plain XA test app startup time by around 11% Additionally, rename `string_segment::append(const char*)` to `string_segment::append_c(const char*)`. We [found that][0] the C++ compiler preferred the non-`template` `append(const char*)` overload over the `template append(const char (&s)[S])` overload, meaning `append("foo")` would hit the slower `strlen()`-based method. Likewise create `assign_c(const char*)` and `starts_with_c(const char*)` member functions. Finally, [upon review][1] of the exported symbols, we found that many symbols that we didn't want to export were being exported: % $HOME/android-toolchain/ndk/toolchains/llvm/prebuilt/darwin-x86_64/bin/x86_64-linux-android-nm -D -C bin/Debug/lib/xamarin.android/xbuild/Xamarin/Android/lib/x86_64/libmono-android.debug.so … 000000000003e480 T std::terminate() 0000000000014f80 T operator delete[](void*) 0000000000014f90 T operator delete[](void*, std::nothrow_t const&) 0000000000014fa0 T operator delete[](void*, unsigned long) 0000000000014f50 T operator delete(void*) 0000000000014f60 T operator delete(void*, std::nothrow_t const&) 0000000000014f70 T operator delete(void*, unsigned long) 0000000000014ef0 T operator new[](unsigned long) 0000000000014f30 T operator new[](unsigned long, std::nothrow_t const&) 0000000000014e90 T operator new(unsigned long) 0000000000014ed0 T operator new(unsigned long, std::nothrow_t const&) Update our declarations of these functions so that they are instead *weak* symbols, or otherwise no longer exported: 0000000000011e10 W operator delete[](void*) 0000000000011dfc W operator delete(void*) 0000000000011dec W operator new[](unsigned long) 0000000000011da8 W operator new(unsigned long) [0]: https://github.com/xamarin/xamarin-android/pull/6159#discussion_r686185106 [1]: https://github.com/xamarin/xamarin-android/pull/6159#discussion_r685533429 --- .../Tasks/GeneratePackageManagerJava.cs | 8 + .../Utilities/EnvironmentHelper.cs | 10 +- ...pplicationConfigNativeAssemblyGenerator.cs | 4 + .../Xamarin.Android.Common.targets | 1 + src/monodroid/CMakeLists.txt | 1 + src/monodroid/jni/android-system.cc | 24 +- src/monodroid/jni/application_dso_stub.cc | 27 +- src/monodroid/jni/basic-android-system.cc | 4 +- src/monodroid/jni/basic-utilities.cc | 6 +- src/monodroid/jni/basic-utilities.hh | 35 ++- src/monodroid/jni/cpp-util.hh | 104 +------ src/monodroid/jni/cxx-abi/terminate.cc | 1 - src/monodroid/jni/cxx-abi/vector.cc | 12 + src/monodroid/jni/debug.cc | 18 +- src/monodroid/jni/embedded-assemblies-zip.cc | 255 ++++++++++-------- src/monodroid/jni/embedded-assemblies.cc | 161 +++++------ src/monodroid/jni/embedded-assemblies.hh | 80 +++++- src/monodroid/jni/monodroid-glue-internal.hh | 2 + src/monodroid/jni/monodroid-glue.cc | 90 +++---- src/monodroid/jni/new_delete.cc | 15 +- src/monodroid/jni/pinvoke-override-api.cc | 25 -- src/monodroid/jni/strings.hh | 46 +++- src/monodroid/jni/xamarin-app.hh | 1 + 23 files changed, 485 insertions(+), 445 deletions(-) create mode 100644 src/monodroid/jni/cxx-abi/vector.cc diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs index 9e89f9ef496..23edd420ae7 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs @@ -28,6 +28,8 @@ public class GeneratePackageManagerJava : AndroidTask [Required] public ITaskItem[] ResolvedUserAssemblies { get; set; } + public ITaskItem[] SatelliteAssemblies { get; set; } + [Required] public string OutputDirectory { get; set; } @@ -264,6 +266,11 @@ void AddEnvironment () throw new InvalidOperationException ($"Unsupported BoundExceptionType value '{BoundExceptionType}'"); } + int assemblyCount = ResolvedAssemblies.Length; + if (SatelliteAssemblies != null) { + assemblyCount += SatelliteAssemblies.Length; + } + bool haveRuntimeConfigBlob = !String.IsNullOrEmpty (RuntimeConfigBinFilePath) && File.Exists (RuntimeConfigBinFilePath); var appConfState = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal (ApplicationConfigTaskState.RegisterTaskObjectKey, RegisteredTaskObjectLifetime.Build); foreach (string abi in SupportedAbis) { @@ -284,6 +291,7 @@ void AddEnvironment () InstantRunEnabled = InstantRunEnabled, JniAddNativeMethodRegistrationAttributePresent = appConfState != null ? appConfState.JniAddNativeMethodRegistrationAttributePresent : false, HaveRuntimeConfigBlob = haveRuntimeConfigBlob, + NumberOfAssembliesInApk = assemblyCount, }; using (var sw = MemoryStreamPool.Shared.CreateStreamWriter ()) { diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs index 1e00bd36d28..0dd769eacc7 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs @@ -30,9 +30,10 @@ public sealed class ApplicationConfig public uint package_naming_policy; public uint environment_variable_count; public uint system_property_count; + public uint number_of_assemblies_in_apk; public string android_package_name; }; - const uint ApplicationConfigFieldCount = 13; + const uint ApplicationConfigFieldCount = 14; static readonly object ndkInitLock = new object (); static readonly char[] readElfFieldSeparator = new [] { ' ', '\t' }; @@ -176,7 +177,12 @@ static ApplicationConfig ReadApplicationConfig (string envFile) ret.system_property_count = ConvertFieldToUInt32 ("system_property_count", envFile, i, field [1]); break; - case 12: // android_package_name: string / [pointer type] + case 12: // number_of_assemblies_in_apk: uint32_t / .word | .long + Assert.IsTrue (expectedUInt32Types.Contains (field [0]), $"Unexpected uint32_t field type in '{envFile}:{i}': {field [0]}"); + ret.number_of_assemblies_in_apk = ConvertFieldToUInt32 ("number_of_assemblies_in_apk", envFile, i, field [1]); + break; + + case 13: // android_package_name: string / [pointer type] Assert.IsTrue (expectedPointerTypes.Contains (field [0]), $"Unexpected pointer field type in '{envFile}:{i}': {field [0]}"); pointers.Add (field [1].Trim ()); break; diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs index 0b7f9009180..e1013bdb397 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs @@ -23,6 +23,7 @@ class ApplicationConfigNativeAssemblyGenerator : NativeAssemblyGenerator public bool InstantRunEnabled { get; set; } public bool JniAddNativeMethodRegistrationAttributePresent { get; set; } public bool HaveRuntimeConfigBlob { get; set; } + public int NumberOfAssembliesInApk { get; set; } public PackageNamingPolicy PackageNamingPolicy { get; set; } @@ -86,6 +87,9 @@ protected override void WriteSymbols (StreamWriter output) WriteCommentLine (output, "system_property_count"); size += WriteData (output, systemProperties == null ? 0 : systemProperties.Count * 2); + WriteCommentLine (output, "number_of_assemblies_in_apk"); + size += WriteData (output, NumberOfAssembliesInApk); + WriteCommentLine (output, "android_package_name"); size += WritePointer (output, MakeLocalLabel (stringLabel)); diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets index c9cde7bd350..1b1c1f2cfb9 100644 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets @@ -1574,6 +1574,7 @@ because xbuild doesn't support framework reference assemblies. override_file (utils.path_combine (override_dirs [oi], name)); + std::unique_ptr override_file {utils.path_combine (override_dirs [oi], name)}; log_info (LOG_DEFAULT, "Trying to get property from %s", override_file.get ()); - size_t result = _monodroid_get_system_property_from_file (override_file, value); + size_t result = _monodroid_get_system_property_from_file (override_file.get (), value); if (result == 0 || value == nullptr || (*value) == nullptr || **value == '\0') { continue; } @@ -351,9 +351,9 @@ AndroidSystem::get_full_dso_path (const char *base_dir, const char *dso_path, dy if (base_dir == nullptr || utils.is_path_rooted (dso_path)) return const_cast(dso_path); // Absolute path or no base path, can't do much with it - path.assign (base_dir) + path.assign_c (base_dir) .append (MONODROID_PATH_SEPARATOR) - .append (dso_path); + .append_c (dso_path); return true; } @@ -567,7 +567,7 @@ AndroidSystem::setup_environment_from_override_file (const char *path) auto file_size = static_cast(sbuf.st_size); size_t nread = 0; ssize_t r; - simple_pointer_guard buf (new char [file_size]); + auto buf = std::make_unique (file_size); do { auto read_count = static_cast(file_size - nread); @@ -603,14 +603,14 @@ AndroidSystem::setup_environment_from_override_file (const char *path) } char *endptr; - unsigned long name_width = strtoul (buf, &endptr, 16); - if ((name_width == std::numeric_limits::max () && errno == ERANGE) || (*buf != '\0' && *endptr != '\0')) { + unsigned long name_width = strtoul (buf.get (), &endptr, 16); + if ((name_width == std::numeric_limits::max () && errno == ERANGE) || (buf[0] != '\0' && *endptr != '\0')) { log_warn (LOG_DEFAULT, "Malformed header of the environment override file %s: name width has invalid format", path); return; } unsigned long value_width = strtoul (buf.get () + 11, &endptr, 16); - if ((value_width == std::numeric_limits::max () && errno == ERANGE) || (*buf != '\0' && *endptr != '\0')) { + if ((value_width == std::numeric_limits::max () && errno == ERANGE) || (buf[0] != '\0' && *endptr != '\0')) { log_warn (LOG_DEFAULT, "Malformed header of the environment override file %s: value width has invalid format", path); return; } @@ -625,7 +625,7 @@ AndroidSystem::setup_environment_from_override_file (const char *path) char *name = buf.get () + OVERRIDE_ENVIRONMENT_FILE_HEADER_SIZE; while (data_size > 0 && data_size >= data_width) { if (*name == '\0') { - log_warn (LOG_DEFAULT, "Malformed environment override file %s: name at offset %lu is empty", path, name - buf); + log_warn (LOG_DEFAULT, "Malformed environment override file %s: name at offset %lu is empty", path, name - buf.get ()); return; } @@ -707,9 +707,9 @@ AndroidSystem::setup_environment () #if defined (DEBUG) || !defined (ANDROID) // TODO: for debug read from file in the override directory named `environment` for (size_t oi = 0; oi < MAX_OVERRIDES; oi++) { - simple_pointer_guard env_override_file (utils.path_combine (override_dirs [oi], OVERRIDE_ENVIRONMENT_FILE_NAME)); - if (utils.file_exists (env_override_file)) { - setup_environment_from_override_file (env_override_file); + std::unique_ptr env_override_file {utils.path_combine (override_dirs [oi], OVERRIDE_ENVIRONMENT_FILE_NAME)}; + if (utils.file_exists (env_override_file.get ())) { + setup_environment_from_override_file (env_override_file.get ()); } } #endif diff --git a/src/monodroid/jni/application_dso_stub.cc b/src/monodroid/jni/application_dso_stub.cc index 0173b9cdf6b..8b4a1cddba1 100644 --- a/src/monodroid/jni/application_dso_stub.cc +++ b/src/monodroid/jni/application_dso_stub.cc @@ -35,19 +35,20 @@ CompressedAssemblies compressed_assemblies = { }; ApplicationConfig application_config = { - /*.uses_mono_llvm =*/ false, - /*.uses_mono_aot =*/ false, - /*.uses_assembly_preload =*/ false, - /*.is_a_bundled_app =*/ false, - /*.broken_exception_transitions =*/ false, - /*.instant_run_enabled =*/ false, - /*.jni_add_native_method_registration_attribute_present =*/ false, - /*.have_runtime_config_blob =*/ false, - /*.bound_exception_type =*/ 0, // System - /*.package_naming_policy =*/ 0, - /*.environment_variable_count =*/ 0, - /*.system_property_count =*/ 0, - /*.android_package_name =*/ "com.xamarin.test", + .uses_mono_llvm = false, + .uses_mono_aot = false, + .uses_assembly_preload = false, + .is_a_bundled_app = false, + .broken_exception_transitions = false, + .instant_run_enabled = false, + .jni_add_native_method_registration_attribute_present = false, + .have_runtime_config_blob = false, + .bound_exception_type = 0, // System + .package_naming_policy = 0, + .environment_variable_count = 0, + .system_property_count = 0, + .number_of_assemblies_in_apk = 0, + .android_package_name = "com.xamarin.test", }; const char* mono_aot_mode_name = ""; diff --git a/src/monodroid/jni/basic-android-system.cc b/src/monodroid/jni/basic-android-system.cc index 5140ab21df0..12c5f7e5191 100644 --- a/src/monodroid/jni/basic-android-system.cc +++ b/src/monodroid/jni/basic-android-system.cc @@ -16,9 +16,9 @@ void BasicAndroidSystem::detect_embedded_dso_mode (jstring_array_wrapper& appDirs) noexcept { // appDirs[2] points to the native library directory - simple_pointer_guard libmonodroid_path = utils.path_combine (appDirs[2].get_cstr (), "libmonodroid.so"); + std::unique_ptr libmonodroid_path {utils.path_combine (appDirs[2].get_cstr (), "libmonodroid.so")}; log_debug (LOG_ASSEMBLY, "Checking if libmonodroid was unpacked to %s", libmonodroid_path.get ()); - if (!utils.file_exists (libmonodroid_path)) { + if (!utils.file_exists (libmonodroid_path.get ())) { log_debug (LOG_ASSEMBLY, "%s not found, assuming application/android:extractNativeLibs == false", libmonodroid_path.get ()); set_embedded_dso_mode_enabled (true); } else { diff --git a/src/monodroid/jni/basic-utilities.cc b/src/monodroid/jni/basic-utilities.cc index 1523b470bfb..9b063d77cef 100644 --- a/src/monodroid/jni/basic-utilities.cc +++ b/src/monodroid/jni/basic-utilities.cc @@ -65,14 +65,14 @@ BasicUtilities::create_directory (const char *pathname, mode_t mode) mode_t oldumask; #endif oldumask = umask (022); - simple_pointer_guard path (strdup_new (pathname)); + std::unique_ptr path {strdup_new (pathname)}; int rv, ret = 0; - for (char *d = path; d != nullptr && *d; ++d) { + for (char *d = path.get (); d != nullptr && *d; ++d) { if (*d != '/') continue; *d = 0; if (*path) { - rv = make_directory (path, mode); + rv = make_directory (path.get (), mode); if (rv == -1 && errno != EEXIST) { ret = -1; break; diff --git a/src/monodroid/jni/basic-utilities.hh b/src/monodroid/jni/basic-utilities.hh index ff0a0ae07b1..7a4856aac61 100644 --- a/src/monodroid/jni/basic-utilities.hh +++ b/src/monodroid/jni/basic-utilities.hh @@ -58,17 +58,17 @@ namespace xamarin::android abort_unless (path1 != nullptr || path2 != nullptr, "At least one path must be a valid pointer"); if (path1 == nullptr) { - buf.append (path2); + buf.append_c (path2); return; } if (path2 == nullptr) { - buf.append (path1); + buf.append_c (path1); return; } buf.append (path1, path1_len); - buf.append (MONODROID_PATH_SEPARATOR, MONODROID_PATH_SEPARATOR_LENGTH); + buf.append (MONODROID_PATH_SEPARATOR); buf.append (path2, path2_len); } @@ -109,6 +109,35 @@ namespace xamarin::android return p != nullptr && p [N - 1] == '\0'; } + template + bool ends_with (internal::string_base const& str, const char (&end)[N]) const noexcept + { + constexpr size_t end_length = N - 1; + + size_t len = str.length (); + if (XA_UNLIKELY (len < end_length)) { + return false; + } + + return memcmp (str.get () + len - end_length, end, end_length) == 0; + } + + template + const TChar* find_last (internal::string_base const& str, const char ch) const noexcept + { + if (str.empty ()) { + return nullptr; + } + + for (size_t i = str.length () - 1; i >= 0; i--) { + if (str[i] == ch) { + return str.get () + i; + } + } + + return nullptr; + } + void *xmalloc (size_t size) { return ::xmalloc (size); diff --git a/src/monodroid/jni/cpp-util.hh b/src/monodroid/jni/cpp-util.hh index 84c9584f167..64b085d915f 100644 --- a/src/monodroid/jni/cpp-util.hh +++ b/src/monodroid/jni/cpp-util.hh @@ -3,6 +3,7 @@ #include #include +#include #if defined (ANDROID) #include @@ -41,108 +42,15 @@ do_abort_unless (bool condition, const char* fmt, ...) namespace xamarin::android { template - class simple_pointer_guard_type + struct CDeleter final { - public: - simple_pointer_guard_type () = default; - simple_pointer_guard_type (T* _tp) - : tp (_tp) - {} - simple_pointer_guard_type (simple_pointer_guard_type &other) = delete; - simple_pointer_guard_type (const simple_pointer_guard_type &other) = delete; - - T& operator= (simple_pointer_guard_type &other) = delete; - T& operator= (const simple_pointer_guard_type &other) = delete; - T* operator= (T* ptr) = delete; - const T* operator= (const T* ptr) = delete; - - T* get () const noexcept - { - return tp; - } - - T* operator->() const noexcept - { - return tp; - } - - T& operator*() const noexcept - { - return *tp; - } - - explicit operator bool () const noexcept - { - return tp != nullptr; - } - - operator T* () const noexcept - { - return tp; - } - - operator T const* () const noexcept + void operator() (T* p) { - return tp; + std::free (p); } - - private: - T *tp = nullptr; - }; - - template - class simple_pointer_guard : public simple_pointer_guard_type - { - public: - simple_pointer_guard () = default; - simple_pointer_guard (T* _tp) - : simple_pointer_guard_type (_tp) - {} - simple_pointer_guard (simple_pointer_guard &other) = delete; - simple_pointer_guard (const simple_pointer_guard &other) = delete; - - ~simple_pointer_guard () - { - T *ptr = simple_pointer_guard_type::get (); - if constexpr (uses_cpp_alloc) { - delete ptr; - } else { - free (ptr); - } - } - - T& operator= (simple_pointer_guard &other) = delete; - T& operator= (const simple_pointer_guard &other) = delete; - T* operator= (T* ptr) = delete; - const T* operator= (const T* ptr) = delete; }; - template - class simple_pointer_guard : public simple_pointer_guard_type - { - public: - simple_pointer_guard () = default; - simple_pointer_guard (T* _tp) - : simple_pointer_guard_type (_tp) - {} - simple_pointer_guard (simple_pointer_guard &other) = delete; - simple_pointer_guard (const simple_pointer_guard &other) = delete; - - ~simple_pointer_guard () - { - T *ptr = simple_pointer_guard_type::get (); - - if constexpr (uses_cpp_alloc) { - delete[] ptr; - } else { - free (ptr); - } - } - - T& operator= (simple_pointer_guard &other) = delete; - T& operator= (const simple_pointer_guard &other) = delete; - const T* operator= (const T (ptr)[]) = delete; - T* operator= (T (ptr)[]) = delete; - }; + template + using c_unique_ptr = std::unique_ptr>; } #endif // !def __CPP_UTIL_HH diff --git a/src/monodroid/jni/cxx-abi/terminate.cc b/src/monodroid/jni/cxx-abi/terminate.cc index 624274a30d8..3379cbbcaef 100644 --- a/src/monodroid/jni/cxx-abi/terminate.cc +++ b/src/monodroid/jni/cxx-abi/terminate.cc @@ -4,7 +4,6 @@ // Does NOT support terminate handlers, since we don't use them. // #include -#include #include namespace std { diff --git a/src/monodroid/jni/cxx-abi/vector.cc b/src/monodroid/jni/cxx-abi/vector.cc new file mode 100644 index 00000000000..277224ae0a5 --- /dev/null +++ b/src/monodroid/jni/cxx-abi/vector.cc @@ -0,0 +1,12 @@ +// +// Defining the macro will make the the explicit instantations below truely hidden +// +#define _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS + +#include + +_LIBCPP_BEGIN_NAMESPACE_STD + +template class __attribute__ ((__visibility__("hidden"))) __vector_base_common; + +_LIBCPP_END_NAMESPACE_STD diff --git a/src/monodroid/jni/debug.cc b/src/monodroid/jni/debug.cc index 01a3fce7d19..43b727a1d6a 100644 --- a/src/monodroid/jni/debug.cc +++ b/src/monodroid/jni/debug.cc @@ -82,18 +82,18 @@ Debug::monodroid_profiler_load (const char *libmono_path, const char *desc, cons } else { mname_ptr = utils.strdup_new (desc); } - simple_pointer_guard mname (mname_ptr); + std::unique_ptr mname {mname_ptr}; unsigned int dlopen_flags = JAVA_INTEROP_LIB_LOAD_LOCALLY; - simple_pointer_guard libname (utils.string_concat ("libmono-profiler-", mname.get (), ".so")); + std::unique_ptr libname {utils.string_concat ("libmono-profiler-", mname.get (), ".so")}; bool found = false; - void *handle = androidSystem.load_dso_from_any_directories (libname, dlopen_flags); - found = load_profiler_from_handle (handle, desc, mname); + void *handle = androidSystem.load_dso_from_any_directories (libname.get (), dlopen_flags); + found = load_profiler_from_handle (handle, desc, mname.get ()); if (!found && libmono_path != nullptr) { - simple_pointer_guard full_path (utils.path_combine (libmono_path, libname)); - handle = androidSystem.load_dso (full_path, dlopen_flags, FALSE); - found = load_profiler_from_handle (handle, desc, mname); + std::unique_ptr full_path {utils.path_combine (libmono_path, libname.get ())}; + handle = androidSystem.load_dso (full_path.get (), dlopen_flags, FALSE); + found = load_profiler_from_handle (handle, desc, mname.get ()); } if (found && logfile != nullptr) @@ -129,8 +129,8 @@ Debug::load_profiler_from_handle (void *dso_handle, const char *desc, const char if (!dso_handle) return false; - simple_pointer_guard symbol (utils.string_concat (INITIALIZER_NAME, "_", name)); - bool result = load_profiler (dso_handle, desc, symbol); + std::unique_ptr symbol {utils.string_concat (INITIALIZER_NAME, "_", name)}; + bool result = load_profiler (dso_handle, desc, symbol.get ()); if (result) return true; diff --git a/src/monodroid/jni/embedded-assemblies-zip.cc b/src/monodroid/jni/embedded-assemblies-zip.cc index 4e655032ea5..982d5b8201f 100644 --- a/src/monodroid/jni/embedded-assemblies-zip.cc +++ b/src/monodroid/jni/embedded-assemblies-zip.cc @@ -1,5 +1,7 @@ +#include #include #include +#include #include #include @@ -10,6 +12,17 @@ using namespace xamarin::android::internal; +// This type is needed when calling read(2) in a MinGW build, as it defines the `count` parameter as `unsigned int` +// instead of `size_t` which then causes the following warning if we pass a value of type `size_t`: +// +// warning: conversion from ‘size_t’ {aka ‘long long unsigned int’} to ‘unsigned int’ may change value [-Wconversion] +// +#if defined (WINDOWS) +using read_count_type = unsigned int; +#else +using read_count_type = size_t; +#endif + void EmbeddedAssemblies::zip_load_entries (int fd, const char *apk_name, monodroid_should_register should_register) { @@ -32,19 +45,16 @@ EmbeddedAssemblies::zip_load_entries (int fd, const char *apk_name, monodroid_sh exit (FATAL_EXIT_NO_ASSEMBLIES); } - // C++17 allows template parameter type inference, but alas, Apple's antiquated compiler does - // not support this particular part of the spec... - simple_pointer_guard buf (new uint8_t[cd_size]); + std::vector buf (cd_size); const char *prefix = get_assemblies_prefix (); - size_t prefix_len = strlen (prefix); + size_t prefix_len = get_assemblies_prefix_length (); size_t buf_offset = 0; uint16_t compression_method; uint32_t local_header_offset; uint32_t data_offset; uint32_t file_size; - char *file_name; - ssize_t nread = read (fd, buf.get (), cd_size); + ssize_t nread = read (fd, buf.data (), static_cast(buf.size ())); if (static_cast(nread) != cd_size) { log_fatal (LOG_ASSEMBLY, "Failed to read Central Directory from the APK archive %s. %s (nread: %d; errno: %d)", apk_name, std::strerror (errno), nread, errno); exit (FATAL_EXIT_NO_ASSEMBLIES); @@ -55,6 +65,19 @@ EmbeddedAssemblies::zip_load_entries (int fd, const char *apk_name, monodroid_sh bool runtime_config_blob_found = false; #endif // def NET6 + bool bundled_assemblies_slow_path = false; + if (bundled_assemblies.empty ()) { + // We're called for the first time, let's allocate space for all the assemblies counted during build. resize() + // will allocate all the memory and zero it in one operation + bundled_assemblies.resize (application_config.number_of_assemblies_in_apk); + } else { + // We are probably registering some assemblies dynamically, possibly loading from another apk, we need to extend + // the storage instead of taking advantage of the pre-allocated slots. + bundled_assemblies_slow_path = true; + } + + int64_t assembly_count = application_config.number_of_assemblies_in_apk; + // clang-tidy claims we have a leak in the loop: // // Potential leak of memory pointed to by 'assembly_name' @@ -66,13 +89,12 @@ EmbeddedAssemblies::zip_load_entries (int fd, const char *apk_name, monodroid_sh for (size_t i = 0; i < cd_entries; i++) { entry_name.clear (); - bool result = zip_read_entry_info (buf.get (), cd_size, buf_offset, compression_method, local_header_offset, file_size, entry_name); - file_name = entry_name.get (); + bool result = zip_read_entry_info (buf, buf_offset, compression_method, local_header_offset, file_size, entry_name); #ifdef DEBUG - log_warn (LOG_ASSEMBLY, "%s entry: %s", apk_name, file_name == nullptr ? "unknown" : file_name); + log_warn (LOG_ASSEMBLY, "%s entry: %s", apk_name, entry_name.get () == nullptr ? "unknown" : entry_name.get ()); #endif - if (!result || file_name == nullptr) { + if (!result || entry_name.empty ()) { log_fatal (LOG_ASSEMBLY, "Failed to read Central Directory info for entry %u in APK file %s", i, apk_name); exit (FATAL_EXIT_NO_ASSEMBLIES); } @@ -87,14 +109,14 @@ EmbeddedAssemblies::zip_load_entries (int fd, const char *apk_name, monodroid_sh if (compression_method != 0) continue; - if (strncmp (prefix, file_name, prefix_len) != 0) + if (strncmp (prefix, entry_name.get (), prefix_len) != 0) continue; #if defined (NET6) if (application_config.have_runtime_config_blob && !runtime_config_blob_found) { - if (utils.ends_with (file_name, SharedConstants::RUNTIME_CONFIG_BLOB_NAME)) { + if (utils.ends_with (entry_name, SharedConstants::RUNTIME_CONFIG_BLOB_NAME)) { runtime_config_blob_found = true; - runtime_config_blob_mmap = md_mmap_apk_file (fd, data_offset, file_size, file_name, apk_name); + runtime_config_blob_mmap = md_mmap_apk_file (fd, data_offset, file_size, entry_name.get (), apk_name); continue; } } @@ -102,64 +124,73 @@ EmbeddedAssemblies::zip_load_entries (int fd, const char *apk_name, monodroid_sh // assemblies must be 4-byte aligned, or Bad Things happen if ((data_offset & 0x3) != 0) { - log_fatal (LOG_ASSEMBLY, "Assembly '%s' is located at bad offset %lu within the .apk\n", file_name, data_offset); + log_fatal (LOG_ASSEMBLY, "Assembly '%s' is located at bad offset %lu within the .apk\n", entry_name.get (), data_offset); log_fatal (LOG_ASSEMBLY, "You MUST run `zipalign` on %s\n", strrchr (apk_name, '/') + 1); exit (FATAL_EXIT_MISSING_ZIPALIGN); } - bool entry_is_overridden = !should_register (strrchr (file_name, '/') + 1); + const char *last_slash = utils.find_last (entry_name, '/'); + bool entry_is_overridden = last_slash == nullptr ? false : !should_register (last_slash + 1); - if ((utils.ends_with (file_name, ".pdb") || utils.ends_with (file_name, ".mdb")) && + if ((utils.ends_with (entry_name, ".pdb") || utils.ends_with (entry_name, ".mdb")) && register_debug_symbols && !entry_is_overridden && - bundled_assemblies != nullptr) { - md_mmap_info map_info = md_mmap_apk_file(fd, data_offset, file_size, file_name, apk_name); - if (register_debug_symbols_for_assembly (file_name, (bundled_assemblies) [bundled_assemblies_count - 1], (const mono_byte*)map_info.area, static_cast(file_size))) + bundled_assembly_index >= 1) { + md_mmap_info map_info = md_mmap_apk_file(fd, data_offset, file_size, entry_name.get (), apk_name); + if (register_debug_symbols_for_assembly (entry_name, bundled_assemblies [bundled_assembly_index - 1], (const mono_byte*)map_info.area, static_cast(file_size))) continue; } #if !defined(NET6) - if (utils.ends_with (file_name, ".config") && bundled_assemblies != nullptr) { - char *assembly_name = strdup (basename (file_name)); + if (utils.ends_with (entry_name, ".config") && !bundled_assemblies.empty ()) { + char *assembly_name = strdup (basename (entry_name.get ())); // Remove '.config' suffix *strrchr (assembly_name, '.') = '\0'; - md_mmap_info map_info = md_mmap_apk_file (fd, data_offset, file_size, file_name, apk_name); + md_mmap_info map_info = md_mmap_apk_file (fd, data_offset, file_size, entry_name.get (), apk_name); mono_register_config_for_assembly (assembly_name, (const char*)map_info.area); continue; } #endif // ndef NET6 - if (!utils.ends_with (file_name, ".dll")) + if (!utils.ends_with (entry_name, ".dll")) continue; if (entry_is_overridden) continue; - size_t alloc_size = MULTIPLY_WITH_OVERFLOW_CHECK (size_t, sizeof(void*), bundled_assemblies_count + 1); - bundled_assemblies = reinterpret_cast (utils.xrealloc (bundled_assemblies, alloc_size)); - MonoBundledAssembly *cur = bundled_assemblies [bundled_assemblies_count] = reinterpret_cast (utils.xcalloc (1, sizeof (MonoBundledAssembly))); - ++bundled_assemblies_count; + assembly_count--; + MonoBundledAssembly *cur; + if (XA_UNLIKELY (assembly_count < 0 || (bundled_assemblies_slow_path && bundled_assemblies.size () <= bundled_assembly_index))) { + if (assembly_count == -1) { + log_warn (LOG_ASSEMBLY, "Number of assemblies stored at build time (%u) was incorrect, switching to slow bundling path."); + } + bundled_assemblies.emplace_back (); + cur = &bundled_assemblies.back (); + } else { + cur = &bundled_assemblies [bundled_assembly_index]; + } + bundled_assembly_index++; - md_mmap_info map_info = md_mmap_apk_file (fd, data_offset, file_size, file_name, apk_name); - cur->name = utils.monodroid_strdup_printf ("%s", strstr (file_name, prefix) + prefix_len); + md_mmap_info map_info = md_mmap_apk_file (fd, data_offset, file_size, entry_name.get (), apk_name); + cur->name = utils.strdup_new (entry_name.get () + prefix_len, entry_name.length () - prefix_len); cur->data = (const unsigned char*)map_info.area; // MonoBundledAssembly::size is const?! unsigned int *psize = (unsigned int*) &cur->size; *psize = static_cast(file_size); - if (utils.should_log (LOG_ASSEMBLY)) { + if (XA_UNLIKELY (utils.should_log (LOG_ASSEMBLY))) { const char *p = (const char*) cur->data; - char header[9]; - for (size_t j = 0; j < sizeof(header)-1; ++j) + std::array header; + for (size_t j = 0; j < header.size () - 1; ++j) header[j] = isprint (p [j]) ? p [j] : '.'; - header [sizeof(header)-1] = '\0'; + header [header.size () - 1] = '\0'; log_info_nocheck (LOG_ASSEMBLY, "file-offset: % 8x start: %08p end: %08p len: % 12i zip-entry: %s name: %s [%s]", - (int) data_offset, cur->data, cur->data + *psize, (int) file_size, file_name, cur->name, header); + (int) data_offset, cur->data, cur->data + *psize, (int) file_size, entry_name.get (), cur->name, header.data ()); } } } @@ -167,11 +198,6 @@ EmbeddedAssemblies::zip_load_entries (int fd, const char *apk_name, monodroid_sh bool EmbeddedAssemblies::zip_read_cd_info (int fd, uint32_t& cd_offset, uint32_t& cd_size, uint16_t& cd_entries) { -#if defined (WINDOWS) - using read_count_type = unsigned int; -#else - using read_count_type = size_t; -#endif // The simplest case - no file comment off_t ret = ::lseek (fd, -ZIP_EOCD_LEN, SEEK_END); if (ret < 0) { @@ -179,23 +205,23 @@ EmbeddedAssemblies::zip_read_cd_info (int fd, uint32_t& cd_offset, uint32_t& cd_ return false; } - uint8_t eocd[ZIP_EOCD_LEN]; - ssize_t nread = ::read (fd, eocd, static_cast(ZIP_EOCD_LEN)); + std::array eocd; + ssize_t nread = ::read (fd, eocd.data (), static_cast(eocd.size ())); if (nread < 0 || nread != ZIP_EOCD_LEN) { log_error (LOG_ASSEMBLY, "Failed to read EOCD from the APK: %s (nread: %d; errno: %d)", std::strerror (errno), nread, errno); return false; } size_t index = 0; // signature - uint8_t signature[4]; + std::array signature; - if (!zip_read_field (eocd, ZIP_EOCD_LEN, index, signature)) { + if (!zip_read_field (eocd, index, signature)) { log_error (LOG_ASSEMBLY, "Failed to read EOCD signature"); return false; } - if (memcmp (signature, ZIP_EOCD_MAGIC, sizeof(signature)) == 0) { - return zip_extract_cd_info (eocd, ZIP_EOCD_LEN, cd_offset, cd_size, cd_entries); + if (memcmp (signature.data (), ZIP_EOCD_MAGIC, signature.size ()) == 0) { + return zip_extract_cd_info (eocd, cd_offset, cd_size, cd_entries); } // Most probably a ZIP with comment @@ -206,12 +232,9 @@ EmbeddedAssemblies::zip_read_cd_info (int fd, uint32_t& cd_offset, uint32_t& cd_ return false; } - simple_pointer_guard buf (new uint8_t[alloc_size]); - // The cast removes warning on mingw: - // - // warning: conversion from ‘size_t’ {aka ‘long long unsigned int’} to ‘unsigned int’ may change value [-Wconversion] - // - nread = ::read (fd, buf, static_cast(alloc_size)); + std::vector buf (alloc_size); + + nread = ::read (fd, buf.data (), static_cast(buf.size ())); if (nread < 0 || static_cast(nread) != alloc_size) { log_error (LOG_ASSEMBLY, "Failed to read EOCD and comment from the APK: %s (nread: %d; errno: %d)", std::strerror (errno), nread, errno); @@ -220,12 +243,13 @@ EmbeddedAssemblies::zip_read_cd_info (int fd, uint32_t& cd_offset, uint32_t& cd_ // We scan from the end to save time bool found = false; + const uint8_t* data = buf.data (); for (ssize_t i = static_cast(alloc_size - (ZIP_EOCD_LEN + 2)); i >= 0; i--) { - if (memcmp (buf.get () + i, ZIP_EOCD_MAGIC, sizeof(ZIP_EOCD_MAGIC)) != 0) + if (memcmp (data + i, ZIP_EOCD_MAGIC, sizeof(ZIP_EOCD_MAGIC)) != 0) continue; found = true; - memcpy (eocd, buf.get () + i, ZIP_EOCD_LEN); + memcpy (eocd.data (), data + i, ZIP_EOCD_LEN); break; } @@ -234,7 +258,7 @@ EmbeddedAssemblies::zip_read_cd_info (int fd, uint32_t& cd_offset, uint32_t& cd_ return false; } - return zip_extract_cd_info (eocd, ZIP_EOCD_LEN, cd_offset, cd_size, cd_entries); + return zip_extract_cd_info (eocd, cd_offset, cd_size, cd_entries); } bool @@ -249,36 +273,36 @@ EmbeddedAssemblies::zip_adjust_data_offset (int fd, size_t local_header_offset, return false; } - uint8_t local_header[ZIP_LOCAL_LEN]; - uint8_t signature[4]; + std::array local_header; + std::array signature; - ssize_t nread = ::read (fd, local_header, static_cast(ZIP_LOCAL_LEN)); + ssize_t nread = ::read (fd, local_header.data (), static_cast(ZIP_LOCAL_LEN)); if (nread < 0 || nread != ZIP_LOCAL_LEN) { log_error (LOG_ASSEMBLY, "Failed to read local header at offset %u: %s (nread: %d; errno: %d)", local_header_offset, std::strerror (errno), nread, errno); return false; } size_t index = 0; - if (!zip_read_field (local_header, ZIP_LOCAL_LEN, index, signature)) { + if (!zip_read_field (local_header, index, signature)) { log_error (LOG_ASSEMBLY, "Failed to read Local Header entry signature at offset %u", local_header_offset); return false; } - if (memcmp (signature, ZIP_LOCAL_MAGIC, sizeof(signature)) != 0) { + if (memcmp (signature.data (), ZIP_LOCAL_MAGIC, sizeof(signature)) != 0) { log_error (LOG_ASSEMBLY, "Invalid Local Header entry signature at offset %u", local_header_offset); return false; } uint16_t file_name_length; index = LH_FILE_NAME_LENGTH_OFFSET; - if (!zip_read_field (local_header, ZIP_LOCAL_LEN, index, file_name_length)) { + if (!zip_read_field (local_header, index, file_name_length)) { log_error (LOG_ASSEMBLY, "Failed to read Local Header 'file name length' field at offset %u", (local_header_offset + index)); return false; } uint16_t extra_field_length; index = LH_EXTRA_LENGTH_OFFSET; - if (!zip_read_field (local_header, ZIP_LOCAL_LEN, index, extra_field_length)) { + if (!zip_read_field (local_header, index, extra_field_length)) { log_error (LOG_ASSEMBLY, "Failed to read Local Header 'extra field length' field at offset %u", (local_header_offset + index)); return false; } @@ -288,29 +312,27 @@ EmbeddedAssemblies::zip_adjust_data_offset (int fd, size_t local_header_offset, return true; } +template bool -EmbeddedAssemblies::zip_extract_cd_info (uint8_t* buf, size_t buf_len, uint32_t& cd_offset, uint32_t& cd_size, uint16_t& cd_entries) +EmbeddedAssemblies::zip_extract_cd_info (std::array const& buf, uint32_t& cd_offset, uint32_t& cd_size, uint16_t& cd_entries) { static constexpr size_t EOCD_TOTAL_ENTRIES_OFFSET = 10; static constexpr size_t EOCD_CD_SIZE_OFFSET = 12; static constexpr size_t EOCD_CD_START_OFFSET = 16; - if (buf_len < ZIP_EOCD_LEN) { - log_fatal (LOG_ASSEMBLY, "Buffer too short for EOCD"); - exit (FATAL_EXIT_OUT_OF_MEMORY); - } + static_assert (BufSize >= ZIP_EOCD_LEN, "Buffer too short for EOCD"); - if (!zip_read_field (buf, buf_len, EOCD_TOTAL_ENTRIES_OFFSET, cd_entries)) { + if (!zip_read_field (buf, EOCD_TOTAL_ENTRIES_OFFSET, cd_entries)) { log_error (LOG_ASSEMBLY, "Failed to read EOCD 'total number of entries' field"); return false; } - if (!zip_read_field (buf, buf_len, EOCD_CD_START_OFFSET, cd_offset)) { + if (!zip_read_field (buf, EOCD_CD_START_OFFSET, cd_offset)) { log_error (LOG_ASSEMBLY, "Failed to read EOCD 'central directory size' field"); return false; } - if (!zip_read_field (buf, buf_len, EOCD_CD_SIZE_OFFSET, cd_size)) { + if (!zip_read_field (buf, EOCD_CD_SIZE_OFFSET, cd_size)) { log_error (LOG_ASSEMBLY, "Failed to read EOCD 'central directory offset' field"); return false; } @@ -318,15 +340,11 @@ EmbeddedAssemblies::zip_extract_cd_info (uint8_t* buf, size_t buf_len, uint32_t& return true; } -bool -EmbeddedAssemblies::zip_ensure_valid_params (uint8_t* buf, size_t buf_len, size_t index, size_t to_read) +template +force_inline bool +EmbeddedAssemblies::zip_ensure_valid_params (T const& buf, size_t index, size_t to_read) const noexcept { - if (buf == nullptr) { - log_error (LOG_ASSEMBLY, "No buffer to read ZIP data into"); - return false; - } - - if (index + to_read > buf_len) { + if (index + to_read > buf.size ()) { log_error (LOG_ASSEMBLY, "Buffer too short to read %u bytes of data", to_read); return false; } @@ -334,59 +352,62 @@ EmbeddedAssemblies::zip_ensure_valid_params (uint8_t* buf, size_t buf_len, size_ return true; } +template bool -EmbeddedAssemblies::zip_read_field (uint8_t* buf, size_t buf_len, size_t index, uint16_t& u) +EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, uint16_t& dst) const noexcept { - if (!zip_ensure_valid_params (buf, buf_len, index, sizeof (u))) { + if (!zip_ensure_valid_params (src, source_index, sizeof (dst))) { return false; } - u = static_cast((buf [index + 1] << 8) | buf [index]); + dst = static_cast((src [source_index + 1] << 8) | src [source_index]); return true; } +template bool -EmbeddedAssemblies::zip_read_field (uint8_t* buf, size_t buf_len, size_t index, uint32_t& u) +EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, uint32_t& dst) const noexcept { - if (!zip_ensure_valid_params (buf, buf_len, index, sizeof (u))) { + if (!zip_ensure_valid_params (src, source_index, sizeof (dst))) { return false; } - u = (static_cast (buf [index + 3]) << 24) | - (static_cast (buf [index + 2]) << 16) | - (static_cast (buf [index + 1]) << 8) | - (static_cast (buf [index + 0])); + dst = + (static_cast (src [source_index + 3]) << 24) | + (static_cast (src [source_index + 2]) << 16) | + (static_cast (src [source_index + 1]) << 8) | + (static_cast (src [source_index + 0])); return true; } +template bool -EmbeddedAssemblies::zip_read_field (uint8_t* buf, size_t buf_len, size_t index, uint8_t (&sig)[4]) +EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, std::array& dst_sig) const noexcept { - static constexpr size_t sig_size = sizeof(sig); - - if (!zip_ensure_valid_params (buf, buf_len, index, sig_size)) { + if (!zip_ensure_valid_params (src, source_index, dst_sig.size ())) { return false; } - memcpy (sig, buf + index, sig_size); + memcpy (dst_sig.data (), src.data () + source_index, dst_sig.size ()); return true; } +template bool -EmbeddedAssemblies::zip_read_field (uint8_t* buf, size_t buf_len, size_t index, size_t count, dynamic_local_string& characters) +EmbeddedAssemblies::zip_read_field (T const& buf, size_t index, size_t count, dynamic_local_string& characters) const noexcept { - if (!zip_ensure_valid_params (buf, buf_len, index, count)) { + if (!zip_ensure_valid_params (buf, index, count)) { return false; } - characters.assign (reinterpret_cast(buf) + index, count); + characters.assign (reinterpret_cast(buf.data () + index), count); return true; } bool -EmbeddedAssemblies::zip_read_entry_info (uint8_t* buf, size_t buf_len, size_t& buf_offset, uint16_t& compression_method, uint32_t& local_header_offset, uint32_t& file_size, dynamic_local_string& file_name) +EmbeddedAssemblies::zip_read_entry_info (std::vector const& buf, size_t& buf_offset, uint16_t& compression_method, uint32_t& local_header_offset, uint32_t& file_size, dynamic_local_string& file_name) { static constexpr size_t CD_COMPRESSION_METHOD_OFFSET = 10; static constexpr size_t CD_UNCOMPRESSED_SIZE_OFFSET = 24; @@ -396,54 +417,54 @@ EmbeddedAssemblies::zip_read_entry_info (uint8_t* buf, size_t buf_len, size_t& b static constexpr size_t CD_COMMENT_LENGTH_OFFSET = 32; size_t index = buf_offset; - zip_ensure_valid_params (buf, buf_len, index, ZIP_CENTRAL_LEN); + zip_ensure_valid_params (buf, index, ZIP_CENTRAL_LEN); - uint8_t signature[4]; - if (!zip_read_field (buf, buf_len, index, signature)) { + std::array signature; + if (!zip_read_field (buf, index, signature)) { log_error (LOG_ASSEMBLY, "Failed to read Central Directory entry signature"); return false; } - if (memcmp (signature, ZIP_CENTRAL_MAGIC, sizeof(signature)) != 0) { + if (memcmp (signature.data (), ZIP_CENTRAL_MAGIC, sizeof(signature)) != 0) { log_error (LOG_ASSEMBLY, "Invalid Central Directory entry signature"); return false; } index = buf_offset + CD_COMPRESSION_METHOD_OFFSET; - if (!zip_read_field (buf, buf_len, index, compression_method)) { + if (!zip_read_field (buf, index, compression_method)) { log_error (LOG_ASSEMBLY, "Failed to read Central Directory entry 'compression method' field"); return false; } index = buf_offset + CD_UNCOMPRESSED_SIZE_OFFSET;; - if (!zip_read_field (buf, buf_len, index, file_size)) { + if (!zip_read_field (buf, index, file_size)) { log_error (LOG_ASSEMBLY, "Failed to read Central Directory entry 'uncompressed size' field"); return false; } uint16_t file_name_length; index = buf_offset + CD_FILENAME_LENGTH_OFFSET; - if (!zip_read_field (buf, buf_len, index, file_name_length)) { + if (!zip_read_field (buf, index, file_name_length)) { log_error (LOG_ASSEMBLY, "Failed to read Central Directory entry 'file name length' field"); return false; } uint16_t extra_field_length; index = buf_offset + CD_EXTRA_LENGTH_OFFSET; - if (!zip_read_field (buf, buf_len, index, extra_field_length)) { + if (!zip_read_field (buf, index, extra_field_length)) { log_error (LOG_ASSEMBLY, "Failed to read Central Directory entry 'extra field length' field"); return false; } uint16_t comment_length; index = buf_offset + CD_COMMENT_LENGTH_OFFSET; - if (!zip_read_field (buf, buf_len, index, comment_length)) { + if (!zip_read_field (buf, index, comment_length)) { log_error (LOG_ASSEMBLY, "Failed to read Central Directory entry 'file comment length' field"); return false; } index = buf_offset + CD_LOCAL_HEADER_POS_OFFSET; - if (!zip_read_field (buf, buf_len, index, local_header_offset)) { + if (!zip_read_field (buf, index, local_header_offset)) { log_error (LOG_ASSEMBLY, "Failed to read Central Directory entry 'relative offset of local header' field"); return false; } @@ -451,7 +472,7 @@ EmbeddedAssemblies::zip_read_entry_info (uint8_t* buf, size_t buf_len, size_t& b if (file_name_length == 0) { file_name.clear (); - } else if (!zip_read_field (buf, buf_len, index, file_name_length, file_name)) { + } else if (!zip_read_field (buf, index, file_name_length, file_name)) { log_error (LOG_ASSEMBLY, "Failed to read Central Directory entry 'file name' field"); return false; } @@ -459,3 +480,25 @@ EmbeddedAssemblies::zip_read_entry_info (uint8_t* buf, size_t buf_len, size_t& b buf_offset += ZIP_CENTRAL_LEN + file_name_length + extra_field_length + comment_length; return true; } + +template +bool +EmbeddedAssemblies::register_debug_symbols_for_assembly (dynamic_local_string const& entry_name, MonoBundledAssembly const& assembly, const mono_byte *debug_contents, int debug_size) +{ + const char *entry_basename = utils.find_last (entry_name, '/') + 1; // strrchr (entry_name, '/') + 1; + // System.dll, System.dll.mdb case + if (strncmp (assembly.name, entry_basename, strlen (assembly.name)) != 0) { + // That failed; try for System.dll, System.pdb case + const char *eb_ext = utils.find_last (entry_name, '.'); + if (eb_ext == nullptr) + return false; + off_t basename_len = static_cast(eb_ext - entry_basename); + abort_unless (basename_len > 0, "basename must have a length!"); + if (strncmp (assembly.name, entry_basename, static_cast(basename_len)) != 0) + return false; + } + + mono_register_symfile_for_assembly (assembly.name, debug_contents, debug_size); + + return true; +} diff --git a/src/monodroid/jni/embedded-assemblies.cc b/src/monodroid/jni/embedded-assemblies.cc index c854628891b..7ea97831cdb 100644 --- a/src/monodroid/jni/embedded-assemblies.cc +++ b/src/monodroid/jni/embedded-assemblies.cc @@ -64,10 +64,10 @@ void EmbeddedAssemblies::set_assemblies_prefix (const char *prefix) } force_inline void -EmbeddedAssemblies::get_assembly_data (const MonoBundledAssembly *e, char*& assembly_data, uint32_t& assembly_data_size) +EmbeddedAssemblies::get_assembly_data (MonoBundledAssembly const& e, char*& assembly_data, uint32_t& assembly_data_size) { #if defined (ANDROID) && defined (HAVE_LZ4) && defined (RELEASE) - auto header = reinterpret_cast(e->data); + auto header = reinterpret_cast(e.data); if (header->magic == COMPRESSED_DATA_MAGIC) { if (XA_UNLIKELY (compressed_assemblies.descriptors == nullptr)) { log_fatal (LOG_ASSEMBLY, "Compressed assembly found but no descriptor defined"); @@ -79,43 +79,44 @@ EmbeddedAssemblies::get_assembly_data (const MonoBundledAssembly *e, char*& asse } CompressedAssemblyDescriptor &cad = compressed_assemblies.descriptors[header->descriptor_index]; - assembly_data_size = e->size - sizeof(CompressedAssemblyHeader); + assembly_data_size = e.size - sizeof(CompressedAssemblyHeader); if (!cad.loaded) { if (XA_UNLIKELY (cad.data == nullptr)) { log_fatal (LOG_ASSEMBLY, "Invalid compressed assembly descriptor at %u: no data", header->descriptor_index); exit (FATAL_EXIT_MISSING_ASSEMBLY); } + bool log_timing = utils.should_log (LOG_TIMING) && !(log_timing_categories & LOG_TIMING_BARE); timing_period decompress_time; - if (XA_UNLIKELY (utils.should_log (LOG_TIMING))) { + if (XA_UNLIKELY (log_timing)) { decompress_time.mark_start (); } if (header->uncompressed_length != cad.uncompressed_file_size) { if (header->uncompressed_length > cad.uncompressed_file_size) { - log_fatal (LOG_ASSEMBLY, "Compressed assembly '%s' is larger than when the application was built (expected at most %u, got %u). Assemblies don't grow just like that!", e->name, cad.uncompressed_file_size, header->uncompressed_length); + log_fatal (LOG_ASSEMBLY, "Compressed assembly '%s' is larger than when the application was built (expected at most %u, got %u). Assemblies don't grow just like that!", e.name, cad.uncompressed_file_size, header->uncompressed_length); exit (FATAL_EXIT_MISSING_ASSEMBLY); } else { - log_debug (LOG_ASSEMBLY, "Compressed assembly '%s' is smaller than when the application was built. Adjusting accordingly.", e->name); + log_debug (LOG_ASSEMBLY, "Compressed assembly '%s' is smaller than when the application was built. Adjusting accordingly.", e.name); } cad.uncompressed_file_size = header->uncompressed_length; } - const char *data_start = reinterpret_cast(e->data + sizeof(CompressedAssemblyHeader)); + const char *data_start = reinterpret_cast(e.data + sizeof(CompressedAssemblyHeader)); int ret = LZ4_decompress_safe (data_start, reinterpret_cast(cad.data), static_cast(assembly_data_size), static_cast(cad.uncompressed_file_size)); - if (XA_UNLIKELY (utils.should_log (LOG_TIMING))) { + if (XA_UNLIKELY (log_timing)) { decompress_time.mark_end (); - TIMING_LOG_INFO (decompress_time, "%s LZ4 decompression time", e->name); + TIMING_LOG_INFO (decompress_time, "%s LZ4 decompression time", e.name); } if (ret < 0) { - log_fatal (LOG_ASSEMBLY, "Decompression of assembly %s failed with code %d", e->name, ret); + log_fatal (LOG_ASSEMBLY, "Decompression of assembly %s failed with code %d", e.name, ret); exit (FATAL_EXIT_MISSING_ASSEMBLY); } if (static_cast(ret) != cad.uncompressed_file_size) { - log_debug (LOG_ASSEMBLY, "Decompression of assembly %s yielded a different size (expected %lu, got %u)", e->name, cad.uncompressed_file_size, static_cast(ret)); + log_debug (LOG_ASSEMBLY, "Decompression of assembly %s yielded a different size (expected %lu, got %u)", e.name, cad.uncompressed_file_size, static_cast(ret)); exit (FATAL_EXIT_MISSING_ASSEMBLY); } cad.loaded = true; @@ -125,8 +126,8 @@ EmbeddedAssemblies::get_assembly_data (const MonoBundledAssembly *e, char*& asse } else #endif { - assembly_data = reinterpret_cast(const_cast(e->data)); - assembly_data_size = e->size; + assembly_data = reinterpret_cast(const_cast(e.data)); + assembly_data_size = e.size; } } @@ -172,43 +173,48 @@ EmbeddedAssemblies::open_from_bundles (MonoAssemblyName* aname, std::function name; if (culture != nullptr && *culture != '\0') { - name.append (culture); - name.append ("/"); - } - name.append (asmname); - if (!utils.ends_with (name.get (), ".dll")) { - name.append (".dll"); + name.append_c (culture); + name.append (path_separator); } + name.append_c (asmname); - MonoAssembly *a = nullptr; - MonoBundledAssembly **p; + constexpr char dll_extension[] = ".dll"; + if (!utils.ends_with (name, dll_extension)) { + name.append (dll_extension); + } - log_info (LOG_ASSEMBLY, "open_from_bundles: looking for bundled name: '%s'", name.get ()); + log_debug (LOG_ASSEMBLY, "open_from_bundles: looking for bundled name: '%s'", name.get ()); dynamic_local_string abi_name; abi_name - .assign (BasicAndroidSystem::get_built_for_abi_name ()) - .append ("/") - .append (name.get ()); - - for (p = bundled_assemblies; p != nullptr && *p; ++p) { - MonoImage *image = nullptr; - MonoImageOpenStatus status; - const MonoBundledAssembly *e = *p; - char *assembly_data = nullptr; - uint32_t assembly_data_size; - - if (strcmp (e->name, name.get ()) != 0) { - if (strcmp (e->name, abi_name.get ()) != 0) { + .assign_c (BasicAndroidSystem::get_built_for_abi_name ()) + .append (path_separator) + .append (name); + + MonoImage *image; + MonoImageOpenStatus status; + char *assembly_data; + uint32_t assembly_data_size; + MonoAssembly *a = nullptr; + + for (auto const& assembly : bundled_assemblies) { + if (assembly.name == nullptr) { + // There are no "empty" entries interleaved with "filled" ones + break; + } + + if (strcmp (assembly.name, name.get ()) != 0) { + if (strcmp (assembly.name, abi_name.get ()) != 0) { continue; } else { - log_info (LOG_ASSEMBLY, "open_from_bundles: found architecture-specific: '%s'", abi_name.get ()); + log_debug (LOG_ASSEMBLY, "open_from_bundles: found architecture-specific: '%s'", abi_name.get ()); } } - get_assembly_data (e, assembly_data, assembly_data_size); + get_assembly_data (assembly, assembly_data, assembly_data_size); image = loader (assembly_data, assembly_data_size, name.get ()); if (image == nullptr) { continue; @@ -219,12 +225,15 @@ EmbeddedAssemblies::open_from_bundles (MonoAssemblyName* aname, std::function java_type_name (mono_string_to_utf8 (java_type)); + c_unique_ptr java_type_name {mono_string_to_utf8 (java_type)}; if (XA_UNLIKELY (!java_type_name || *java_type_name == '\0')) { log_warn (LOG_ASSEMBLY, "typemap: empty Java type name passed to 'typemap_java_to_managed'"); return nullptr; @@ -488,7 +497,7 @@ EmbeddedAssemblies::typemap_managed_to_java (const char *managed_type_name) inline const char* EmbeddedAssemblies::typemap_managed_to_java ([[maybe_unused]] MonoType *type, MonoClass *klass, [[maybe_unused]] const uint8_t *mvid) { - simple_pointer_guard type_name (mono_type_get_name_full (type, MONO_TYPE_NAME_FORMAT_FULL_NAME)); + c_unique_ptr type_name {mono_type_get_name_full (type, MONO_TYPE_NAME_FORMAT_FULL_NAME)}; MonoImage *image = mono_class_get_image (klass); const char *image_name = mono_image_get_name (image); size_t type_name_len = strlen (type_name.get ()); @@ -654,27 +663,6 @@ EmbeddedAssemblies::md_mmap_apk_file (int fd, uint32_t offset, uint32_t size, co return file_info; } -bool -EmbeddedAssemblies::register_debug_symbols_for_assembly (const char *entry_name, MonoBundledAssembly *assembly, const mono_byte *debug_contents, int debug_size) -{ - const char *entry_basename = strrchr (entry_name, '/') + 1; - // System.dll, System.dll.mdb case - if (strncmp (assembly->name, entry_basename, strlen (assembly->name)) != 0) { - // That failed; try for System.dll, System.pdb case - const char *eb_ext = strrchr (entry_basename, '.'); - if (eb_ext == nullptr) - return false; - off_t basename_len = static_cast(eb_ext - entry_basename); - abort_unless (basename_len > 0, "basename must have a length!"); - if (strncmp (assembly->name, entry_basename, static_cast(basename_len)) != 0) - return false; - } - - mono_register_symfile_for_assembly (assembly->name, debug_contents, debug_size); - - return true; -} - void EmbeddedAssemblies::gather_bundled_assemblies_from_apk (const char* apk, monodroid_should_register should_register) { @@ -716,8 +704,8 @@ EmbeddedAssemblies::typemap_read_header ([[maybe_unused]] int dir_fd, const char int res; #if __ANDROID_API__ < 21 - simple_pointer_guard full_file_path = utils.path_combine (dir_path, file_path); - res = stat (full_file_path, &sbuf); + std::unique_ptr full_file_path {utils.path_combine (dir_path, file_path)}; + res = stat (full_file_path.get (), &sbuf); #else res = fstatat (dir_fd, file_path, &sbuf, 0); #endif @@ -733,7 +721,7 @@ EmbeddedAssemblies::typemap_read_header ([[maybe_unused]] int dir_fd, const char } #if __ANDROID_API__ < 21 - fd = open (full_file_path, O_RDONLY); + fd = open (full_file_path.get (), O_RDONLY); #else fd = openat (dir_fd, file_path, O_RDONLY); #endif @@ -766,7 +754,7 @@ EmbeddedAssemblies::typemap_read_header ([[maybe_unused]] int dir_fd, const char return true; } -uint8_t* +std::unique_ptr EmbeddedAssemblies::typemap_load_index (TypeMapIndexHeader &header, size_t file_size, int index_fd) { size_t entry_size = header.module_file_name_width; @@ -776,14 +764,14 @@ EmbeddedAssemblies::typemap_load_index (TypeMapIndexHeader &header, size_t file_ return nullptr; } - auto data = new uint8_t [data_size]; - ssize_t nread = do_read (index_fd, data, data_size); + auto data = std::make_unique (data_size); + ssize_t nread = do_read (index_fd, data.get (), data_size); if (nread != static_cast(data_size)) { log_error (LOG_ASSEMBLY, "typemap: failed to read %u bytes from index file. %s", data_size, strerror (errno)); return nullptr; } - uint8_t *p = data; + uint8_t *p = data.get (); for (size_t i = 0; i < type_map_count; i++) { type_maps[i].assembly_name = reinterpret_cast(p); p += entry_size; @@ -792,7 +780,7 @@ EmbeddedAssemblies::typemap_load_index (TypeMapIndexHeader &header, size_t file_ return data; } -uint8_t* +std::unique_ptr EmbeddedAssemblies::typemap_load_index (int dir_fd, const char *dir_path, const char *index_path) { log_debug (LOG_ASSEMBLY, "typemap: loading TypeMap index file '%s/%s'", dir_path, index_path); @@ -800,17 +788,14 @@ EmbeddedAssemblies::typemap_load_index (int dir_fd, const char *dir_path, const TypeMapIndexHeader header; size_t file_size; int fd = -1; - uint8_t *data = nullptr; + std::unique_ptr data; - if (!typemap_read_header (dir_fd, "TypeMap index", dir_path, index_path, MODULE_INDEX_MAGIC, header, file_size, fd)) { - goto cleanup; + if (typemap_read_header (dir_fd, "TypeMap index", dir_path, index_path, MODULE_INDEX_MAGIC, header, file_size, fd)) { + type_map_count = header.entry_count; + type_maps = new TypeMap[type_map_count](); + data = typemap_load_index (header, file_size, fd); } - type_map_count = header.entry_count; - type_maps = new TypeMap[type_map_count](); - data = typemap_load_index (header, file_size, fd); - - cleanup: if (fd >= 0) close (fd); @@ -933,9 +918,9 @@ EmbeddedAssemblies::try_load_typemaps_from_directory (const char *path) return; } - simple_pointer_guard dir_path (utils.path_combine (path, "typemaps")); + std::unique_ptr dir_path {utils.path_combine (path, "typemaps")}; monodroid_dir_t *dir; - if ((dir = utils.monodroid_opendir (dir_path)) == nullptr) { + if ((dir = utils.monodroid_opendir (dir_path.get ())) == nullptr) { log_warn (LOG_ASSEMBLY, "typemap: could not open directory: `%s`", dir_path.get ()); return; } @@ -951,7 +936,7 @@ EmbeddedAssemblies::try_load_typemaps_from_directory (const char *path) // The pointer must be stored here because, after index is loaded, module.assembly_name points into the index data // and must be valid until after the actual module file is loaded. - simple_pointer_guard index_data = typemap_load_index (dir_fd, dir_path, index_name); + std::unique_ptr index_data = typemap_load_index (dir_fd, dir_path.get (), index_name); if (!index_data) { log_fatal (LOG_ASSEMBLY, "typemap: unable to load TypeMap data index from '%s/%s'", dir_path.get (), index_name); exit (FATAL_EXIT_NO_ASSEMBLIES); // TODO: use a new error code here @@ -959,7 +944,7 @@ EmbeddedAssemblies::try_load_typemaps_from_directory (const char *path) for (size_t i = 0; i < type_map_count; i++) { TypeMap *module = &type_maps[i]; - if (!typemap_load_file (dir_fd, dir_path, module->assembly_name, *module)) { + if (!typemap_load_file (dir_fd, dir_path.get (), module->assembly_name, *module)) { continue; } } @@ -971,17 +956,11 @@ EmbeddedAssemblies::try_load_typemaps_from_directory (const char *path) size_t EmbeddedAssemblies::register_from (const char *apk_file, monodroid_should_register should_register) { - size_t prev = bundled_assemblies_count; + size_t prev = bundled_assembly_index; gather_bundled_assemblies_from_apk (apk_file, should_register); - log_info (LOG_ASSEMBLY, "Package '%s' contains %i assemblies", apk_file, bundled_assemblies_count - prev); - - if (bundled_assemblies) { - size_t alloc_size = MULTIPLY_WITH_OVERFLOW_CHECK (size_t, sizeof(void*), bundled_assemblies_count + 1); - bundled_assemblies = reinterpret_cast (utils.xrealloc (bundled_assemblies, alloc_size)); - bundled_assemblies [bundled_assemblies_count] = nullptr; - } + log_info (LOG_ASSEMBLY, "Package '%s' contains %i assemblies", apk_file, bundled_assembly_index - prev); - return bundled_assemblies_count; + return bundled_assembly_index; } diff --git a/src/monodroid/jni/embedded-assemblies.hh b/src/monodroid/jni/embedded-assemblies.hh index 88eb07056e8..3cede35756a 100644 --- a/src/monodroid/jni/embedded-assemblies.hh +++ b/src/monodroid/jni/embedded-assemblies.hh @@ -2,9 +2,21 @@ #ifndef INC_MONODROID_EMBEDDED_ASSEMBLIES_H #define INC_MONODROID_EMBEDDED_ASSEMBLIES_H +#include + +#undef HAVE_CONCEPTS + +// Xcode has supports for concepts only since 12.5 +#if __has_include () +#define HAVE_CONCEPTS +#include +#endif // __has_include + #include #include #include +#include + #include #include @@ -22,6 +34,18 @@ namespace xamarin::android::internal { #if defined (DEBUG) || !defined (ANDROID) struct TypeMappingInfo; #endif + +#if defined (HAVE_CONCEPTS) + template + concept ByteArrayContainer = requires (T a) { + a.size (); + a.data (); + requires std::same_as; + }; +#else +#define ByteArrayContainer class +#endif + class EmbeddedAssemblies { struct md_mmap_info { @@ -106,14 +130,15 @@ namespace xamarin::android::internal { #if defined (DEBUG) || !defined (ANDROID) template bool typemap_read_header (int dir_fd, const char *file_type, const char *dir_path, const char *file_path, uint32_t expected_magic, H &header, size_t &file_size, int &fd); - uint8_t* typemap_load_index (int dir_fd, const char *dir_path, const char *index_path); - uint8_t* typemap_load_index (TypeMapIndexHeader &header, size_t file_size, int index_fd); + std::unique_ptr typemap_load_index (int dir_fd, const char *dir_path, const char *index_path); + std::unique_ptr typemap_load_index (TypeMapIndexHeader &header, size_t file_size, int index_fd); bool typemap_load_file (int dir_fd, const char *dir_path, const char *file_path, TypeMap &module); bool typemap_load_file (BinaryTypeMapHeader &header, const char *dir_path, const char *file_path, int file_fd, TypeMap &module); static ssize_t do_read (int fd, void *buf, size_t count); const TypeMapEntry *typemap_managed_to_java (const char *managed_type_name); #endif // DEBUG || !ANDROID - bool register_debug_symbols_for_assembly (const char *entry_name, MonoBundledAssembly *assembly, const mono_byte *debug_contents, int debug_size); + template + bool register_debug_symbols_for_assembly (dynamic_local_string const& entry_name, MonoBundledAssembly const& assembly, const mono_byte *debug_contents, int debug_size); static md_mmap_info md_mmap_apk_file (int fd, uint32_t offset, uint32_t size, const char* filename, const char* apk); static MonoAssembly* open_from_bundles_full (MonoAssemblyName *aname, char **assemblies_path, void *user_data); @@ -122,24 +147,53 @@ namespace xamarin::android::internal { #else // def NET6 static MonoAssembly* open_from_bundles_refonly (MonoAssemblyName *aname, char **assemblies_path, void *user_data); #endif // ndef NET6 - static void get_assembly_data (const MonoBundledAssembly *e, char*& assembly_data, uint32_t& assembly_data_size); + static void get_assembly_data (MonoBundledAssembly const& e, char*& assembly_data, uint32_t& assembly_data_size); void zip_load_entries (int fd, const char *apk_name, monodroid_should_register should_register); bool zip_read_cd_info (int fd, uint32_t& cd_offset, uint32_t& cd_size, uint16_t& cd_entries); bool zip_adjust_data_offset (int fd, size_t local_header_offset, uint32_t &data_start_offset); - bool zip_extract_cd_info (uint8_t* buf, size_t buf_len, uint32_t& cd_offset, uint32_t& cd_size, uint16_t& cd_entries); - bool zip_ensure_valid_params (uint8_t* buf, size_t buf_len, size_t index, size_t to_read); - bool zip_read_field (uint8_t* buf, size_t buf_len, size_t index, uint16_t& u); - bool zip_read_field (uint8_t* buf, size_t buf_len, size_t index, uint32_t& u); - bool zip_read_field (uint8_t* buf, size_t buf_len, size_t index, uint8_t (&sig)[4]); - bool zip_read_field (uint8_t* buf, size_t buf_len, size_t index, size_t count, dynamic_local_string& characters); - bool zip_read_entry_info (uint8_t* buf, size_t buf_len, size_t& buf_offset, uint16_t& compression_method, uint32_t& local_header_offset, uint32_t& file_size, dynamic_local_string& file_name); + + template + bool zip_extract_cd_info (std::array const& buf, uint32_t& cd_offset, uint32_t& cd_size, uint16_t& cd_entries); + + template + bool zip_ensure_valid_params (T const& buf, size_t index, size_t to_read) const noexcept; + + // template + // bool zip_read_field (std::array const& buf, size_t index, uint16_t& u) + // { + // return zip_read_field_unchecked (buf, u, index); + // } + + // bool zip_read_field (std::vector const& buf, size_t index, uint16_t& u) + // { + // return zip_read_field_unchecked (buf, u, index); + // } + + template + bool zip_read_field (T const& src, size_t source_index, uint16_t& dst) const noexcept; + + template + bool zip_read_field (T const& src, size_t source_index, uint32_t& dst) const noexcept; + + template + bool zip_read_field (T const& src, size_t source_index, std::array& dst_sig) const noexcept; + + template + bool zip_read_field (T const& buf, size_t index, size_t count, dynamic_local_string& characters) const noexcept; + + bool zip_read_entry_info (std::vector const& buf, size_t& buf_offset, uint16_t& compression_method, uint32_t& local_header_offset, uint32_t& file_size, dynamic_local_string& file_name); const char* get_assemblies_prefix () const { return assemblies_prefix_override != nullptr ? assemblies_prefix_override : assemblies_prefix; } + size_t get_assemblies_prefix_length () const noexcept + { + return assemblies_prefix_override != nullptr ? strlen (assemblies_prefix_override) : sizeof(assemblies_prefix) - 1; + } + template const Entry* binary_search (const Key *key, const Entry *base, size_t nmemb, size_t extra_size = 0); @@ -153,8 +207,8 @@ namespace xamarin::android::internal { private: bool register_debug_symbols; - MonoBundledAssembly **bundled_assemblies = nullptr; - size_t bundled_assemblies_count; + std::vector bundled_assemblies; + size_t bundled_assembly_index = 0; #if defined (DEBUG) || !defined (ANDROID) TypeMappingInfo *java_to_managed_maps; TypeMappingInfo *managed_to_java_maps; diff --git a/src/monodroid/jni/monodroid-glue-internal.hh b/src/monodroid/jni/monodroid-glue-internal.hh index 1e567f30a64..f8624d36d7d 100644 --- a/src/monodroid/jni/monodroid-glue-internal.hh +++ b/src/monodroid/jni/monodroid-glue-internal.hh @@ -118,6 +118,7 @@ namespace xamarin::android::internal true; #endif +#if !defined (NET6) #define MAKE_API_DSO_NAME(_ext_) "libxa-internal-api." # _ext_ #if defined (WINDOWS) static constexpr char API_DSO_NAME[] = MAKE_API_DSO_NAME (dll); @@ -126,6 +127,7 @@ namespace xamarin::android::internal #else // !defined(WINDOWS) && !defined(APPLE_OS_X) static constexpr char API_DSO_NAME[] = MAKE_API_DSO_NAME (so); #endif // defined(WINDOWS) +#endif // ndef NET6 public: static constexpr int XA_LOG_COUNTERS = MONO_COUNTER_JIT | MONO_COUNTER_METADATA | MONO_COUNTER_GC | MONO_COUNTER_GENERICS | MONO_COUNTER_INTERP; diff --git a/src/monodroid/jni/monodroid-glue.cc b/src/monodroid/jni/monodroid-glue.cc index 2b07d43cb8c..9b1c56ee489 100644 --- a/src/monodroid/jni/monodroid-glue.cc +++ b/src/monodroid/jni/monodroid-glue.cc @@ -108,13 +108,6 @@ void *MonodroidRuntime::api_dso_handle = nullptr; #else // ndef NET6 std::mutex MonodroidRuntime::pinvoke_map_write_lock; -// The pragmas will go away once we switch to C++20, where the designator becomes part of the language standard. Both -// gcc and clang support it now as an extension, though. -#if defined (__clang__) -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wc99-designator" -#endif - MonoCoreRuntimeProperties MonodroidRuntime::monovm_core_properties = { .trusted_platform_assemblies = nullptr, .app_paths = nullptr, @@ -122,9 +115,6 @@ MonoCoreRuntimeProperties MonodroidRuntime::monovm_core_properties = { .pinvoke_override = &MonodroidRuntime::monodroid_pinvoke_override }; -#if defined (__clang__) -#pragma clang diagnostic pop -#endif #endif // def NET6 #ifdef WINDOWS @@ -331,8 +321,10 @@ MonodroidRuntime::open_from_update_dir (MonoAssemblyName *aname, [[maybe_unused] size_t name_len = strlen (name); static_local_string pname (name_len + culture_len); - if (culture_len > 0) + if (culture_len > 0) { pname.append (culture, culture_len); + pname.append ("/"); + } pname.append (name, name_len); constexpr char dll_extension[] = ".dll"; @@ -375,6 +367,10 @@ bool MonodroidRuntime::should_register_file ([[maybe_unused]] const char *filename) { #ifndef RELEASE + if (filename == nullptr) { + return true; + } + size_t filename_len = strlen (filename) + 1; // includes space for path separator for (size_t i = 0; i < AndroidSystem::MAX_OVERRIDES; ++i) { const char *odir = androidSystem.get_override_dir (i); @@ -512,12 +508,15 @@ MonodroidRuntime::Java_JNI_OnLoad (JavaVM *vm, [[maybe_unused]] void *reserved) void MonodroidRuntime::parse_gdb_options () { - char *val; + dynamic_local_string val; - if (!(androidSystem.monodroid_get_system_property (Debug::DEBUG_MONO_GDB_PROPERTY, &val) > 0)) + if (!(androidSystem.monodroid_get_system_property (Debug::DEBUG_MONO_GDB_PROPERTY, val) > 0)) return; - if (strstr (val, "wait:") == val) { + constexpr char wait_param[] = "wait:"; + constexpr size_t wait_param_length = sizeof(wait_param) - 1; + + if (val.starts_with (wait_param)) { /* * The form of the property should be: 'wait:', where should be * the output of date +%s in the android shell. @@ -527,20 +526,18 @@ MonodroidRuntime::parse_gdb_options () */ bool do_wait = true; - long long v = atoll (val + strlen ("wait:")); + long long v = atoll (val.get () + wait_param_length); if (v > 100000) { time_t secs = time (nullptr); if (v + 10 < secs) { - log_warn (LOG_DEFAULT, "Found stale %s property with value '%s', not waiting.", Debug::DEBUG_MONO_GDB_PROPERTY, val); + log_warn (LOG_DEFAULT, "Found stale %s property with value '%s', not waiting.", Debug::DEBUG_MONO_GDB_PROPERTY, val.get ()); do_wait = false; } } wait_for_gdb = do_wait; } - - delete[] val; } #if defined (DEBUG) && !defined (WINDOWS) @@ -769,9 +766,9 @@ MonodroidRuntime::mono_runtime_init ([[maybe_unused]] dynamic_local_string jit_log_path = utils.path_combine (androidSystem.get_override_dir (0), "methods.txt"); - jit_log = utils.monodroid_fopen (jit_log_path, "a"); - utils.set_world_accessable (jit_log_path); + std::unique_ptr jit_log_path {utils.path_combine (androidSystem.get_override_dir (0), "methods.txt")}; + jit_log = utils.monodroid_fopen (jit_log_path.get (), "a"); + utils.set_world_accessable (jit_log_path.get ()); } profiler_handle = mono_profiler_create (nullptr); @@ -917,9 +914,7 @@ MonodroidRuntime::create_domain (JNIEnv *env, jstring_array_wrapper &runtimeApks if constexpr (is_running_on_desktop) { if (is_root_domain) { - // Check that our corlib is coherent with the version of Mono we loaded otherwise - // tell the IDE that the project likely need to be recompiled. - simple_pointer_guard corlib_error_message_guard = const_cast(mono_check_corlib_version ()); + c_unique_ptr corlib_error_message_guard {const_cast(mono_check_corlib_version ())}; char *corlib_error_message = corlib_error_message_guard.get (); if (corlib_error_message == nullptr) { @@ -1309,7 +1304,7 @@ MonodroidRuntime::monodroid_dlopen (const char *name, int flags, char **err) size_t len = base_len + LIBAOT_PREFIX_SIZE; // includes the trailing \0 static_local_string basename (len); - basename.append (LIBAOT_PREFIX).append (basename_part); + basename.append (LIBAOT_PREFIX).append_c (basename_part); h = androidSystem.load_dso_from_any_directories (basename.get (), dl_flags); @@ -1323,7 +1318,6 @@ MonodroidRuntime::monodroid_dlopen (const char *name, int flags, char **err) void* MonodroidRuntime::monodroid_dlopen (const char *name, int flags, char **err, [[maybe_unused]] void *user_data) { - log_warn (LOG_DEFAULT, "MonodroidRuntime::monodroid_dlopen (\"%s\", 0x%x, %p, %p)", name, flags, err, user_data); if (name == nullptr) { log_warn (LOG_ASSEMBLY, "monodroid_dlopen got a null name. This is not supported in NET6+"); return nullptr; @@ -1339,7 +1333,7 @@ MonodroidRuntime::monodroid_dlopen (const char *name, int flags, char **err, [[m size_t len = ADD_WITH_OVERFLOW_CHECK (size_t, strlen (name), DSO_EXTENSION_SIZE); // includes the trailing \0 static_local_string full_name (len); - full_name.append (name).append (DSO_EXTENSION); + full_name.append_c (name).append (DSO_EXTENSION); return monodroid_dlopen (full_name.get (), flags, err); } #else // def NET6 && def ANDROID @@ -1449,7 +1443,7 @@ MonodroidRuntime::monodroid_dlopen (const char *name, int flags, char **err, [[m size_t len = base_len + LIBAOT_PREFIX_SIZE; // includes the trailing \0 static_local_string basename (len); - basename.append (LIBAOT_PREFIX).append (basename_part); + basename.append (LIBAOT_PREFIX).append_c (basename_part); h = androidSystem.load_dso_from_any_directories (basename.get (), dl_flags); @@ -1585,7 +1579,7 @@ MonodroidRuntime::set_profile_options () if (androidSystem.monodroid_get_system_property (Debug::DEBUG_MONO_PROFILE_PROPERTY, prop_value) == 0) return; - value.assign (prop_value.get (), prop_value.length ()); + value.assign (prop_value); } // setenv(3) makes copies of its arguments @@ -1629,11 +1623,8 @@ MonodroidRuntime::set_profile_options () if (!have_output_arg) { constexpr char MLPD_EXT[] = "mlpd"; - constexpr size_t MLPD_EXT_LEN = sizeof(MLPD_EXT) - 1; constexpr char AOT_EXT[] = "aotprofile"; - constexpr size_t AOT_EXT_LEN = sizeof(AOT_EXT) - 1; constexpr char COV_EXT[] = "xml"; - constexpr size_t COV_EXT_LEN = sizeof(COV_EXT) - 1; constexpr char LOG_PREFIX[] = "log:"; constexpr size_t LOG_PREFIX_LENGTH = sizeof(LOG_PREFIX) - 1; constexpr char AOT_PREFIX[] = "aot:"; @@ -1643,23 +1634,22 @@ MonodroidRuntime::set_profile_options () constexpr char COVERAGE_PREFIX[] = "coverage:"; constexpr size_t COVERAGE_PREFIX_LENGTH = sizeof(COVERAGE_PREFIX) - 1; constexpr char PROFILE_FILE_NAME_PREFIX[] = "profile."; - constexpr size_t PROFILE_FILE_NAME_PREFIX_LEN = sizeof(PROFILE_FILE_NAME_PREFIX) - 1; size_t length_adjust = colon_idx >= 1 ? 0 : 1; output_path - .assign (androidSystem.get_override_dir (0)) + .assign_c (androidSystem.get_override_dir (0)) .append (MONODROID_PATH_SEPARATOR) - .append (PROFILE_FILE_NAME_PREFIX, PROFILE_FILE_NAME_PREFIX_LEN); + .append (PROFILE_FILE_NAME_PREFIX); if (value.starts_with (LOG_PREFIX, LOG_PREFIX_LENGTH - length_adjust)) { - output_path.append (MLPD_EXT, MLPD_EXT_LEN); + output_path.append (MLPD_EXT); } else if (value.starts_with (AOT_PREFIX, AOT_PREFIX_LENGTH - length_adjust)) { - output_path.append (AOT_EXT, AOT_EXT_LEN); + output_path.append (AOT_EXT); } else if (value.starts_with (DEFAULT_PREFIX, DEFAULT_PREFIX_LENGTH - length_adjust)) { - output_path.append (MLPD_EXT, MLPD_EXT_LEN); + output_path.append (MLPD_EXT); } else if (value.starts_with (COVERAGE_PREFIX, COVERAGE_PREFIX_LENGTH - length_adjust)) { - output_path.append (COV_EXT, COV_EXT_LEN); + output_path.append (COV_EXT); } else { size_t len = colon_idx < 0 ? value.length () : static_cast(colon_idx + 1); output_path.append (value.get (), len); @@ -1670,7 +1660,7 @@ MonodroidRuntime::set_profile_options () else value.append (","); value - .append (OUTPUT_ARG, OUTPUT_ARG_LEN) + .append (OUTPUT_ARG) .append (output_path.get (), output_path.length ()); } @@ -1717,8 +1707,6 @@ MonodroidRuntime::disable_external_signal_handlers (void) inline void MonodroidRuntime::load_assembly (MonoAssemblyLoadContextGCHandle alc_handle, jstring_wrapper &assembly) { - log_warn (LOG_ASSEMBLY, __PRETTY_FUNCTION__); - timing_period total_time; if (XA_UNLIKELY (utils.should_log (LOG_TIMING))) total_time.mark_start (); @@ -1913,18 +1901,19 @@ MonodroidRuntime::setup_mono_tracing (char const* const& mono_log_mask) constexpr size_t MASK_DLL_LEN = sizeof(MASK_DLL) - 1; constexpr char MASK_GC[] = "gc"; constexpr size_t MASK_GC_LEN = sizeof(MASK_GC) - 1; + constexpr char COMMA[] = ","; dynamic_local_string log_mask; if (mono_log_mask == nullptr || *mono_log_mask == '\0') { if (have_log_assembly) { log_mask.append (MASK_ASM); - log_mask.append (","); + log_mask.append (COMMA); log_mask.append (MASK_DLL); } if (have_log_gc) { if (log_mask.length () > 0) { - log_mask.append (","); + log_mask.append (COMMA); } log_mask.append (MASK_GC); } @@ -1941,7 +1930,7 @@ MonodroidRuntime::setup_mono_tracing (char const* const& mono_log_mask) bool need_asm = have_log_assembly, need_dll = have_log_assembly, need_gc = have_log_gc; dynamic_local_string input_log_mask; - input_log_mask.assign (mono_log_mask); + input_log_mask.assign_c (mono_log_mask); string_segment token; while (input_log_mask.next_token (',', token)) { @@ -1960,17 +1949,17 @@ MonodroidRuntime::setup_mono_tracing (char const* const& mono_log_mask) } if (need_asm) { - input_log_mask.append (","); + input_log_mask.append (COMMA); input_log_mask.append (MASK_ASM); } if (need_dll) { - input_log_mask.append (","); + input_log_mask.append (COMMA); input_log_mask.append (MASK_DLL); } if (need_gc) { - input_log_mask.append (","); + input_log_mask.append (COMMA); input_log_mask.append (MASK_GC); } @@ -2092,11 +2081,12 @@ MonodroidRuntime::Java_mono_android_Runtime_initInternal (JNIEnv *env, jclass kl utils.set_world_accessable (counters_path.get ()); } +#if !defined (NET6) void *dso_handle = nullptr; #if defined (WINDOWS) || defined (APPLE_OS_X) const char *my_location = get_my_location (); if (my_location != nullptr) { - simple_pointer_guard dso_path (utils.path_combine (my_location, API_DSO_NAME)); + std::unique_ptr dso_path {utils.path_combine (my_location, API_DSO_NAME)}; log_info (LOG_DEFAULT, "Attempting to load %s", dso_path.get ()); dso_handle = java_interop_lib_load (dso_path.get (), JAVA_INTEROP_LIB_LOAD_GLOBALLY, nullptr); #if defined (APPLE_OS_X) @@ -2113,7 +2103,7 @@ MonodroidRuntime::Java_mono_android_Runtime_initInternal (JNIEnv *env, jclass kl #endif // defined(WINDOWS) || defined(APPLE_OS_X) if (dso_handle == nullptr) dso_handle = androidSystem.load_dso_from_any_directories (API_DSO_NAME, JAVA_INTEROP_LIB_LOAD_GLOBALLY); -#if !defined (NET6) + init_internal_api_dso (dso_handle); #endif // ndef NET6 mono_dl_fallback_register (monodroid_dlopen, monodroid_dlsym, nullptr, nullptr); diff --git a/src/monodroid/jni/new_delete.cc b/src/monodroid/jni/new_delete.cc index e19051e9fbe..2e853a501f6 100644 --- a/src/monodroid/jni/new_delete.cc +++ b/src/monodroid/jni/new_delete.cc @@ -1,5 +1,10 @@ #include -#include + +namespace std +{ + struct nothrow_t {}; + extern const nothrow_t nothrow; +} #include "java-interop-util.h" @@ -9,6 +14,7 @@ do_alloc (size_t size) return ::malloc (size == 0 ? 1 : size); } +__attribute__((__weak__)) void* operator new (size_t size) { @@ -27,6 +33,7 @@ operator new (size_t size, const std::nothrow_t&) noexcept return do_alloc (size); } +__attribute__((__weak__)) void* operator new[] (size_t size) { @@ -39,8 +46,9 @@ operator new[] (size_t size, const std::nothrow_t&) noexcept return do_alloc (size); } +__attribute__((__weak__)) void -operator delete (void* ptr) +operator delete (void* ptr) noexcept { if (ptr) ::free (ptr); @@ -58,8 +66,9 @@ operator delete (void* ptr, size_t) noexcept ::operator delete (ptr); } +__attribute__((__weak__)) void -operator delete[] (void* ptr) +operator delete[] (void* ptr) noexcept { ::operator delete (ptr); } diff --git a/src/monodroid/jni/pinvoke-override-api.cc b/src/monodroid/jni/pinvoke-override-api.cc index a9f55223021..39da7ac64ad 100644 --- a/src/monodroid/jni/pinvoke-override-api.cc +++ b/src/monodroid/jni/pinvoke-override-api.cc @@ -633,13 +633,6 @@ MonodroidRuntime::fetch_or_create_pinvoke_map_entry (std::string const& library_ void* MonodroidRuntime::monodroid_pinvoke_override (const char *library_name, const char *entrypoint_name) { - log_warn (LOG_DEFAULT, "MonodroidRuntime::monodroid_pinvoke_override (\"%s\", \"%s\")", library_name, entrypoint_name); - - timing_period total_time; - if (XA_UNLIKELY (utils.should_log (LOG_TIMING))) { - total_time.mark_start (); - } - if (library_name == nullptr || *library_name == '\0' || entrypoint_name == nullptr || *entrypoint_name == '\0') { return nullptr; } @@ -688,19 +681,9 @@ MonodroidRuntime::monodroid_pinvoke_override (const char *library_name, const ch handle = fetch_or_create_pinvoke_map_entry (lib_name, func_name, iter->second, /* need_lock */ true); } - if (XA_UNLIKELY (utils.should_log (LOG_TIMING))) { - total_time.mark_end (); - - TIMING_LOG_INFO (total_time, "p/invoke override for '%s' (foreign)", entrypoint_name); - } - return handle; } - timing_period lookup_time; - if (XA_UNLIKELY (utils.should_log (LOG_TIMING))) { - lookup_time.mark_start (); - } auto iter = xa_pinvoke_map.find (entrypoint_name); if (iter == xa_pinvoke_map.end ()) { log_fatal (LOG_ASSEMBLY, "Internal p/invoke symbol '%s @ %s' not found in compile-time map.", library_name, entrypoint_name); @@ -712,13 +695,5 @@ MonodroidRuntime::monodroid_pinvoke_override (const char *library_name, const ch return nullptr; } - if (XA_UNLIKELY (utils.should_log (LOG_TIMING))) { - lookup_time.mark_end (); - total_time.mark_end (); - - TIMING_LOG_INFO (lookup_time, "p/invoke cache lookup for '%s' (internal)", entrypoint_name); - TIMING_LOG_INFO (total_time, "p/invoke override for '%s' (internal)", entrypoint_name); - } - log_warn (LOG_DEFAULT, "Found %s@%s in internal p/invoke map (%p)", library_name, entrypoint_name, iter->second); return iter->second; } diff --git a/src/monodroid/jni/strings.hh b/src/monodroid/jni/strings.hh index 175589da163..b3e019f096d 100644 --- a/src/monodroid/jni/strings.hh +++ b/src/monodroid/jni/strings.hh @@ -14,15 +14,15 @@ namespace xamarin::android::internal { - constexpr size_t SENSIBLE_TYPE_NAME_LENGTH = 128; - constexpr size_t SENSIBLE_PATH_MAX = 256; + static constexpr size_t SENSIBLE_TYPE_NAME_LENGTH = 128; + static constexpr size_t SENSIBLE_PATH_MAX = 256; // 64-bit unsigned or 64-bit signed with sign - constexpr size_t MAX_INTEGER_DIGIT_COUNT_BASE10 = 21; + static constexpr size_t MAX_INTEGER_DIGIT_COUNT_BASE10 = 21; #if DEBUG - constexpr bool BoundsCheck = true; + static constexpr bool BoundsCheck = true; #else - constexpr bool BoundsCheck = false; + static constexpr bool BoundsCheck = false; #endif class string_segment @@ -100,7 +100,7 @@ namespace xamarin::android::internal template force_inline bool starts_with (const char (&s)[Size]) const noexcept { - return starts_with (s, Size); + return starts_with (s, Size - 1); } force_inline bool has_at (const char ch, size_t index) const noexcept @@ -424,13 +424,19 @@ namespace xamarin::android::internal return *this; } + template + force_inline string_base& append (internal::string_base const& str) noexcept + { + return append (str.get (), str.length ()); + } + template - force_inline string_base& append (const char (&s)[Size]) const noexcept + force_inline string_base& append (const char (&s)[Size]) noexcept { - return append (s, Size); + return append (s, Size - 1); } - force_inline string_base& append (const char *s) noexcept + force_inline string_base& append_c (const char *s) noexcept { if (s == nullptr) return *this; @@ -486,7 +492,7 @@ namespace xamarin::android::internal return append (s, length); } - force_inline string_base& assign (const TChar* s) noexcept + force_inline string_base& assign_c (const TChar* s) noexcept { if (s == nullptr) return *this; @@ -497,7 +503,13 @@ namespace xamarin::android::internal template force_inline string_base& assign (const char (&s)[Size]) const noexcept { - return assign (s, Size); + return assign (s, Size - 1); + } + + template + force_inline string_base& assign (internal::string_base const& str) noexcept + { + return assign (str.get (), str.length ()); } force_inline string_base& assign (const TChar* s, size_t offset, size_t count) noexcept @@ -568,7 +580,7 @@ namespace xamarin::android::internal return -1; } - force_inline bool starts_with (const TChar *s, size_t s_length) noexcept + force_inline bool starts_with (const TChar *s, size_t s_length) const noexcept { if (s == nullptr || s_length == 0 || s_length > buffer.size ()) return false; @@ -576,7 +588,7 @@ namespace xamarin::android::internal return memcmp (buffer.get (), s, s_length) == 0; } - force_inline bool starts_with (const char* s) const noexcept + force_inline bool starts_with_c (const char* s) noexcept { if (s == nullptr) return false; @@ -584,6 +596,12 @@ namespace xamarin::android::internal return starts_with (s, strlen (s)); } + template + force_inline bool starts_with (const char (&s)[Size]) noexcept + { + return starts_with (s, Size - 1); + } + force_inline void set_length_after_direct_write (size_t new_length) noexcept { set_length (new_length); @@ -604,7 +622,7 @@ namespace xamarin::android::internal force_inline const TChar get_at (size_t index) const noexcept { ensure_valid_index (index); - return buffer.get () + index; + return *(buffer.get () + index); } force_inline TChar& get_at (size_t index) noexcept diff --git a/src/monodroid/jni/xamarin-app.hh b/src/monodroid/jni/xamarin-app.hh index 6b5cb9ba41e..fe034ccb4bc 100644 --- a/src/monodroid/jni/xamarin-app.hh +++ b/src/monodroid/jni/xamarin-app.hh @@ -110,6 +110,7 @@ struct ApplicationConfig uint32_t package_naming_policy; uint32_t environment_variable_count; uint32_t system_property_count; + uint32_t number_of_assemblies_in_apk; const char *android_package_name; };