Skip to content

Commit 73f4925

Browse files
[native] Replace the AndroidSystem path statics with a POD buffer (#12552)
Part of #12533 (drop the `libc++` dependency), stacked on #12551. `AndroidSystem` kept five of its members in `std::string` / `std::array<std::string, 1>`: `primary_override_dir`, `native_libraries_dir`, `app_code_cache_dir`, `single_app_lib_directory` and `override_dirs`. Because they are `inline static` with dynamic initialization, the compiler emits a guard variable **and** an `atexit` registration for them in *every* translation unit that includes `android-system.hh` — even in ones that never touch them. `logger.cc`, `internal-pinvokes-clr.cc`, `internal-pinvokes-shared.cc` and `android-system-shared.cc` each paid four libc++ references (`~basic_string`, `operator delete`, `__cxa_guard_acquire`, `__cxa_guard_release`) without using a single one of these directories: ``` $ llvm-nm --undefined-only logger.cc.o | llvm-cxxfilt std::__ndk1::basic_string<...>::~basic_string() operator delete(void*) __cxa_guard_acquire __cxa_guard_release $ llvm-objdump -r logger.cc.o | grep _ZGV | llvm-cxxfilt guard variable for xamarin::android::AndroidSystem::override_dirs guard variable for xamarin::android::AndroidSystem::app_code_cache_dir guard variable for xamarin::android::AndroidSystem::native_libraries_dir guard variable for xamarin::android::AndroidSystem::primary_override_dir guard variable for xamarin::android::AndroidSystem::single_app_lib_directory ``` ## What changed All five become plain pointers. The three path members are `const char*` initialized to `""` and assigned once, early during startup, with a copy made by a new `Util::duplicate_string()` helper that aborts if the allocation fails. Pointers to a string literal are **constant-initialized**, so neither a guard variable nor an `atexit` registration is emitted. The two directory arrays become plain `const char*` arrays whose entries are `malloc`ed, which also drops an `operator new[]` from the non-split-APK path. Since there is no longer a fixed-size buffer anywhere, there is also no hard limit on the path length and no abort when it is exceeded — which is what NativeAOT's `char[SENSIBLE_PATH_MAX]` `primary_override_dir` used to do. That lets `primary_override_dir` be shared by all three hosts, removing three `#if defined (XA_HOST_NATIVEAOT)` blocks and `determine_primary_override_dir()` entirely. ## Results Undefined libc++ references in the three CoreCLR archives — **58 → 31**: | object | before | after | |---|---:|---:| | `assembly-store.cc.o` | 13 | 11 | | `host.cc.o` | 11 | 11 | | `android-system.cc.o` | 11 | 5 | | `timing-internal.cc.o` | 5 | 2 | | `logger.cc.o` | 4 | **0** | | `internal-pinvokes-shared.cc.o` | 4 | **0** | | `internal-pinvokes-clr.cc.o` | 4 | **0** | | `android-system-shared.cc.o` | 4 | **0** | | `typemap.cc.o` | 2 | 2 | Every `__cxa_guard_*` reference coming from this header is gone; the only ones left are `host.cc`'s own function-local statics. `libnet-android.release.so`: **539,464 → 536,368 bytes (−3,096)**. The DEBUG-only code paths were compile-checked separately (there is no Debug ninja directory) and go from 12 to 7 references; `llvm-nm` confirms `add_system_property`, `find_bundled_property` and `setup_environment_from_override_file` are genuinely emitted rather than silently `#if`'d out. CoreCLR, NativeAOT and MonoVM all build clean.
1 parent 8527784 commit 73f4925

8 files changed

Lines changed: 143 additions & 113 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -311,8 +311,8 @@ namespace {
311311
return;
312312
}
313313

314-
std::string const& code_cache_dir = AndroidSystem::get_app_code_cache_dir ();
315-
if (code_cache_dir.empty ()) {
314+
const char *code_cache_dir = AndroidSystem::get_app_code_cache_dir ();
315+
if (*code_cache_dir == '\0') {
316316
return;
317317
}
318318

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ auto FastDevAssemblies::open_assembly (std::string_view const& name, int64_t &si
3636
return nullptr;
3737
}
3838

39-
std::string const& override_dir_path = AndroidSystem::get_primary_override_dir ();
39+
const char *override_dir_path = AndroidSystem::get_primary_override_dir ();
4040
if (!Util::dir_exists (override_dir_path)) [[unlikely]] {
41-
log_debugf (LOG_ASSEMBLY, "Override directory '%s' does not exist", override_dir_path.c_str ());
41+
log_debugf (LOG_ASSEMBLY, "Override directory '%s' does not exist", override_dir_path);
4242
return nullptr;
4343
}
4444

@@ -47,9 +47,9 @@ auto FastDevAssemblies::open_assembly (std::string_view const& name, int64_t &si
4747
if (override_dir_fd < 0) [[unlikely]] {
4848
lock_guard dir_lock { override_dir_lock };
4949
if (override_dir_fd < 0) [[likely]] {
50-
override_dir = opendir (override_dir_path.c_str ());
50+
override_dir = opendir (override_dir_path);
5151
if (override_dir == nullptr) [[unlikely]] {
52-
log_warnf (LOG_ASSEMBLY, "Failed to open override dir '%s'. %s", override_dir_path.c_str (), strerror (errno));
52+
log_warnf (LOG_ASSEMBLY, "Failed to open override dir '%s'. %s", override_dir_path, strerror (errno));
5353
return nullptr;
5454
}
5555
override_dir_fd = dirfd (override_dir);
@@ -60,7 +60,7 @@ auto FastDevAssemblies::open_assembly (std::string_view const& name, int64_t &si
6060
LOG_ASSEMBLY,
6161
"Attempting to load FastDev assembly '%.*s' from override directory '%s'",
6262
static_cast<int>(name.length ()), name.data (),
63-
override_dir_path.c_str ()
63+
override_dir_path
6464
);
6565

6666
if (!Util::file_exists (override_dir_fd, name)) {
@@ -133,14 +133,14 @@ auto FastDevAssemblies::build_tpa_list (std::string &tpa_list) noexcept -> bool
133133
{
134134
tpa_list.clear ();
135135

136-
std::string const& override_dir_path = AndroidSystem::get_primary_override_dir ();
136+
const char *override_dir_path = AndroidSystem::get_primary_override_dir ();
137137
if (!Util::dir_exists (override_dir_path)) {
138138
return false;
139139
}
140140

141-
DIR *dir = opendir (override_dir_path.c_str ());
141+
DIR *dir = opendir (override_dir_path);
142142
if (dir == nullptr) {
143-
log_warnf (LOG_ASSEMBLY, "FastDev: failed to open override dir '%s'. %s", override_dir_path.c_str (), std::strerror (errno));
143+
log_warnf (LOG_ASSEMBLY, "FastDev: failed to open override dir '%s'. %s", override_dir_path, std::strerror (errno));
144144
return false;
145145
}
146146

@@ -183,7 +183,7 @@ auto FastDevAssemblies::build_tpa_list (std::string &tpa_list) noexcept -> bool
183183
LOG_ASSEMBLY,
184184
"FastDev: built TPA list with %zu assemblies from '%s' (corelib=%s, r2r=%s)",
185185
count,
186-
override_dir_path.c_str (),
186+
override_dir_path,
187187
found_corelib ? "true" : "false",
188188
found_r2r ? "true" : "false"
189189
);

src/native/clr/host/host.cc

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -91,16 +91,16 @@ bool Host::clr_external_assembly_probe (const char *path, void **data_start, int
9191
[[gnu::always_inline]]
9292
void Host::scan_filesystem_for_assemblies_and_libraries () noexcept
9393
{
94-
std::string const& native_lib_dir = AndroidSystem::get_native_libraries_dir ();
95-
log_debugf (LOG_ASSEMBLY, "Looking for assemblies in '%s'", native_lib_dir.c_str ());
94+
const char *native_lib_dir = AndroidSystem::get_native_libraries_dir ();
95+
log_debugf (LOG_ASSEMBLY, "Looking for assemblies in '%s'", native_lib_dir);
9696

97-
DIR *lib_dir = opendir (native_lib_dir.c_str ());
97+
DIR *lib_dir = opendir (native_lib_dir);
9898
if (lib_dir == nullptr) [[unlikely]] {
9999
Helpers::abort_applicationf (
100100
LOG_ASSEMBLY,
101101
std::source_location::current (),
102102
"Unable to open native library directory '%s'. %s",
103-
native_lib_dir.c_str (),
103+
native_lib_dir,
104104
std::strerror (errno)
105105
);
106106
}
@@ -111,7 +111,7 @@ void Host::scan_filesystem_for_assemblies_and_libraries () noexcept
111111
LOG_ASSEMBLY,
112112
std::source_location::current (),
113113
"Unable to obtain file descriptor for opened directory '%s'. %s",
114-
native_lib_dir.c_str (),
114+
native_lib_dir,
115115
std::strerror (errno)
116116
);
117117
}
@@ -121,7 +121,7 @@ void Host::scan_filesystem_for_assemblies_and_libraries () noexcept
121121
dirent *cur = readdir (lib_dir);
122122
if (cur == nullptr) {
123123
if (errno != 0) {
124-
log_warnf (LOG_ASSEMBLY, "Failed to open a directory entry from '%s': %s", native_lib_dir.c_str (), std::strerror (errno));
124+
log_warnf (LOG_ASSEMBLY, "Failed to open a directory entry from '%s': %s", native_lib_dir, std::strerror (errno));
125125
continue; // No harm, keep going
126126
}
127127
break; // we're done
@@ -138,7 +138,7 @@ void Host::scan_filesystem_for_assemblies_and_libraries () noexcept
138138
continue;
139139
}
140140

141-
log_debugf (LOG_ASSEMBLY, "Found assembly store in '%s/%s'", native_lib_dir.c_str (), Constants::assembly_store_file_name.data ());
141+
log_debugf (LOG_ASSEMBLY, "Found assembly store in '%s/%s'", native_lib_dir, Constants::assembly_store_file_name.data ());
142142

143143
std::string store_path = native_lib_dir;
144144
store_path.append ("/"sv);
@@ -339,7 +339,7 @@ void Host::Java_mono_android_Runtime_initInternal (
339339
AndroidSystem::set_app_code_cache_dir (applicationDirs[Constants::APP_DIRS_CODE_CACHE_DIR_INDEX]);
340340
AndroidSystem::create_update_dir (AndroidSystem::get_primary_override_dir ());
341341
AndroidSystem::setup_environment ();
342-
Logger::init_reference_logging (AndroidSystem::get_primary_override_dir ().c_str ());
342+
Logger::init_reference_logging (AndroidSystem::get_primary_override_dir ());
343343

344344
jstring_array_wrapper runtimeApks (env, runtimeApksJava);
345345
AndroidSystem::setup_app_library_directories (runtimeApks, applicationDirs, haveSplitApks);

src/native/clr/include/host/host-environment.hh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ namespace xamarin::android {
9292
[[gnu::flatten, gnu::always_inline]]
9393
static void create_xdg_directory (jstring_wrapper &home, std::string_view const& relative_path, std::string_view const& environment_variable_name) noexcept
9494
{
95-
std::string_view home_path = home.get_string_view ();
95+
const char *home_path = home.get_cstr ();
9696
char stack_buffer [Util::LocalPathBufferSize];
9797
ssize_t result = Util::format_joined_path (stack_buffer, sizeof (stack_buffer), home_path, relative_path);
9898
abort_unless (result >= 0, "XDG directory path is too long");

src/native/clr/include/runtime-base/android-system.hh

Lines changed: 34 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
#include <cstdio>
55
#include <limits>
66
#include <span>
7-
#include <string>
87
#include <string_view>
98

109
#include "../constants.hh"
@@ -32,11 +31,11 @@ namespace xamarin::android {
3231
#if !defined (XA_HOST_NATIVEAOT)
3332
// This optimizes things a little bit. The array is allocated at build time, so we pay no cost for its
3433
// allocation and at run time it allows us to skip dynamic memory allocation.
35-
inline static std::array<std::string, 1> single_app_lib_directory{};
36-
inline static std::span<std::string> app_lib_directories;
34+
inline static const char *single_app_lib_directory [1] { "" };
35+
inline static std::span<const char*> app_lib_directories;
3736

3837
// TODO: override dirs not implemented
39-
inline static std::array<std::string, 1> override_dirs{};
38+
inline static const char *override_dirs [1] { "" };
4039

4140
static constexpr std::array<std::string_view, 7> android_abi_names {
4241
std::string_view { "unknown" }, // CPU_KIND_UNKNOWN
@@ -73,45 +72,47 @@ namespace xamarin::android {
7372
running_in_emulator = yesno;
7473
}
7574

76-
#if defined (XA_HOST_NATIVEAOT)
7775
static auto get_primary_override_dir () noexcept -> const char*
7876
{
7977
return primary_override_dir;
8078
}
81-
#else
82-
static auto get_primary_override_dir () noexcept -> std::string const&
83-
{
84-
return primary_override_dir;
85-
}
86-
#endif
8779

8880
static void set_primary_override_dir (jstring_wrapper& home) noexcept
8981
{
90-
#if defined (XA_HOST_NATIVEAOT)
91-
ssize_t result = format_primary_override_dir (home, primary_override_dir, sizeof (primary_override_dir));
92-
abort_unless (result >= 0, "Primary override directory path is too long");
93-
#else
94-
primary_override_dir = determine_primary_override_dir (home);
95-
#endif
82+
char stack_buffer [Constants::SENSIBLE_PATH_MAX];
83+
char *path = stack_buffer;
84+
ssize_t result = format_primary_override_dir (home, path, sizeof (stack_buffer));
85+
if (result < 0) {
86+
size_t required_capacity = static_cast<size_t>(-result);
87+
path = static_cast<char*> (std::malloc (required_capacity));
88+
abort_unless (path != nullptr, "Failed to allocate primary override directory path");
89+
result = format_primary_override_dir (home, path, required_capacity);
90+
}
91+
abort_unless (result >= 0, "Failed to format primary override directory path using the required capacity");
92+
93+
primary_override_dir = Util::duplicate_string (path);
94+
if (path != stack_buffer) {
95+
std::free (path);
96+
}
9697
}
9798

9899
#if !defined (XA_HOST_NATIVEAOT)
99-
static auto get_app_code_cache_dir () noexcept -> std::string const&
100+
static auto get_app_code_cache_dir () noexcept -> const char*
100101
{
101102
return app_code_cache_dir;
102103
}
103104

104105
static void set_app_code_cache_dir (jstring_wrapper& code_cache_dir) noexcept
105106
{
106-
app_code_cache_dir.assign (code_cache_dir.get_cstr ());
107+
app_code_cache_dir = Util::duplicate_string (code_cache_dir.get_cstr ());
107108
}
108109

109-
static auto get_native_libraries_dir () noexcept -> std::string const&
110+
static auto get_native_libraries_dir () noexcept -> const char*
110111
{
111112
return native_libraries_dir;
112113
}
113114

114-
static void create_update_dir (std::string const& override_dir) noexcept
115+
static void create_update_dir (const char *override_dir) noexcept
115116
{
116117
if constexpr (Constants::is_release_build) {
117118
/*
@@ -129,8 +130,8 @@ namespace xamarin::android {
129130
}
130131
}
131132

132-
log_debugf (LOG_DEFAULT, "Creating public update directory: `%s`", override_dir.c_str ());
133-
Util::create_public_directory (override_dir.c_str ());
133+
log_debugf (LOG_DEFAULT, "Creating public update directory: `%s`", override_dir);
134+
Util::create_public_directory (override_dir);
134135
}
135136
#endif
136137

@@ -158,9 +159,9 @@ namespace xamarin::android {
158159
static auto load_dso_from_any_directories (std::string_view const& name, int dl_flags, bool is_jni) noexcept -> void*;
159160

160161
private:
161-
static auto format_full_dso_path (std::string const& base_dir, std::string_view const& dso_path, char *buffer, size_t buffer_size) noexcept -> ssize_t;
162+
static auto format_full_dso_path (const char *base_dir, std::string_view const& dso_path, char *buffer, size_t buffer_size) noexcept -> ssize_t;
162163

163-
static auto get_full_dso_path (std::string const& base_dir, std::string_view const& dso_path, char *stack_buffer, size_t stack_buffer_size) noexcept -> char*
164+
static auto get_full_dso_path (const char *base_dir, std::string_view const& dso_path, char *stack_buffer, size_t stack_buffer_size) noexcept -> char*
164165
{
165166
ssize_t result = format_full_dso_path (base_dir, dso_path, stack_buffer, stack_buffer_size);
166167
if (result >= 0) {
@@ -226,38 +227,18 @@ namespace xamarin::android {
226227
return static_cast<ssize_t>(length);
227228
}
228229

229-
#if !defined (XA_HOST_NATIVEAOT)
230-
static auto determine_primary_override_dir (jstring_wrapper &home) noexcept -> std::string
231-
{
232-
char stack_buffer [Constants::SENSIBLE_PATH_MAX];
233-
size_t length;
234-
char *name = Util::format_with_retry (
235-
stack_buffer,
236-
sizeof (stack_buffer),
237-
[&home](char *buffer, size_t buffer_size) noexcept {
238-
return format_primary_override_dir (home, buffer, buffer_size);
239-
},
240-
&length
241-
);
242-
243-
std::string path { name, length };
244-
if (name != stack_buffer) {
245-
std::free (name);
246-
}
247-
return path;
248-
}
249-
#endif
250-
251230
private:
252231
static inline long max_gref_count = 0;
253232
static inline bool running_in_emulator = false;
254233
static inline bool embedded_dso_mode_enabled = false;
255-
#if defined (XA_HOST_NATIVEAOT)
256-
static inline char primary_override_dir[Constants::SENSIBLE_PATH_MAX] {};
257-
#else
258-
static inline std::string primary_override_dir;
259-
static inline std::string native_libraries_dir;
260-
static inline std::string app_code_cache_dir;
234+
// These are set once, early during startup, and are read for as long as the process lives.
235+
// They are plain pointers so that they are constant-initialized: a `std::string` here would
236+
// make the compiler emit a guard variable and an `atexit` registration in every translation
237+
// unit which includes this header.
238+
static inline const char *primary_override_dir = "";
239+
#if !defined (XA_HOST_NATIVEAOT)
240+
static inline const char *native_libraries_dir = "";
241+
static inline const char *app_code_cache_dir = "";
261242

262243
#if defined (DEBUG)
263244
static inline BundledProperty *bundled_properties = nullptr;

src/native/clr/include/runtime-base/util.hh

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,21 @@ namespace xamarin::android {
4141
public:
4242
static constexpr size_t LocalPathBufferSize = Constants::SENSIBLE_PATH_MAX;
4343

44+
// Returns a copy of `str` allocated with `malloc`, aborting the application if the
45+
// allocation fails. Used for values which are set once, early during startup, and which
46+
// then live for as long as the process does - the copies are never freed.
47+
static auto duplicate_string (const char *str) noexcept -> char*
48+
{
49+
abort_unless (str != nullptr, "String to duplicate must not be null");
50+
51+
char *ret = strdup (str);
52+
if (ret == nullptr) [[unlikely]] {
53+
Helpers::abort_application (LOG_DEFAULT, "Unable to allocate memory for a string copy");
54+
}
55+
56+
return ret;
57+
}
58+
4459
static int create_directory (const char *pathname, mode_t mode);
4560

4661
static auto create_directory (std::string_view const& dir, mode_t mode) noexcept -> int
@@ -310,6 +325,22 @@ namespace xamarin::android {
310325
return !path.empty () && path.contains ('/');
311326
}
312327

328+
[[gnu::flatten, gnu::always_inline]]
329+
static auto ends_with (const char *value, const char *suffix) noexcept -> bool
330+
{
331+
if (value == nullptr || suffix == nullptr) {
332+
return false;
333+
}
334+
335+
size_t value_length = strlen (value);
336+
size_t suffix_length = strlen (suffix);
337+
if (suffix_length > value_length) {
338+
return false;
339+
}
340+
341+
return memcmp (value + value_length - suffix_length, suffix, suffix_length) == 0;
342+
}
343+
313344
// Returns the path length excluding NUL, or the negative required capacity including NUL.
314345
static auto format_joined_path (char *buffer, size_t buffer_size, std::string_view first, std::string_view second) noexcept -> ssize_t
315346
{

0 commit comments

Comments
 (0)