Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 26, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c6124e3
Cleanup + implement runtime property host contract API
grendello May 22, 2025
6e3c23e
Cleanup
grendello May 22, 2025
55283c9
More consistent use of string_view instead of char* for literals
grendello May 22, 2025
43dfecc
Work around a crash in Mono.Android_Tests
grendello May 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 20 additions & 21 deletions src/native/clr/host/assembly-store.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
)
);
Expand Down Expand Up @@ -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
)
);
Expand All @@ -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;
}
Expand All @@ -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
)
Expand All @@ -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)
Expand All @@ -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);
Expand Down Expand Up @@ -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
)
Expand All @@ -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),
Expand All @@ -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;
}
Expand All @@ -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 {
Expand All @@ -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 ()
)
);
Expand All @@ -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
Expand All @@ -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 ());
}
20 changes: 10 additions & 10 deletions src/native/clr/host/fastdev-assemblies.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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);
Expand All @@ -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;
}

Expand All @@ -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
)
Expand All @@ -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)
);
Expand All @@ -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);
}
4 changes: 3 additions & 1 deletion src/native/clr/host/host-jni.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Loading