[CoreCLR] Code cleanup and filling-in gaps (#10148)

This commit performs a comprehensive code cleanup by converting function 
parameter types and log messages to use `std::string_view` literal (`sv`) 
syntax and by removing unused or redundant parameters. 

`std::string_view` is used to avoid potential conversions between `char*` 
and `char[]`  code and make it faster, at points.

It also implements a handful of `p/invokes` which haven't had implementation
so far. One notable exception here is `_monodroid_gref_log_delete` which, when
enabled, causes a managed exception towards the end of `Mono.Android_Tests`
run:

    FATAL EXCEPTION: Instr: xamarin.android.runtimetests.NUnitInstrumentation
    Process: Mono.Android.NET_Tests, PID: 16194
      android.runtime.JavaProxyThrowable: [System.ObjectDisposedException]: Cannot access disposed object with JniIdentityHashCode=166175665.
      Object name: 'Xamarin.Android.RuntimeTests.NUnitInstrumentation'.
             at Java.Interop.JniPeerMembers.AssertSelf + 0x2f(Unknown Source)
             at Java.Interop.JniPeerMembers+JniInstanceMethods.InvokeVirtualVoidMethod + 0x1(Unknown Source)
             at Android.App.Instrumentation.Finish + 0x46(Unknown Source)
             at Xamarin.Android.UnitTests.TestInstrumentation`1.OnStart + 0x6d(Unknown Source)
             at Android.App.Instrumentation.n_OnStart + 0x1f(Unknown Source)
             at crc643df67da7b13bb6b1.TestInstrumentation_1.n_onStart(Native Method)
             at crc643df67da7b13bb6b1.TestInstrumentation_1.onStart(TestInstrumentation_1.java:32)
             at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:2606)

The above issue likely stems from the imbalance in calls to `_monodroid_gref_log_new` and
`_monodroid_gref_log_delete` in `Mono.Android.dll` or `Java.Interop.dll`. It will be
investigated and fixed in a separate PR.

Key changes include:

   * Updating function signatures to replace `const char*` with `std::string_view` 
     and removal of unused parameters.
   * Converting logging format strings throughout the codebase to use the new sv literal.
   * Minor refactoring in host and assembly-related source files to improve code consistency.
   * Add implementation of a handful of missing internal `p/invokes`

Author: grendello

src/native/clr/host/assembly-store.cc

@@ -21,15 +21,15 @@ void AssemblyStore::set_assembly_data_and_size (uint8_t* source_assembly_data, u
 }
 
 [[gnu::always_inline]]
-auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData const& e, std::string_view const& name, bool force_rw) noexcept -> std::tuple<uint8_t*, uint32_t>
+auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData const& e, std::string_view const& name) noexcept -> std::tuple<uint8_t*, uint32_t>
 {
 	uint8_t *assembly_data = nullptr;
 	uint32_t assembly_data_size = 0;
 
 #if defined (HAVE_LZ4) && defined (RELEASE)
 	auto header = reinterpret_cast<const CompressedAssemblyHeader*>(e.image_data);
 	if (header->magic == COMPRESSED_DATA_MAGIC) {
-		log_debug (LOG_ASSEMBLY, "Decompressing assembly '{}' from the assembly store", name);
+		log_debug (LOG_ASSEMBLY, "Decompressing assembly '{}' from the assembly store"sv, name);
 
 		if (FastTiming::enabled ()) [[unlikely]] {
 			internal_timing.start_event (TimingEventKind::AssemblyDecompression);
@@ -42,7 +42,7 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co
 			Helpers::abort_application (
 				LOG_ASSEMBLY,
 				std::format (
-					"Invalid compressed assembly descriptor index {}",
+					"Invalid compressed assembly descriptor index {}"sv,
 					header->descriptor_index
 				)
 			);
@@ -71,7 +71,7 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co
 				Helpers::abort_application (
 					LOG_ASSEMBLY,
 					std::format (
-						"Invalid compressed assembly descriptor at {}: no data",
+						"Invalid compressed assembly descriptor at {}: no data"sv,
 						header->descriptor_index
 					)
 				);
@@ -82,14 +82,14 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co
 					Helpers::abort_application (
 						LOG_ASSEMBLY,
 						std::format (
-							"Compressed assembly '{}' is larger than when the application was built (expected at most {}, got {}). Assemblies don't grow just like that!",
+							"Compressed assembly '{}' is larger than when the application was built (expected at most {}, got {}). Assemblies don't grow just like that!"sv,
 							name,
 							cad.uncompressed_file_size,
 							header->uncompressed_length
 						)
 					);
 				} else {
-					log_debug (LOG_ASSEMBLY, "Compressed assembly '{}' is smaller than when the application was built. Adjusting accordingly.", name);
+					log_debug (LOG_ASSEMBLY, "Compressed assembly '{}' is smaller than when the application was built. Adjusting accordingly."sv, name);
 				}
 				cad.uncompressed_file_size = header->uncompressed_length;
 			}
@@ -101,7 +101,7 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co
 				Helpers::abort_application (
 					LOG_ASSEMBLY,
 					std::format (
-						"Decompression of assembly {} failed with code {}",
+						"Decompression of assembly {} failed with code {}"sv,
 						name,
 						ret
 					)
@@ -112,7 +112,7 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co
 				Helpers::abort_application (
 					LOG_ASSEMBLY,
 					std::format (
-						"Decompression of assembly {} yielded a different size (expected {}, got {})",
+						"Decompression of assembly {} yielded a different size (expected {}, got {})"sv,
 						name,
 						cad.uncompressed_file_size,
 						static_cast<uint32_t>(ret)
@@ -130,12 +130,12 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co
 	} else
 #endif // def HAVE_LZ4 && def RELEASE
 	{
-		log_debug (LOG_ASSEMBLY, "Assembly '{}' is not compressed in the assembly store", name);
+		log_debug (LOG_ASSEMBLY, "Assembly '{}' is not compressed in the assembly store"sv, name);
 
 		// HACK! START
 		// Currently, MAUI crashes when we return a pointer to read-only data, so we must copy
 		// the assembly data to a read-write area.
-		log_debug (LOG_ASSEMBLY, "Copying assembly data to an r/w memory area");
+		log_debug (LOG_ASSEMBLY, "Copying assembly data to an r/w memory area"sv);
 
 		if (FastTiming::enabled ()) [[unlikely]] {
 			internal_timing.start_event (TimingEventKind::AssemblyLoad);
@@ -176,29 +176,29 @@ auto AssemblyStore::find_assembly_store_entry (hash_t hash, const AssemblyStoreI
 auto AssemblyStore::open_assembly (std::string_view const& name, int64_t &size) noexcept -> void*
 {
 	hash_t name_hash = xxhash::hash (name.data (), name.length ());
-	log_debug (LOG_ASSEMBLY, "AssemblyStore::open_assembly: looking for bundled name: '{}' (hash {:x})", optional_string (name.data ()), name_hash);
+	log_debug (LOG_ASSEMBLY, "AssemblyStore::open_assembly: looking for bundled name: '{}' (hash {:x})"sv, optional_string (name.data ()), name_hash);
 
 	if constexpr (Constants::is_debug_build) {
 		// TODO: implement filesystem lookup here
 
 		// In fastdev mode we might not have any assembly store.
 		if (assembly_store_hashes == nullptr) {
-			log_warn (LOG_ASSEMBLY, "Assembly store not registered. Unable to look up assembly '{}'", name);
+			log_warn (LOG_ASSEMBLY, "Assembly store not registered. Unable to look up assembly '{}'"sv, name);
 			return nullptr;
 		}
 	}
 
 	const AssemblyStoreIndexEntry *hash_entry = find_assembly_store_entry (name_hash, assembly_store_hashes, assembly_store.index_entry_count);
 	if (hash_entry == nullptr) {
-		log_warn (LOG_ASSEMBLY, "Assembly '{}' (hash 0x{:x}) not found", optional_string (name.data ()), name_hash);
+		log_warn (LOG_ASSEMBLY, "Assembly '{}' (hash 0x{:x}) not found"sv, optional_string (name.data ()), name_hash);
 		return nullptr;
 	}
 
 	if (hash_entry->descriptor_index >= assembly_store.assembly_count) {
 		Helpers::abort_application (
 			LOG_ASSEMBLY,
 			std::format (
-				"Invalid assembly descriptor index {}, exceeds the maximum value of {}",
+				"Invalid assembly descriptor index {}, exceeds the maximum value of {}"sv,
 				hash_entry->descriptor_index,
 				assembly_store.assembly_count - 1
 			)
@@ -219,7 +219,7 @@ auto AssemblyStore::open_assembly (std::string_view const& name, int64_t &size)
 
 		log_debug (
 			LOG_ASSEMBLY,
-			"Mapped: image_data == {:p}; debug_info_data == {:p}; config_data == {:p}; descriptor == {:p}; data size == {}; debug data size == {}; config data size == {}; name == '{}'",
+			"Mapped: image_data == {:p}; debug_info_data == {:p}; config_data == {:p}; descriptor == {:p}; data size == {}; debug data size == {}; config data size == {}; name == '{}'"sv,
 			static_cast<void*>(assembly_runtime_info.image_data),
 			static_cast<void*>(assembly_runtime_info.debug_info_data),
 			static_cast<void*>(assembly_runtime_info.config_data),
@@ -231,8 +231,7 @@ auto AssemblyStore::open_assembly (std::string_view const& name, int64_t &size)
 		);
 	}
 
-	constexpr hash_t mscorlib_hash = 0x579a06fed6eec900;
-	auto [assembly_data, assembly_data_size] = get_assembly_data (assembly_runtime_info, name, name_hash == mscorlib_hash);
+	auto [assembly_data, assembly_data_size] = get_assembly_data (assembly_runtime_info, name);
 	size = assembly_data_size;
 	return assembly_data;
 }
@@ -242,7 +241,7 @@ void AssemblyStore::map (int fd, std::string_view const& apk_path, std::string_v
 	detail::mmap_info assembly_store_map = Util::mmap_file (fd, offset, size, store_path);
 
 	auto [payload_start, payload_size] = Util::get_wrapper_dso_payload_pointer_and_size (assembly_store_map, store_path);
-	log_debug (LOG_ASSEMBLY, "Adjusted assembly store pointer: {:p}; size: {}", payload_start, payload_size);
+	log_debug (LOG_ASSEMBLY, "Adjusted assembly store pointer: {:p}; size: {}"sv, payload_start, payload_size);
 	auto header = static_cast<AssemblyStoreHeader*>(payload_start);
 
 	auto get_full_store_path = [&apk_path, &store_path]() -> std::string {
@@ -264,7 +263,7 @@ void AssemblyStore::map (int fd, std::string_view const& apk_path, std::string_v
 		Helpers::abort_application (
 			LOG_ASSEMBLY,
 			std::format (
-				"Assembly store '{}' is not a valid .NET for Android assembly store file",
+				"Assembly store '{}' is not a valid .NET for Android assembly store file"sv,
 				get_full_store_path ()
 			)
 		);
@@ -274,7 +273,7 @@ void AssemblyStore::map (int fd, std::string_view const& apk_path, std::string_v
 		Helpers::abort_application (
 			LOG_ASSEMBLY,
 			std::format (
-				"Assembly store '{}' uses format version {:x}, instead of the expected {:x}",
+				"Assembly store '{}' uses format version {:x}, instead of the expected {:x}"sv,
 				get_full_store_path (),
 				header->version,
 				ASSEMBLY_STORE_FORMAT_VERSION
@@ -290,5 +289,5 @@ void AssemblyStore::map (int fd, std::string_view const& apk_path, std::string_v
 	assembly_store.assemblies = reinterpret_cast<AssemblyStoreEntryDescriptor*>(assembly_store.data_start + header_size + header->index_size);
 	assembly_store_hashes = reinterpret_cast<AssemblyStoreIndexEntry*>(assembly_store.data_start + header_size);
 
-	log_debug (LOG_ASSEMBLY, "Mapped assembly store {}", get_full_store_path ());
+	log_debug (LOG_ASSEMBLY, "Mapped assembly store {}"sv, get_full_store_path ());
 }

src/native/clr/host/fastdev-assemblies.cc

@@ -20,7 +20,7 @@ auto FastDevAssemblies::open_assembly (std::string_view const& name, int64_t &si
 
 	std::string const& override_dir_path = AndroidSystem::get_primary_override_dir ();
 	if (!Util::dir_exists (override_dir_path)) [[unlikely]] {
-		log_debug (LOG_ASSEMBLY, "Override directory '{}' does not exist", override_dir_path);
+		log_debug (LOG_ASSEMBLY, "Override directory '{}' does not exist"sv, override_dir_path);
 		return nullptr;
 	}
 
@@ -31,7 +31,7 @@ auto FastDevAssemblies::open_assembly (std::string_view const& name, int64_t &si
 		if (override_dir_fd < 0) [[likely]] {
 			override_dir = opendir (override_dir_path.c_str ());
 			if (override_dir == nullptr) [[unlikely]] {
-				log_warn (LOG_ASSEMBLY, "Failed to open override dir '{}'. {}", override_dir_path, strerror (errno));
+				log_warn (LOG_ASSEMBLY, "Failed to open override dir '{}'. {}"sv, override_dir_path, strerror (errno));
 				return nullptr;
 			}
 			override_dir_fd = dirfd (override_dir);
@@ -40,20 +40,20 @@ auto FastDevAssemblies::open_assembly (std::string_view const& name, int64_t &si
 
 	log_debug (
 		LOG_ASSEMBLY,
-		"Attempting to load FastDev assembly '{}' from override directory '{}'",
+		"Attempting to load FastDev assembly '{}' from override directory '{}'"sv,
 		name,
 		override_dir_path
 	);
 
 	if (!Util::file_exists (override_dir_fd, name)) {
-		log_warn (LOG_ASSEMBLY, "FastDev assembly '{}' not found.", name);
+		log_warn (LOG_ASSEMBLY, "FastDev assembly '{}' not found."sv, name);
 		return nullptr;
 	}
-	log_debug (LOG_ASSEMBLY, "Found FastDev assembly '{}'", name);
+	log_debug (LOG_ASSEMBLY, "Found FastDev assembly '{}'"sv, name);
 
 	auto file_size = Util::get_file_size_at (override_dir_fd, name);
 	if (!file_size) [[unlikely]] {
-		log_warn (LOG_ASSEMBLY, "Unable to determine FastDev assembly '{}' file size", name);
+		log_warn (LOG_ASSEMBLY, "Unable to determine FastDev assembly '{}' file size"sv, name);
 		return nullptr;
 	}
 
@@ -62,7 +62,7 @@ auto FastDevAssemblies::open_assembly (std::string_view const& name, int64_t &si
 		Helpers::abort_application (
 			LOG_ASSEMBLY,
 			std::format (
-				"FastDev assembly '{}' size exceeds the maximum supported value of {}",
+				"FastDev assembly '{}' size exceeds the maximum supported value of {}"sv,
 				name,
 				MAX_SIZE
 			)
@@ -74,7 +74,7 @@ auto FastDevAssemblies::open_assembly (std::string_view const& name, int64_t &si
 	if (asm_fd < 0) {
 		log_warn (
 			LOG_ASSEMBLY,
-			"Failed to open FastDev assembly '{}' for reading. {}",
+			"Failed to open FastDev assembly '{}' for reading. {}"sv,
 			name,
 			strerror (errno)
 		);
@@ -99,15 +99,15 @@ auto FastDevAssemblies::open_assembly (std::string_view const& name, int64_t &si
 
 		log_warn (
 			LOG_ASSEMBLY,
-			"Failed to read FastDev assembly '{}' data. {}",
+			"Failed to read FastDev assembly '{}' data. {}"sv,
 			name,
 			strerror (errno)
 		);
 
 		size = 0;
 		return nullptr;
 	}
-	log_debug (LOG_ASSEMBLY, "Read {} bytes of FastDev assembly '{}'", nread, name);
+	log_debug (LOG_ASSEMBLY, "Read {} bytes of FastDev assembly '{}'"sv, nread, name);
 
 	return reinterpret_cast<void*>(buffer);
 }

src/native/clr/host/host-jni.cc

@@ -49,11 +49,13 @@ Java_mono_android_Runtime_initInternal (JNIEnv *env, jclass klass, jstring lang,
 }
 
 JNIEXPORT void
-JNICALL Java_mono_android_Runtime_propagateUncaughtException (JNIEnv *env, [[maybe_unused]] jclass klass, jobject javaThread, jthrowable javaException)
+JNICALL Java_mono_android_Runtime_propagateUncaughtException ([[maybe_unused]] JNIEnv *env, [[maybe_unused]] jclass klass, [[maybe_unused]] jobject javaThread, [[maybe_unused]] jthrowable javaException)
 {
+	// TODO: implement or remove
 }
 
 JNIEXPORT void
 JNICALL Java_mono_android_Runtime_notifyTimeZoneChanged ([[maybe_unused]] JNIEnv *env, [[maybe_unused]] jclass klass)
 {
+	// TODO: implement or remove
 }