A handful of fixes and a lot of changes

All globals are gone, classes that were instantiated are now fully
static.

Author: grendello

src/monodroid/CMakeLists.txt

@@ -332,7 +332,6 @@ endif()
 set(LINK_LIBS "")
 
 set(LOCAL_COMMON_COMPILER_ARGS
-  -fno-threadsafe-statics
   -Wall
   -Wconversion
   -Wdeprecated
@@ -360,6 +359,9 @@ endif()
 
 set(LOCAL_COMMON_LINKER_ARGS "")
 if(ANDROID)
+  # We don't need thread-safe initialization of statics on Android, our entire init sequence runs in a single thread
+  list(APPEND LOCAL_COMMON_COMPILER_ARGS -fno-threadsafe-statics)
+
   if (ENABLE_CLANG_ASAN OR ENABLE_CLANG_UBSAN)
     list(APPEND LOCAL_COMMON_COMPILER_ARGS
       -fno-omit-frame-pointer

src/monodroid/clang-tidy.yaml

@@ -1,5 +1,5 @@
 ---
-Checks:          'clang-diagnostic-*,clang-analyzer-*,cppcoreguidelines-*,-cppcoreguidelines-pro-type-vararg,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-avoid-c-arrays,android-*,bugprone-*,-bugprone-easily-swappable-parameters,performance-*,-bugprone-dynamic-static-initializers,modernize-*,-modernize-use-trailing-return-type,-modernize-avoid-c-arrays,readability-*'
+Checks:          'clang-diagnostic-*,clang-analyzer-*,cppcoreguidelines-*,-cppcoreguidelines-pro-type-vararg,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-avoid-c-arrays,android-*,bugprone-*,-bugprone-easily-swappable-parameters,performance-*,-bugprone-dynamic-static-initializers,modernize-*,-modernize-use-trailing-return-type,-modernize-avoid-c-arrays,readability-*,-readability-braces-around-statements,-readability-identifier-length'
 WarningsAsErrors: ''
 HeaderFilterRegex: ''
 AnalyzeTemporaryDtors: false

src/monodroid/jni/android-system.cc

@@ -16,8 +16,10 @@
 #include <shlwapi.h>
 #endif
 
-#include "globals.hh"
+#include <mono/metadata/object.h>
+
 #include "android-system.hh"
+#include "debug.hh"
 #include "monodroid.h"
 #include "monodroid-glue-internal.hh"
 #include "jni-wrappers.hh"
@@ -163,7 +165,7 @@ AndroidSystem::_monodroid__system_property_get (const char *name, char *sp_value
 	if (!name || !sp_value)
 		return -1;
 
-	char *env_name = utils.monodroid_strdup_printf ("__XA_%s", name);
+	char *env_name = Util::monodroid_strdup_printf ("__XA_%s", name);
 	monodroid_strreplace (env_name, '.', '_');
 	char *env_value = getenv (env_name);
 	free (env_name);
@@ -243,7 +245,7 @@ AndroidSystem::monodroid_get_system_property_impl (const char *name, dynamic_loc
 int
 AndroidSystem::monodroid_get_system_property (const char *name, gsl::owner<char**> value) noexcept
 {
-	if (value)
+	if (value != nullptr)
 		*value = nullptr;
 
 	dynamic_local_string<PROPERTY_VALUE_BUFFER_LEN> sp_value;
@@ -259,7 +261,7 @@ AndroidSystem::monodroid_get_system_property (const char *name, gsl::owner<char*
 		}
 	}
 
-	if (len >= 0 && value) {
+	if (len >= 0 && value != nullptr) {
 		auto alloc_size = ADD_WITH_OVERFLOW_CHECK (size_t, static_cast<size_t>(len), 1);
 		*value = new char [alloc_size];
 		if (*value == nullptr)
@@ -285,7 +287,7 @@ AndroidSystem::_monodroid_get_system_property_from_file (const char *path, char
 	if (value != nullptr)
 		*value = nullptr;
 
-	FILE* fp = utils.monodroid_fopen (path, "r");
+	FILE* fp = Util::monodroid_fopen (path, "r");
 	if (fp == nullptr)
 		return 0;
 
@@ -325,7 +327,7 @@ AndroidSystem::monodroid_get_system_property_from_overrides ([[maybe_unused]] co
 			continue;
 		}
 
-		std::unique_ptr<char[]> override_file {utils.path_combine (dir, name)};
+		std::unique_ptr<char[]> override_file {Util::path_combine (dir, 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.get (), value);
@@ -350,13 +352,13 @@ AndroidSystem::create_update_dir (char *override_dir) noexcept
 	 * However, if any logging is enabled (which should _not_ happen with
 	 * pre-loaded apps!), we need the .__override__ directory...
 	 */
-	if (log_categories == 0 && monodroid_system_property_exists (Debug::DEBUG_MONO_PROFILE_PROPERTY) == 0) {
+	if (log_categories == 0 && monodroid_system_property_exists (Debug::DEBUG_MONO_PROFILE_PROPERTY)) {
 		return;
 	}
 #endif
 
 	set_override_dir (0, override_dir);
-	utils.create_public_directory (override_dir);
+	Util::create_public_directory (override_dir);
 	log_warn (LOG_DEFAULT, "Creating public update directory: `%s`", override_dir);
 }
 
@@ -367,7 +369,7 @@ AndroidSystem::get_full_dso_path (const char *base_dir, const char *dso_path, dy
 		return false;
 	}
 
-	if (base_dir == nullptr || utils.is_path_rooted (dso_path)) {
+	if (base_dir == nullptr || Util::is_path_rooted (dso_path)) {
 		path.assign_c (dso_path); // Absolute path or no base path, can't do much with it
 		return true;
 	}
@@ -386,14 +388,14 @@ AndroidSystem::load_dso (const char *path, unsigned int dl_flags, bool skip_exis
 		return nullptr;
 
 	log_info (LOG_ASSEMBLY, "Trying to load shared library '%s'", path);
-	if (!skip_exists_check && !is_embedded_dso_mode_enabled () && !utils.file_exists (path)) {
+	if (!skip_exists_check && !is_embedded_dso_mode_enabled () && !Util::file_exists (path)) {
 		log_info (LOG_ASSEMBLY, "Shared library '%s' not found", path);
 		return nullptr;
 	}
 
 	char *error = nullptr;
 	void *handle = java_interop_lib_load (path, dl_flags, &error);
-	if (handle == nullptr && utils.should_log (LOG_ASSEMBLY))
+	if (handle == nullptr && Util::should_log (LOG_ASSEMBLY))
 		log_info_nocheck (LOG_ASSEMBLY, "Failed to load shared library '%s'. %s", path, error);
 	java_interop_free (error);
 	return handle;
@@ -438,10 +440,7 @@ AndroidSystem::load_dso_from_any_directories (const char *name, unsigned int dl_
 bool
 AndroidSystem::get_existing_dso_path_on_disk (const char *base_dir, const char *dso_name, dynamic_local_string<SENSIBLE_PATH_MAX>& path) noexcept
 {
-	if (get_full_dso_path (base_dir, dso_name, path) && utils.file_exists (path.get ()))
-		return true;
-
-	return false;
+	return get_full_dso_path (base_dir, dso_name, path) && Util::file_exists (path.get ());
 }
 
 bool
@@ -474,19 +473,19 @@ AndroidSystem::count_override_assemblies () noexcept
 	int c = 0;
 
 	for (auto const dir_path : override_dirs ()) {
-		if (dir_path == nullptr || !utils.directory_exists (dir_path))
+		if (dir_path == nullptr || !Util::directory_exists (dir_path))
 			continue;
 
-		monodroid_dir_t *dir = utils.monodroid_opendir (dir_path);
+		monodroid_dir_t *dir = Util::monodroid_opendir (dir_path);
 		if (dir == nullptr)
 			continue;
 
 		monodroid_dirent_t *e = nullptr;
 		while ((e = readdir (dir)) != nullptr && e) {
-			if (utils.monodroid_dirent_hasextension (e, ".dll"))
+			if (Util::monodroid_dirent_hasextension (e, ".dll"))
 				++c;
 		}
-		utils.monodroid_closedir (dir);
+		Util::monodroid_closedir (dir);
 	}
 
 	return c;
@@ -565,7 +564,7 @@ AndroidSystem::setup_environment_from_override_file (const char *path) noexcept
 	using read_count_type = size_t;
 #endif
 	monodroid_stat_t sbuf;
-	if (utils.monodroid_stat (path, &sbuf) < 0) {
+	if (Util::monodroid_stat (path, &sbuf) < 0) {
 		log_warn (LOG_DEFAULT, "Failed to stat the environment override file %s: %s", path, strerror (errno));
 		return;
 	}
@@ -717,8 +716,8 @@ AndroidSystem::setup_environment () noexcept
 #if defined (DEBUG) || !defined (ANDROID)
 	// TODO: for debug read from file in the override directory named `environment`
 	for (const auto override_dir : override_dirs ()) {
-		std::unique_ptr<char[]> env_override_file {utils.path_combine (override_dir, OVERRIDE_ENVIRONMENT_FILE_NAME)};
-		if (utils.file_exists (env_override_file.get ())) {
+		std::unique_ptr<char[]> env_override_file {Util::path_combine (override_dir, OVERRIDE_ENVIRONMENT_FILE_NAME)};
+		if (Util::file_exists (env_override_file.get ())) {
 			setup_environment_from_override_file (env_override_file.get ());
 		}
 	}
@@ -783,15 +782,15 @@ AndroidSystem::get_libmonoandroid_directory_path () noexcept
 
 	GetModuleFileNameW (module, module_path, sizeof (module_path) / sizeof (module_path[0]));
 	PathRemoveFileSpecW (module_path);
-	libmonoandroid_directory_path = utils.utf16_to_utf8 (module_path);
+	libmonoandroid_directory_path = Util::utf16_to_utf8 (module_path);
 	return libmonoandroid_directory_path;
 }
 
 int
 AndroidSystem::setenv (const char *name, const char *value, [[maybe_unused]] int overwrite) noexcept
 {
-	wchar_t *wname  = utils.utf8_to_utf16 (name);
-	wchar_t *wvalue = utils.utf8_to_utf16 (value);
+	wchar_t *wname  = Util::utf8_to_utf16 (name);
+	wchar_t *wvalue = Util::utf8_to_utf16 (value);
 
 	BOOL result = SetEnvironmentVariableW (wname, wvalue);
 	free (wname);
@@ -803,7 +802,7 @@ AndroidSystem::setenv (const char *name, const char *value, [[maybe_unused]] int
 int
 AndroidSystem::symlink (const char *target, const char *linkpath) noexcept
 {
-	return utils.file_copy (target, linkpath);
+	return Util::file_copy (target, linkpath);
 }
 #else
 #endif

src/monodroid/jni/basic-android-system.cc

@@ -2,7 +2,8 @@
 
 #include "basic-android-system.hh"
 #include "cpp-util.hh"
-#include "globals.hh"
+#include "shared-constants.hh"
+#include "util.hh"
 
 using namespace xamarin::android;
 using namespace xamarin::android::internal;
@@ -11,9 +12,9 @@ void
 BasicAndroidSystem::detect_embedded_dso_mode (jstring_array_wrapper& appDirs) noexcept
 {
 	// appDirs[SharedConstants::APP_DIRS_DATA_DIR_INDEX] points to the native library directory
-	std::unique_ptr<char> libmonodroid_path {utils.path_combine (appDirs[SharedConstants::APP_DIRS_DATA_DIR_INDEX].get_cstr (), "libmonodroid.so")};
+	std::unique_ptr<char> libmonodroid_path {Util::path_combine (appDirs[SharedConstants::APP_DIRS_DATA_DIR_INDEX].get_cstr (), "libmonodroid.so")};
 	log_debug (LOG_ASSEMBLY, "Checking if libmonodroid was unpacked to %s", libmonodroid_path.get ());
-	if (!utils.file_exists (libmonodroid_path.get ())) {
+	if (!Util::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 {
@@ -27,7 +28,7 @@ BasicAndroidSystem::setup_app_library_directories (jstring_array_wrapper& runtim
 {
 	if (!is_embedded_dso_mode_enabled ()) {
 		log_info (LOG_DEFAULT, "Setting up for DSO lookup in app data directories");
-		app_lib_directories ().push_back (utils.strdup_new (appDirs[SharedConstants::APP_DIRS_DATA_DIR_INDEX].get_cstr ()));
+		app_lib_directories ().push_back (Util::strdup_new (appDirs[SharedConstants::APP_DIRS_DATA_DIR_INDEX].get_cstr ()));
 		log_debug (LOG_ASSEMBLY, "Added filesystem DSO lookup location: %s", appDirs[SharedConstants::APP_DIRS_DATA_DIR_INDEX].get_cstr ());
 	} else {
 		log_info (LOG_DEFAULT, "Setting up for DSO lookup directly in the APK");
@@ -53,7 +54,7 @@ BasicAndroidSystem::for_each_apk (jstring_array_wrapper &runtimeApks, ForEachApk
 force_inline void
 BasicAndroidSystem::add_apk_libdir (const char *apk, size_t &index, const char *abi) noexcept
 {
-	app_lib_directories ().push_back (utils.string_concat (apk, "!/lib/", abi));
+	app_lib_directories ().push_back (Util::string_concat (apk, "!/lib/", abi));
 	log_debug (LOG_ASSEMBLY, "Added APK DSO lookup location: %s", app_lib_directories ()[index]);
 	index++;
 }
@@ -69,7 +70,7 @@ BasicAndroidSystem::setup_apk_directories (unsigned short running_on_cpu, jstrin
 		const char *apk = e.get_cstr ();
 
 		if (have_split_apks) {
-			if (utils.ends_with (apk, SharedConstants::split_config_abi_apk_name)) {
+			if (Util::ends_with (apk, SharedConstants::split_config_abi_apk_name)) {
 				add_apk_libdir (apk, number_of_added_directories, abi);
 				break;
 			}
@@ -82,7 +83,7 @@ BasicAndroidSystem::setup_apk_directories (unsigned short running_on_cpu, jstrin
 gsl::owner<char*>
 BasicAndroidSystem::determine_primary_override_dir (jstring_wrapper &home) noexcept
 {
-	return utils.path_combine (home.get_cstr (), ".__override__");
+	return Util::path_combine (home.get_cstr (), ".__override__");
 }
 
 const char*

src/monodroid/jni/basic-utilities.cc

@@ -125,24 +125,6 @@ BasicUtilities::set_user_executable ([[maybe_unused]] const char *path) noexcept
 #endif
 }
 
-bool
-BasicUtilities::file_exists (const char *file) noexcept
-{
-	monodroid_stat_t s;
-	if (monodroid_stat (file, &s) == 0 && (s.st_mode & S_IFMT) == S_IFREG)
-		return true;
-	return false;
-}
-
-bool
-BasicUtilities::directory_exists (const char *directory) noexcept
-{
-	monodroid_stat_t s;
-	if (monodroid_stat (directory, &s) == 0 && (s.st_mode & S_IFMT) == S_IFDIR)
-		return true;
-	return false;
-}
-
 bool
 BasicUtilities::file_copy (const char *to, const char *from) noexcept
 {

src/monodroid/jni/basic-utilities.hh

@@ -39,10 +39,20 @@ namespace xamarin::android
 		static int                create_directory (const char *pathname, mode_t mode) noexcept;
 		static void               set_world_accessable (const char *path) noexcept;
 		static void               set_user_executable (const char *path) noexcept;
-		static bool               file_exists (const char *file) noexcept;
-		static bool               directory_exists (const char *directory) noexcept;
-		static bool               file_copy (const char *to, const char *from) noexcept;
 
+		static bool file_exists (const char *file) noexcept
+		{
+			monodroid_stat_t s;
+			return monodroid_stat (file, &s) == 0 && (s.st_mode & S_IFMT) == S_IFREG;
+		}
+
+		static bool directory_exists (const char *directory) noexcept
+		{
+			monodroid_stat_t s;
+			return monodroid_stat (directory, &s) == 0 && (s.st_mode & S_IFMT) == S_IFDIR;
+		}
+
+		static bool               file_copy (const char *to, const char *from) noexcept;
 
 		// Make sure that `buf` has enough space! This is by design, the methods are supposed to be fast.
 		template<size_t MaxStackSpace, typename TBuffer>