diff --git a/src/native/clr/host/assembly-store.cc b/src/native/clr/host/assembly-store.cc index 12ef3884d57..1639da5ef96 100644 --- a/src/native/clr/host/assembly-store.cc +++ b/src/native/clr/host/assembly-store.cc @@ -296,11 +296,12 @@ namespace { return; } { - dynamic_local_property_string prop_value; - if (AndroidSystem::monodroid_get_system_property ("debug.net.asmcache"sv, prop_value) > 0 && prop_value.get () != nullptr) { - if (prop_value.get ()[0] == '0') { + char prop_value[Constants::PROPERTY_VALUE_BUFFER_LEN]; + const char *cache_prop = AndroidSystem::monodroid_get_system_property ("debug.net.asmcache", prop_value, sizeof (prop_value)); + if (cache_prop != nullptr) { + if (cache_prop [0] == '0') { cache_requested = false; - } else if (prop_value.get ()[0] == '1') { + } else if (cache_prop [0] == '1') { cache_requested = true; } } diff --git a/src/native/clr/host/bridge-processing.cc b/src/native/clr/host/bridge-processing.cc index b4daa4b6c6d..92def2312bd 100644 --- a/src/native/clr/host/bridge-processing.cc +++ b/src/native/clr/host/bridge-processing.cc @@ -1,5 +1,6 @@ #include #include +#include #include #include diff --git a/src/native/clr/host/host.cc b/src/native/clr/host/host.cc index 1b06742d790..8f6b339bbcd 100644 --- a/src/native/clr/host/host.cc +++ b/src/native/clr/host/host.cc @@ -333,7 +333,7 @@ void Host::Java_mono_android_Runtime_initInternal ( AndroidSystem::set_app_code_cache_dir (applicationDirs[Constants::APP_DIRS_CODE_CACHE_DIR_INDEX]); AndroidSystem::create_update_dir (AndroidSystem::get_primary_override_dir ()); AndroidSystem::setup_environment (); - Logger::init_reference_logging (AndroidSystem::get_primary_override_dir ()); + Logger::init_reference_logging (AndroidSystem::get_primary_override_dir ().c_str ()); jstring_array_wrapper runtimeApks (env, runtimeApksJava); AndroidSystem::setup_app_library_directories (runtimeApks, applicationDirs, haveSplitApks); diff --git a/src/native/clr/host/typemap.cc b/src/native/clr/host/typemap.cc index 71748a13410..43c91d024c3 100644 --- a/src/native/clr/host/typemap.cc +++ b/src/native/clr/host/typemap.cc @@ -1,6 +1,9 @@ #include #include +#include +#include #include +#include #include #include @@ -18,6 +21,24 @@ namespace { return value_length == key_length && strncmp (value, key, key_length) == 0; } +#if defined (DEBUG) + // Returns the length of the formatted name, or the negative capacity the caller must provide + // when `buffer` is too small. + auto format_managed_type_name (const char *type_name, const char *assembly_name, char *buffer, size_t buffer_size) noexcept -> ssize_t + { + int full_name_length = snprintf (buffer, buffer_size, "%s, %s", type_name, assembly_name); + abort_unless (full_name_length >= 0, "Failed to format the managed type name"); + + size_t length = static_cast(full_name_length); + abort_unless (length < static_cast(std::numeric_limits::max ()), "Managed type name is too long"); + if (length >= buffer_size) { + return -static_cast(length + 1uz); + } + + return static_cast(length); + } +#endif // def DEBUG + class MonoGuidString { static inline constexpr size_t MVID_SIZE = 16; @@ -152,14 +173,22 @@ auto TypeMapper::index_to_name (ssize_t idx, const char* typeName, const TypeMap [[gnu::always_inline, gnu::flatten]] auto TypeMapper::managed_to_java_debug (const char *typeName, const char *assemblyFullName) noexcept -> const char* { - dynamic_local_path_string full_type_name; - full_type_name.append (typeName); - full_type_name.append (", "sv); - full_type_name.append (assemblyFullName); + char stack_buffer [Constants::SENSIBLE_PATH_MAX]; + char *full_type_name = Util::format_with_retry ( + stack_buffer, + sizeof (stack_buffer), + [typeName, assemblyFullName](char *buffer, size_t buffer_size) noexcept { + return format_managed_type_name (typeName, assemblyFullName, buffer, buffer_size); + } + ); - ssize_t idx = find_index_by_hash (full_type_name.get (), type_map.managed_to_java, type_map_managed_type_names, MANAGED, JAVA); + ssize_t idx = find_index_by_hash (full_type_name, type_map.managed_to_java, type_map_managed_type_names, MANAGED, JAVA); + const char *mapped_name = index_to_name (idx, full_type_name, type_map.managed_to_java, type_map_java_type_names, MANAGED, JAVA); + if (full_type_name != stack_buffer) { + std::free (full_type_name); + } - return index_to_name (idx, full_type_name.get (), type_map.managed_to_java, type_map_java_type_names, MANAGED, JAVA); + return mapped_name; } #endif // def DEBUG diff --git a/src/native/clr/include/host/host-environment.hh b/src/native/clr/include/host/host-environment.hh index 5d50fb46959..575ac02f017 100644 --- a/src/native/clr/include/host/host-environment.hh +++ b/src/native/clr/include/host/host-environment.hh @@ -52,10 +52,11 @@ namespace xamarin::android { } [[gnu::flatten, gnu::always_inline]] - static auto lookup_system_property (std::string_view const& name, size_t &value_len, + static auto lookup_system_property (const char *name, size_t &value_len, uint32_t const count, AppEnvironmentVariable const (&entries)[], const char (&contents)[]) noexcept -> const char* { + value_len = 0; if (count == 0) { return nullptr; } @@ -63,7 +64,7 @@ namespace xamarin::android { for (size_t i = 0; i < count; i++) { AppEnvironmentVariable const& sys_prop = entries[i]; const char *prop_name = &contents[sys_prop.name_index]; - if (name.compare (prop_name) != 0) { + if (strcmp (name, prop_name) != 0) { continue; } diff --git a/src/native/clr/include/host/os-bridge.hh b/src/native/clr/include/host/os-bridge.hh index 20768eaa897..138933af2ef 100644 --- a/src/native/clr/include/host/os-bridge.hh +++ b/src/native/clr/include/host/os-bridge.hh @@ -6,6 +6,8 @@ #include +#include + #include "../runtime-base/logger.hh" namespace xamarin::android { diff --git a/src/native/clr/include/runtime-base/android-system.hh b/src/native/clr/include/runtime-base/android-system.hh index 8a13669e9cc..57ac8d6bbdd 100644 --- a/src/native/clr/include/runtime-base/android-system.hh +++ b/src/native/clr/include/runtime-base/android-system.hh @@ -1,7 +1,7 @@ #pragma once #include -#include +#include #include #include #include @@ -79,7 +79,8 @@ namespace xamarin::android { static void set_primary_override_dir (jstring_wrapper& home) noexcept { #if defined (XA_HOST_NATIVEAOT) - determine_primary_override_dir (home, primary_override_dir, sizeof (primary_override_dir)); + ssize_t result = format_primary_override_dir (home, primary_override_dir, sizeof (primary_override_dir)); + abort_unless (result >= 0, "Primary override directory path is too long"); #else primary_override_dir = determine_primary_override_dir (home); #endif @@ -111,14 +112,14 @@ namespace xamarin::android { * However, if any logging is enabled (which should _not_ happen with * pre-loaded apps!), we need the .__override__ directory... */ - dynamic_local_property_string value; - if (log_categories == 0 && monodroid_get_system_property (Constants::DEBUG_MONO_PROFILE_PROPERTY, value) == 0) [[likely]] { + char value[Constants::PROPERTY_VALUE_BUFFER_LEN]; + if (log_categories == 0 && monodroid_get_system_property (Constants::DEBUG_MONO_PROFILE_PROPERTY.data (), value, sizeof (value)) == nullptr) [[likely]] { return; } } log_debug (LOG_DEFAULT, "Creating public update directory: `{}`", override_dir); - Util::create_public_directory (override_dir); + Util::create_public_directory (override_dir.c_str ()); } #endif @@ -127,26 +128,19 @@ namespace xamarin::android { return embedded_dso_mode_enabled; } - static auto monodroid_get_system_property (std::string_view const& name, dynamic_local_property_string &value) noexcept -> int; - - template - static auto monodroid_get_system_property (std::string_view const& name, char (&value)[Size]) noexcept -> int - { - dynamic_local_property_string property_value; - int result = monodroid_get_system_property (name, property_value); - if (result > 0) { - if (property_value.length () >= Size) { - value [0] = '\0'; - return -1; - } - memcpy (value, property_value.get (), property_value.length ()); - value [property_value.length ()] = '\0'; - } else { - value [0] = '\0'; - } - return result; - } - + // Returns the property's NUL-terminated value, or `nullptr` if it is not set. A property + // that is set to an empty value is reported as not set, matching `__system_property_get`, + // which cannot tell the two apart. + // + // `value` is a scratch buffer of at least `Constants::PROPERTY_VALUE_BUFFER_LEN` bytes, used + // to receive Android system properties. Bundled properties are returned without copying, so + // their length is not limited by `value_size`. + // + // Nothing is ever allocated and the caller must not free the result: it is either `value` or + // a pointer to bundled property data. The latter is only guaranteed to stay valid until the + // next `setup_environment()` call, because in Debug builds bundled properties are stored in + // a mutable map. Copy the value if you need to retain it. + static auto monodroid_get_system_property (const char *name, char *value, size_t value_size) noexcept -> const char*; static void detect_embedded_dso_mode (jstring_array_wrapper& appDirs) noexcept; static void setup_environment () noexcept; static void setup_app_library_directories (jstring_array_wrapper& runtimeApks, jstring_array_wrapper& appDirs, bool have_split_apks) noexcept; @@ -159,8 +153,8 @@ namespace xamarin::android { static auto load_dso_from_specified_dirs (TContainer directories, std::string_view const& dso_name, int dl_flags, bool is_jni) noexcept -> void*; static auto load_dso_from_app_lib_dirs (std::string_view const& name, int dl_flags, bool is_jni) noexcept -> void*; static auto load_dso_from_override_dirs (std::string_view const& name, int dl_flags, bool is_jni) noexcept -> void*; - static auto lookup_system_property (std::string_view const &name, size_t &value_len) noexcept -> const char*; - static auto monodroid__system_property_get (std::string_view const&, char *sp_value, size_t sp_value_len) noexcept -> int; + static auto lookup_system_property (const char *name, size_t &value_len) noexcept -> const char*; + static auto monodroid__system_property_get (const char *name, char *sp_value) noexcept -> int; static auto get_max_gref_count_from_system () noexcept -> long; static void add_apk_libdir (std::string_view const& apk, size_t &index, std::string_view const& abi) noexcept; static void setup_apk_directories (unsigned short running_on_cpu, jstring_array_wrapper &runtimeApks, bool have_split_apks) noexcept; @@ -175,28 +169,53 @@ namespace xamarin::android { embedded_dso_mode_enabled = yesno; } -#if defined (XA_HOST_NATIVEAOT) - static void determine_primary_override_dir (jstring_wrapper &home, char *buffer, size_t buffer_size) noexcept - { - dynamic_local_string name { home.get_cstr () }; - name.append ("/") - .append (Constants::OVERRIDE_DIRECTORY_NAME) - .append ("/") - .append (Constants::android_lib_abi); - - abort_unless (name.length () < buffer_size, "Primary override directory path is too long"); - memcpy (buffer, name.get (), name.length () + 1); + static auto format_primary_override_dir (jstring_wrapper &home, char *buffer, size_t buffer_size) noexcept -> ssize_t + { + abort_unless (buffer != nullptr, "Primary override directory buffer must not be null"); + + // `jstring_wrapper::get_cstr()` returns `nullptr` for a null `jstring`, and passing that + // to `%s` is undefined behaviour. An app without a files directory cannot work anyway. + const char *home_path = home.get_cstr (); + abort_unless (home_path != nullptr, "Application home directory must not be null"); + + int length = snprintf ( + buffer, + buffer_size, + "%s/%.*s/%.*s", + home_path, + static_cast(Constants::OVERRIDE_DIRECTORY_NAME.length ()), + Constants::OVERRIDE_DIRECTORY_NAME.data (), + static_cast(Constants::android_lib_abi.length ()), + Constants::android_lib_abi.data () + ); + abort_unless (length >= 0, "Failed to format primary override directory path"); + size_t required_capacity = Helpers::add_with_overflow_check (static_cast(length), 1uz); + abort_unless (required_capacity <= static_cast(std::numeric_limits::max ()), "Primary override directory path is too long"); + if (buffer_size < required_capacity) { + return -static_cast(required_capacity); + } + return static_cast(length); } -#else + +#if !defined (XA_HOST_NATIVEAOT) static auto determine_primary_override_dir (jstring_wrapper &home) noexcept -> std::string { - dynamic_local_string name { home.get_cstr () }; - name.append ("/") - .append (Constants::OVERRIDE_DIRECTORY_NAME) - .append ("/") - .append (Constants::android_lib_abi); - - return {name.get (), name.length ()}; + char stack_buffer [Constants::SENSIBLE_PATH_MAX]; + size_t length; + char *name = Util::format_with_retry ( + stack_buffer, + sizeof (stack_buffer), + [&home](char *buffer, size_t buffer_size) noexcept { + return format_primary_override_dir (home, buffer, buffer_size); + }, + &length + ); + + std::string path { name, length }; + if (name != stack_buffer) { + std::free (name); + } + return path; } #endif diff --git a/src/native/clr/include/runtime-base/logger.hh b/src/native/clr/include/runtime-base/logger.hh index 0c6121a6bde..4d7020aee0d 100644 --- a/src/native/clr/include/runtime-base/logger.hh +++ b/src/native/clr/include/runtime-base/logger.hh @@ -2,17 +2,14 @@ #include -#include - #include -#include namespace xamarin::android { class Logger { public: static void init_logging_categories () noexcept; - static void init_reference_logging (std::string_view const& override_dir) noexcept; + static void init_reference_logging (const char *override_dir) noexcept; static auto log_timing_categories () noexcept -> LogTimingCategories { @@ -50,11 +47,11 @@ namespace xamarin::android { } private: - static auto open_file (std::string_view const& path) noexcept -> FILE*; - static auto open_file (LogCategories category, std::string_view const& custom_path, std::string_view const& override_dir, std::string_view const& fallback_filename) noexcept -> FILE*; + static auto open_file (const char *path) noexcept -> FILE*; + static auto open_file (LogCategories category, const char *custom_path, const char *override_dir, const char *fallback_filename) noexcept -> FILE*; private: - static bool set_category (std::string_view const& name, string_segment& arg, unsigned int entry, bool arg_starts_with_name = false) noexcept; + static bool set_category (const char *name, const char *arg, size_t arg_length, unsigned int entry, bool arg_starts_with_name = false) noexcept; private: static inline LogTimingCategories _log_timing_categories; diff --git a/src/native/clr/include/runtime-base/util.hh b/src/native/clr/include/runtime-base/util.hh index ea6280418db..6d7472aaf07 100644 --- a/src/native/clr/include/runtime-base/util.hh +++ b/src/native/clr/include/runtime-base/util.hh @@ -48,9 +48,9 @@ namespace xamarin::android { return create_directory (dir.data (), mode); } - static void create_public_directory (std::string_view const& dir); - static auto monodroid_fopen (std::string_view const& filename, std::string_view const& mode) noexcept -> FILE*; - static void set_world_accessable (std::string_view const& path); + static void create_public_directory (const char *dir); + static auto monodroid_fopen (const char *filename, const char *mode) noexcept -> FILE*; + static void set_world_accessable (const char *path); static auto set_world_accessible (int fd) noexcept -> bool; // Puts higher half of the `value` byte as a hexadecimal character in `high_half` and @@ -344,19 +344,46 @@ namespace xamarin::android { return static_cast(path_length); } - static auto join_paths (char *stack_buffer, size_t stack_buffer_size, std::string_view first, std::string_view second) noexcept -> char* + // Formats a string that usually fits in a stack buffer, falling back to the heap when it + // does not. `formatter` must write into the buffer it is given and return the formatted + // length excluding the terminating NUL, or the negative required capacity including it — + // the protocol implemented by `format_joined_path()` and friends. + // + // Returns `stack_buffer`, or a heap block the caller must `std::free ()`. Compare the + // result against `stack_buffer` to tell the two apart. When `length` is not `nullptr`, it + // receives the formatted length excluding the terminating NUL. + template + static auto format_with_retry (char *stack_buffer, size_t stack_buffer_size, TFormatter formatter, size_t *length = nullptr) noexcept -> char* { - ssize_t result = format_joined_path (stack_buffer, stack_buffer_size, first, second); - if (result >= 0) { - return stack_buffer; + ssize_t result = formatter (stack_buffer, stack_buffer_size); + if (result < 0) { + size_t required_capacity = static_cast(-result); + char *heap_buffer = static_cast (std::malloc (required_capacity)); + abort_unless (heap_buffer != nullptr, "Failed to allocate formatted string"); + + result = formatter (heap_buffer, required_capacity); + abort_unless (result >= 0, "Failed to format string using the required capacity"); + if (length != nullptr) { + *length = static_cast(result); + } + return heap_buffer; } - size_t required_capacity = static_cast(-result); - char *heap_buffer = static_cast (std::malloc (required_capacity)); - abort_unless (heap_buffer != nullptr, "Failed to allocate joined path"); - result = format_joined_path (heap_buffer, required_capacity, first, second); - abort_unless (result >= 0, "Failed to join path using the required capacity"); - return heap_buffer; + if (length != nullptr) { + *length = static_cast(result); + } + return stack_buffer; + } + + static auto join_paths (char *stack_buffer, size_t stack_buffer_size, std::string_view first, std::string_view second) noexcept -> char* + { + return format_with_retry ( + stack_buffer, + stack_buffer_size, + [first, second](char *buffer, size_t buffer_size) noexcept { + return format_joined_path (buffer, buffer_size, first, second); + } + ); } private: diff --git a/src/native/clr/runtime-base/android-system-shared.cc b/src/native/clr/runtime-base/android-system-shared.cc index 7eeaa4fc7c6..88c455a47b2 100644 --- a/src/native/clr/runtime-base/android-system-shared.cc +++ b/src/native/clr/runtime-base/android-system-shared.cc @@ -1,53 +1,50 @@ #include -#include #include using namespace xamarin::android; -using std::operator""sv; - -auto AndroidSystem::monodroid_get_system_property (std::string_view const& name, dynamic_local_property_string &value) noexcept -> int +auto AndroidSystem::monodroid_get_system_property (const char *name, char *value, size_t value_size) noexcept -> const char* { - int len = monodroid__system_property_get (name, value.get (), value.size ()); - if (len > 0) { - // Clumsy, but if we want direct writes to be fast, this is the price we pay - value.set_length_after_direct_write (static_cast(len)); - return len; + // `__system_property_get` always writes up to `PROPERTY_VALUE_BUFFER_LEN` bytes, so a smaller + // buffer would overflow. This is a programming error, not a runtime condition. + abort_unless ( + value != nullptr && value_size >= Constants::PROPERTY_VALUE_BUFFER_LEN, + "System property value buffer is too small" + ); + + value [0] = '\0'; + + // `__system_property_get` NUL-terminates what it writes. + if (monodroid__system_property_get (name, value) > 0) { + return value; } - size_t plen; - const char *v = lookup_system_property (name, plen); - if (v == nullptr) { - return len; + // Bundled properties are NUL-terminated strings owned by the application, so return them + // directly rather than copying them into `value`. Their length is therefore not limited by + // `Constants::PROPERTY_VALUE_BUFFER_LEN`. See the header for the exact lifetime guarantee. + // + // A bundled property may be present but empty. `__system_property_get` cannot distinguish an + // empty value from a missing one either, so report both as unset and let callers keep their + // defaults. + size_t property_length; + const char *bundled_value = lookup_system_property (name, property_length); + if (bundled_value == nullptr || property_length == 0) { + return nullptr; } - value.assign (v, plen); - return Helpers::add_with_overflow_check (plen, 0); + return bundled_value; } auto -AndroidSystem::monodroid__system_property_get (std::string_view const& name, char *sp_value, size_t sp_value_len) noexcept -> int +AndroidSystem::monodroid__system_property_get (const char *name, char *sp_value) noexcept -> int { - if (name.empty () || sp_value == nullptr) { + if (name == nullptr || *name == '\0' || sp_value == nullptr) { return -1; } - char *buf = nullptr; - if (sp_value_len < Constants::PROPERTY_VALUE_BUFFER_LEN) { - size_t alloc_size = Helpers::add_with_overflow_check (Constants::PROPERTY_VALUE_BUFFER_LEN, 1uz); - log_warnf (LOG_DEFAULT, "Buffer to store system property may be too small, will copy only %zu bytes", sp_value_len); - buf = new char [alloc_size]; - } - - int len = __system_property_get (name.data (), buf ? buf : sp_value); - if (buf != nullptr) { - strncpy (sp_value, buf, sp_value_len); - sp_value [sp_value_len] = '\0'; - delete[] buf; - } - - return len; + // The caller guarantees that `sp_value` is at least `PROPERTY_VALUE_BUFFER_LEN` bytes long. + return __system_property_get (name, sp_value); } auto @@ -61,10 +58,11 @@ AndroidSystem::get_max_gref_count_from_system () noexcept -> long max = 51200; } - dynamic_local_property_string override; - if (monodroid_get_system_property (Constants::DEBUG_MONO_MAX_GREFC, override) > 0) { + char override[Constants::PROPERTY_VALUE_BUFFER_LEN]; + const char *grefc = monodroid_get_system_property (Constants::DEBUG_MONO_MAX_GREFC.data (), override, sizeof (override)); + if (grefc != nullptr) { char *e; - max = strtol (override.get (), &e, 10); + max = strtol (grefc, &e, 10); switch (*e) { case 'k': e++; @@ -83,10 +81,9 @@ AndroidSystem::get_max_gref_count_from_system () noexcept -> long if (*e) { log_warnf ( LOG_GC, - "Unsupported '%.*s' value '%s'.", - static_cast(Constants::DEBUG_MONO_MAX_GREFC.length ()), + "Unsupported '%s' value '%s'.", Constants::DEBUG_MONO_MAX_GREFC.data (), - override.get () + grefc ); } diff --git a/src/native/clr/runtime-base/android-system.cc b/src/native/clr/runtime-base/android-system.cc index bafb0276289..b8ce6fb56c4 100644 --- a/src/native/clr/runtime-base/android-system.cc +++ b/src/native/clr/runtime-base/android-system.cc @@ -294,15 +294,15 @@ AndroidSystem::detect_embedded_dso_mode (jstring_array_wrapper& appDirs) noexcep } auto -AndroidSystem::lookup_system_property (std::string_view const& name, size_t &value_len) noexcept -> const char* +AndroidSystem::lookup_system_property (const char *name, size_t &value_len) noexcept -> const char* { value_len = 0; #if defined (DEBUG) if (!bundled_properties.empty ()) { - auto prop_iter = bundled_properties.find (name.data ()); + auto prop_iter = bundled_properties.find (name); if (prop_iter != bundled_properties.end ()) { value_len = prop_iter->second.length (); - return prop_iter->first.c_str (); + return prop_iter->second.c_str (); } } #endif // DEBUG diff --git a/src/native/clr/runtime-base/logger.cc b/src/native/clr/runtime-base/logger.cc index c00d23e6cad..5f476d25c59 100644 --- a/src/native/clr/runtime-base/logger.cc +++ b/src/native/clr/runtime-base/logger.cc @@ -17,7 +17,6 @@ #include using namespace xamarin::android; -using std::operator""sv; namespace { char *gref_file = nullptr; @@ -25,16 +24,27 @@ namespace { bool light_gref = false; bool light_lref = false; - void set_log_file (char *&log_file, std::string_view path) noexcept + // Compares a comma-separated parameter, which is not NUL-terminated, against `text`. + bool param_matches (const char *param, size_t param_length, const char *text, bool prefix_only = false) noexcept + { + size_t text_length = strlen (text); + if (prefix_only ? param_length < text_length : param_length != text_length) { + return false; + } + + return strncmp (param, text, text_length) == 0; + } + + void set_log_file (char *&log_file, const char *path, size_t path_length) noexcept { char *new_log_file = nullptr; - if (!path.empty ()) { - size_t allocation_size = Helpers::add_with_overflow_check (path.length (), 1uz); + if (path != nullptr && path_length > 0) { + size_t allocation_size = Helpers::add_with_overflow_check (path_length, 1uz); new_log_file = static_cast (std::malloc (allocation_size)); abort_unless (new_log_file != nullptr, "Failed to allocate reference log file path"); - memcpy (new_log_file, path.data (), path.length ()); - new_log_file [path.length ()] = '\0'; + memcpy (new_log_file, path, path_length); + new_log_file [path_length] = '\0'; } std::free (log_file); @@ -43,17 +53,17 @@ namespace { } [[gnu::always_inline]] -auto Logger::open_file (std::string_view const& path) noexcept -> FILE* +auto Logger::open_file (const char *path) noexcept -> FILE* { - if (path.empty ()) { + if (path == nullptr || *path == '\0') { return nullptr; } // Ignore errors, by design - unlink (path.data ()); + unlink (path); // `monodroid_fopen` will log any errors - FILE *ret = Util::monodroid_fopen (path, "a"sv); + FILE *ret = Util::monodroid_fopen (path, "a"); if (ret != nullptr) { Util::set_world_accessable (path); } @@ -62,11 +72,11 @@ auto Logger::open_file (std::string_view const& path) noexcept -> FILE* } [[gnu::flatten, gnu::always_inline]] -auto Logger::open_file (LogCategories category, std::string_view const& custom_path, std::string_view const& override_dir, std::string_view const& fallback_filename) noexcept -> FILE* +auto Logger::open_file (LogCategories category, const char *custom_path, const char *override_dir, const char *fallback_filename) noexcept -> FILE* { - auto log_and_return = [&category](FILE *f, std::string_view const& path) -> FILE* { + auto log_and_return = [&category](FILE *f, const char *path) -> FILE* { if (f != nullptr) { - log_debugf (category, "Opened file '%.*s' for logging.", static_cast(path.length ()), path.data ()); + log_debugf (category, "Opened file '%s' for logging.", path); } return f; }; @@ -75,12 +85,16 @@ auto Logger::open_file (LogCategories category, std::string_view const& custom_p if (ret != nullptr) { return log_and_return (ret, custom_path); } + + if (override_dir == nullptr || *override_dir == '\0') { + return nullptr; + } + Util::create_public_directory (override_dir); char stack_buffer [Util::LocalPathBufferSize]; char *path_buffer = Util::join_paths (stack_buffer, sizeof (stack_buffer), override_dir, fallback_filename); - std::string_view path_view { path_buffer }; - ret = log_and_return (open_file (path_view), path_view); + ret = log_and_return (open_file (path_buffer), path_buffer); if (path_buffer != stack_buffer) { std::free (path_buffer); } @@ -88,15 +102,10 @@ auto Logger::open_file (LogCategories category, std::string_view const& custom_p } void -Logger::init_reference_logging (std::string_view const& override_dir) noexcept +Logger::init_reference_logging (const char *override_dir) noexcept { if ((log_categories & LOG_GREF) != 0 && !light_gref) { - _gref_log = open_file ( - LOG_GREF, - gref_file == nullptr ? std::string_view {} : std::string_view { gref_file }, - override_dir, - "grefs.txt"sv - ); + _gref_log = open_file (LOG_GREF, gref_file, override_dir, "grefs.txt"); } if ((log_categories & LOG_LREF) != 0 && !light_lref) { @@ -104,12 +113,7 @@ Logger::init_reference_logging (std::string_view const& override_dir) noexcept if (lref_file != nullptr && strcmp (lref_file, gref_file != nullptr ? gref_file : "") == 0) { _lref_log = _gref_log; } else { - _lref_log = open_file ( - LOG_LREF, - lref_file == nullptr ? std::string_view {} : std::string_view { lref_file }, - override_dir, - "lrefs.txt"sv - ); + _lref_log = open_file (LOG_LREF, lref_file, override_dir, "lrefs.txt"); } } @@ -120,13 +124,13 @@ Logger::init_reference_logging (std::string_view const& override_dir) noexcept } [[gnu::always_inline]] bool -Logger::set_category (std::string_view const& name, string_segment& arg, unsigned int entry, bool arg_starts_with_name) noexcept +Logger::set_category (const char *name, const char *arg, size_t arg_length, unsigned int entry, bool arg_starts_with_name) noexcept { if ((log_categories & entry) == entry) { return false; } - if (arg_starts_with_name ? arg.starts_with (name) : arg.equal (name)) { + if (param_matches (arg, arg_length, name, arg_starts_with_name)) { log_categories |= entry; return true; } @@ -139,116 +143,98 @@ Logger::init_logging_categories () noexcept { _log_timing_categories = LogTimingCategories::Default; - dynamic_local_property_string value; - if (AndroidSystem::monodroid_get_system_property (Constants::DEBUG_MONO_LOG_PROPERTY, value) == 0) { + char value[Constants::PROPERTY_VALUE_BUFFER_LEN]; + const char *categories = AndroidSystem::monodroid_get_system_property (Constants::DEBUG_MONO_LOG_PROPERTY.data (), value, sizeof (value)); + if (categories == nullptr) { return; } - string_segment param; - while (value.next_token (',', param)) { - constexpr std::string_view CAT_ALL { "all" }; + // The value may point at immortal bundled property data, so the parameters cannot be + // NUL-terminated in place. Bound every comparison by `param_length` instead. + const char *param = categories; + while (param != nullptr && *param != '\0') { + const char *separator = strchr (param, ','); + size_t param_length = separator != nullptr ? static_cast(separator - param) : strlen (param); + const char *next = separator == nullptr ? nullptr : separator + 1; - if (param.equal (CAT_ALL)) { + if (param_matches (param, param_length, "all")) { log_categories = 0xFFFFFFFF; break; } - if (set_category ("assembly", param, LOG_ASSEMBLY)) { - continue; - } - - if (set_category ("default", param, LOG_DEFAULT)) { - continue; - } - - if (set_category ("debugger", param, LOG_DEBUGGER)) { - continue; - } - - if (set_category ("gc", param, LOG_GC)) { + if (set_category ("assembly", param, param_length, LOG_ASSEMBLY) || + set_category ("default", param, param_length, LOG_DEFAULT) || + set_category ("debugger", param, param_length, LOG_DEBUGGER) || + set_category ("gc", param, param_length, LOG_GC) || + set_category ("gref", param, param_length, LOG_GREF) || + set_category ("lref", param, param_length, LOG_LREF) || + set_category ("timing", param, param_length, LOG_TIMING) || + set_category ("network", param, param_length, LOG_NET) || + set_category ("netlink", param, param_length, LOG_NETLINK)) { + param = next; continue; } - if (set_category ("gref", param, LOG_GREF)) { - continue; - } - - if (set_category ("lref", param, LOG_LREF)) { - continue; - } - - if (set_category ("timing", param, LOG_TIMING)) { - continue; - } - - if (set_category ("network", param, LOG_NET)) { - continue; - } - - if (set_category ("netlink", param, LOG_NETLINK)) { - continue; - } - - auto get_log_file_name = [](std::string_view const& file_kind, string_segment const& segment, size_t offset) -> std::string_view { - auto file_name = segment.at (offset); - - if (!file_name.has_value ()) { - log_warnf ( - LOG_DEFAULT, - "Unable to set path to %.*s log file: %s", - static_cast(file_kind.length ()), - file_kind.data (), - to_string (file_name.error ()) - ); - return {}; + auto set_log_file_from_param = [param, param_length](char *&log_file, const char *file_kind) { + constexpr size_t OFFSET = sizeof ("gref=") - 1; // Both the "gref=" and "lref=" prefixes are this long. + if (OFFSET >= param_length) { + log_warnf (LOG_DEFAULT, "Unable to set path to %s log file: no file name specified", file_kind); + set_log_file (log_file, nullptr, 0uz); + return; } - return { file_name.value (), segment.length () - offset }; + set_log_file (log_file, param + OFFSET, param_length - OFFSET); }; - constexpr std::string_view CAT_GREF_EQUALS { "gref=" }; - if (set_category (CAT_GREF_EQUALS, param, LOG_GREF, true /* arg_starts_with_name */)) { - set_log_file (gref_file, get_log_file_name ("gref"sv, param, CAT_GREF_EQUALS.length ())); + if (set_category ("gref=", param, param_length, LOG_GREF, true /* arg_starts_with_name */)) { + set_log_file_from_param (gref_file, "gref"); + param = next; continue; } - if (set_category ("gref-", param, LOG_GREF)) { + if (set_category ("gref-", param, param_length, LOG_GREF)) { light_gref = true; + param = next; continue; } - if (set_category ("gref+", param, LOG_GREF)) { + if (set_category ("gref+", param, param_length, LOG_GREF)) { _gref_to_logcat = true; + param = next; continue; } - constexpr std::string_view CAT_LREF_EQUALS { "lref=" }; - if (set_category (CAT_LREF_EQUALS, param, LOG_LREF, true /* arg_starts_with_name */)) { - set_log_file (lref_file, get_log_file_name ("lref"sv, param, CAT_LREF_EQUALS.length ())); + if (set_category ("lref=", param, param_length, LOG_LREF, true /* arg_starts_with_name */)) { + set_log_file_from_param (lref_file, "lref"); + param = next; continue; } - if (set_category ("lref-", param, LOG_LREF)) { + if (set_category ("lref-", param, param_length, LOG_LREF)) { light_lref = true; + param = next; continue; } - if (set_category ("lref+", param, LOG_LREF)) { + if (set_category ("lref+", param, param_length, LOG_LREF)) { _lref_to_logcat = true; + param = next; continue; } - if (param.starts_with ("timing=fast-bare")) { + if (param_matches (param, param_length, "timing=fast-bare", true /* prefix_only */)) { log_categories |= LOG_TIMING; _log_timing_categories |= LogTimingCategories::FastBare; + param = next; continue; } - if (param.starts_with ("timing=bare")) { + if (param_matches (param, param_length, "timing=bare", true /* prefix_only */)) { log_categories |= LOG_TIMING; _log_timing_categories |= LogTimingCategories::Bare; - continue; } + + param = next; } if ((log_categories & LOG_GC) != 0) { diff --git a/src/native/clr/runtime-base/util.cc b/src/native/clr/runtime-base/util.cc index afb034eaba5..df5028fe807 100644 --- a/src/native/clr/runtime-base/util.cc +++ b/src/native/clr/runtime-base/util.cc @@ -4,6 +4,7 @@ #include #include +#include #include using namespace xamarin::android; @@ -49,63 +50,45 @@ Util::create_directory (const char *pathname, mode_t mode) } void -Util::create_public_directory (std::string_view const& dir) +Util::create_public_directory (const char *dir) { mode_t m = umask (0); - int ret = create_directory (dir.data (), 0777); + int ret = create_directory (dir, 0777); if (ret < 0) { if (errno == EEXIST) { // Try to change the mode, just in case - chmod (dir.data (), 0777); + chmod (dir, 0777); } else { - log_warnf ( - LOG_DEFAULT, - "Failed to create directory '%.*s'. %s", - static_cast(dir.length ()), - dir.data (), - std::strerror (errno) - ); + log_warnf (LOG_DEFAULT, "Failed to create directory '%s'. %s", dir, std::strerror (errno)); } } umask (m); } auto -Util::monodroid_fopen (std::string_view const& filename, std::string_view const& mode) noexcept -> FILE* +Util::monodroid_fopen (const char *filename, const char *mode) noexcept -> FILE* { /* On Unix, both path and system calls are all assumed * to be UTF-8 compliant. */ - FILE *ret = fopen (filename.data (), mode.data ()); + FILE *ret = fopen (filename, mode); if (ret == nullptr) { - log_errorf ( - LOG_DEFAULT, - "fopen failed for file %.*s: %s", - static_cast(filename.length ()), - filename.data (), - strerror (errno) - ); + log_errorf (LOG_DEFAULT, "fopen failed for file %s: %s", filename, strerror (errno)); return nullptr; } return ret; } -void Util::set_world_accessable (std::string_view const& path) +void Util::set_world_accessable (const char *path) { int r; do { - r = chmod (path.data (), 0664); + r = chmod (path, 0664); } while (r == -1 && errno == EINTR); if (r == -1) { - log_errorf ( - LOG_DEFAULT, - "chmod(\"%.*s\", 0664) failed: %s", - static_cast(path.length ()), - path.data (), - strerror (errno) - ); + log_errorf (LOG_DEFAULT, "chmod(\"%s\", 0664) failed: %s", path, strerror (errno)); } } diff --git a/src/native/common/include/runtime-base/timing-internal.hh b/src/native/common/include/runtime-base/timing-internal.hh index 310f8cb6fb4..fe0a3ee3dfe 100644 --- a/src/native/common/include/runtime-base/timing-internal.hh +++ b/src/native/common/include/runtime-base/timing-internal.hh @@ -487,7 +487,7 @@ namespace xamarin::android { } private: - void parse_options (char *value) noexcept; + void parse_options (const char *options) noexcept; static void really_initialize (bool log_immediately) noexcept; [[gnu::always_inline]] diff --git a/src/native/common/runtime-base/timing-internal.cc b/src/native/common/runtime-base/timing-internal.cc index e399e1ed17a..2d6799b9936 100644 --- a/src/native/common/runtime-base/timing-internal.cc +++ b/src/native/common/runtime-base/timing-internal.cc @@ -30,9 +30,9 @@ void FastTiming::really_initialize (bool log_immediately) noexcept } char value [Constants::PROPERTY_VALUE_BUFFER_LEN]; - int value_length = AndroidSystem::monodroid_get_system_property (Constants::DEBUG_MONO_TIMING, value); - if (value_length > 0) { - internal_timing.parse_options (value); + const char *options = AndroidSystem::monodroid_get_system_property (Constants::DEBUG_MONO_TIMING.data (), value, sizeof (value)); + if (options != nullptr) { + internal_timing.parse_options (options); } log_write ( @@ -42,26 +42,26 @@ void FastTiming::really_initialize (bool log_immediately) noexcept ); } -void FastTiming::parse_options (char *value) noexcept +void FastTiming::parse_options (const char *options) noexcept { - char *param = value; + const char *param = options; while (param != nullptr && *param != '\0') { - char *separator = strchr (param, ','); - if (separator != nullptr) { - *separator = '\0'; - } + // The value may point at immortal bundled property data, so the parameters cannot be + // NUL-terminated in place. Bound every comparison by `param_length` instead. + const char *separator = strchr (param, ','); + size_t param_length = separator != nullptr ? static_cast(separator - param) : strlen (param); - if (strcmp (param, OPT_TO_FILE.data ()) == 0) { + if (param_length == OPT_TO_FILE.length () && strncmp (param, OPT_TO_FILE.data (), param_length) == 0) { log_to_file = true; - } else if (strncmp (param, OPT_FILE_NAME.data (), OPT_FILE_NAME.length ()) == 0) { - output_file_name = std::make_unique (param + OPT_FILE_NAME.length ()); - } else if (strncmp (param, OPT_DURATION.data (), OPT_DURATION.length ()) == 0) { + } else if (param_length >= OPT_FILE_NAME.length () && strncmp (param, OPT_FILE_NAME.data (), OPT_FILE_NAME.length ()) == 0) { + output_file_name = std::make_unique (param + OPT_FILE_NAME.length (), param_length - OPT_FILE_NAME.length ()); + } else if (param_length >= OPT_DURATION.length () && strncmp (param, OPT_DURATION.data (), OPT_DURATION.length ()) == 0) { const char *duration = param + OPT_DURATION.length (); char *end; errno = 0; unsigned long long parsed_duration = strtoull (duration, &end, 10); - if (end == duration || *end != '\0' || errno == ERANGE || parsed_duration > std::numeric_limits::max ()) { - log_warn (LOG_TIMING, "Failed to parse duration in milliseconds from '%s'"sv, param); + if (end == duration || end != param + param_length || errno == ERANGE || parsed_duration > std::numeric_limits::max ()) { + log_warnf (LOG_TIMING, "Failed to parse duration in milliseconds from '%.*s'", static_cast(param_length), param); duration_ms = default_duration_milliseconds; } else { duration_ms = static_cast(parsed_duration); diff --git a/src/native/mono/runtime-base/android-system.cc b/src/native/mono/runtime-base/android-system.cc index 94a5d39a63d..a7994c94878 100644 --- a/src/native/mono/runtime-base/android-system.cc +++ b/src/native/mono/runtime-base/android-system.cc @@ -58,7 +58,7 @@ AndroidSystem::lookup_system_property (const char *name, size_t &value_len) noex BundledProperty *p = lookup_system_property (name); if (p != nullptr) { value_len = p->value_len; - return p->name; + return p->value; } #endif // DEBUG || !ANDROID @@ -172,6 +172,39 @@ AndroidSystem::monodroid_get_system_property (const char *name, dynamic_local_st return Helpers::add_with_overflow_check (plen, 0); } +const char* +AndroidSystem::monodroid_get_system_property (const char *name, char *value, size_t value_size) noexcept +{ + // `__system_property_get` always writes up to `PROPERTY_VALUE_BUFFER_LEN` bytes, so a smaller + // buffer would overflow. This is a programming error, not a runtime condition. + abort_unless ( + value != nullptr && value_size >= PROPERTY_VALUE_BUFFER_LEN, + "System property value buffer is too small" + ); + + value [0] = '\0'; + + // `__system_property_get` NUL-terminates what it writes. + if (_monodroid__system_property_get (name, value, value_size) > 0) { + return value; + } + + // Bundled properties are NUL-terminated strings owned by the application, so return them + // directly rather than copying them into `value`. Their length is therefore not limited by + // `PROPERTY_VALUE_BUFFER_LEN`. See the header for the exact lifetime guarantee. + // + // A bundled property may be present but empty. `__system_property_get` cannot distinguish an + // empty value from a missing one either, so report both as unset and let callers keep their + // defaults. + size_t property_length; + const char *bundled_value = lookup_system_property (name, property_length); + if (bundled_value == nullptr || property_length == 0) { + return nullptr; + } + + return bundled_value; +} + int AndroidSystem::monodroid_get_system_property (const char *name, char **value) noexcept { diff --git a/src/native/mono/runtime-base/android-system.hh b/src/native/mono/runtime-base/android-system.hh index 1a39e3c7cf0..eec1e99fff0 100644 --- a/src/native/mono/runtime-base/android-system.hh +++ b/src/native/mono/runtime-base/android-system.hh @@ -95,6 +95,19 @@ namespace xamarin::android::internal { static void create_update_dir (char *override_dir) noexcept; static int monodroid_get_system_property (const char *name, char **value) noexcept; static int monodroid_get_system_property (const char *name, dynamic_local_string &value) noexcept; + // Returns the property's NUL-terminated value, or `nullptr` if it is not set. A property + // that is set to an empty value is reported as not set, matching `__system_property_get`, + // which cannot tell the two apart. + // + // `value` is a scratch buffer of at least `PROPERTY_VALUE_BUFFER_LEN` bytes, used to receive + // Android system properties. Bundled properties are returned without copying, so their + // length is not limited by `value_size`. + // + // Nothing is ever allocated and the caller must not free the result: it is either `value` or + // a pointer to bundled property data. The latter is only guaranteed to stay valid until the + // next `add_system_property()` call, which frees the value it replaces. Copy the value if + // you need to retain it. + static const char* monodroid_get_system_property (const char *name, char *value, size_t value_size) noexcept; static int monodroid_get_system_property (std::string_view const& name, char **value) noexcept { diff --git a/src/native/nativeaot/runtime-base/android-system.cc b/src/native/nativeaot/runtime-base/android-system.cc index 52e19c153bc..f9ff09d3671 100644 --- a/src/native/nativeaot/runtime-base/android-system.cc +++ b/src/native/nativeaot/runtime-base/android-system.cc @@ -6,7 +6,7 @@ using namespace xamarin::android; -auto AndroidSystem::lookup_system_property (std::string_view const& name, size_t &value_len) noexcept -> const char* +auto AndroidSystem::lookup_system_property (const char *name, size_t &value_len) noexcept -> const char* { return HostEnvironment::lookup_system_property ( name,