Skip to content

Commit

Permalink
[native] More C++ tweaks and changes (#9478)
Browse files Browse the repository at this point in the history 
Continued effort to restructure our native C++ runtime, introducing
C++23 features as well as turning as many classes into static ones
since we don't really have objects that need to be created and
destroyed during lifetime of the application:

  * Replace `abort_if*` preprocessor macros with templated functions.
    This gives us better type safety.  Instead using variadic
    arguments, we now provide overloads which take either a plain
    string without placeholders or a lambda function to format the
    message string on demand.
  * More `std::string_view` for literal strings
  * More functions decorated with `noexcept` (this is going to be
    important once exceptions are enabled in the future)
  * Remove some unused code.
  • Loading branch information
@grendello
grendello authored Dec 5, 2024
1 parent 046b44f commit 7b7cd13
Show file tree
Hide file tree
Showing 18 changed files with 425 additions and 343 deletions.
4 changes: 2 additions & 2 deletions src/native/monodroid/debug.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ Debug::start_debugging (void)
if (sdb_fd == 0)
return;

embeddedAssemblies.set_register_debug_symbols (true);
EmbeddedAssemblies::set_register_debug_symbols (true);

char *debug_arg = Util::monodroid_strdup_printf ("--debugger-agent=transport=socket-fd,address=%d,embedding=1", sdb_fd);
std::array<char*, 2> debug_options = {
Expand Down Expand Up @@ -603,7 +603,7 @@ Debug::enable_soft_breakpoints (void)
void*
xamarin::android::conn_thread (void *arg)
{
abort_if_invalid_pointer_argument (arg);
abort_if_invalid_pointer_argument (arg, "arg");

int res;
Debug *instance = static_cast<Debug*> (arg);
Expand Down
22 changes: 11 additions & 11 deletions src/native/monodroid/embedded-assemblies-zip.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ EmbeddedAssemblies::zip_load_assembly_store_entries (std::vector<uint8_t> const&
dynamic_local_string<SENSIBLE_PATH_MAX> entry_name;
bool assembly_store_found = false;

log_debug (LOG_ASSEMBLY, "Looking for assembly stores in APK ('%s)", assembly_store_file_path.data ());
log_debug (LOG_ASSEMBLY, "Looking for assembly stores in APK ('%s')", assembly_store_file_path.data ());
for (size_t i = 0uz; i < num_entries; i++) {
if (all_required_zip_entries_found ()) {
need_to_scan_more_apks = false;
Expand Down Expand Up @@ -298,7 +298,7 @@ EmbeddedAssemblies::zip_load_assembly_store_entries (std::vector<uint8_t> const&
}

void
EmbeddedAssemblies::zip_load_entries (int fd, const char *apk_name, [[maybe_unused]] monodroid_should_register should_register)
EmbeddedAssemblies::zip_load_entries (int fd, const char *apk_name, [[maybe_unused]] monodroid_should_register should_register) noexcept
{
uint32_t cd_offset;
uint32_t cd_size;
Expand Down Expand Up @@ -412,7 +412,7 @@ EmbeddedAssemblies::set_debug_entry_data (XamarinAndroidBundledAssembly &entry,
}

bool
EmbeddedAssemblies::zip_read_cd_info (int fd, uint32_t& cd_offset, uint32_t& cd_size, uint16_t& cd_entries)
EmbeddedAssemblies::zip_read_cd_info (int fd, uint32_t& cd_offset, uint32_t& cd_size, uint16_t& cd_entries) noexcept
{
// The simplest case - no file comment
off_t ret = ::lseek (fd, -ZIP_EOCD_LEN, SEEK_END);
Expand Down Expand Up @@ -478,7 +478,7 @@ EmbeddedAssemblies::zip_read_cd_info (int fd, uint32_t& cd_offset, uint32_t& cd_
}

bool
EmbeddedAssemblies::zip_adjust_data_offset (int fd, ZipEntryLoadState &state)
EmbeddedAssemblies::zip_adjust_data_offset (int fd, ZipEntryLoadState &state) noexcept
{
static constexpr size_t LH_FILE_NAME_LENGTH_OFFSET = 26uz;
static constexpr size_t LH_EXTRA_LENGTH_OFFSET = 28uz;
Expand Down Expand Up @@ -530,7 +530,7 @@ EmbeddedAssemblies::zip_adjust_data_offset (int fd, ZipEntryLoadState &state)

template<size_t BufSize>
bool
EmbeddedAssemblies::zip_extract_cd_info (std::array<uint8_t, BufSize> const& buf, uint32_t& cd_offset, uint32_t& cd_size, uint16_t& cd_entries)
EmbeddedAssemblies::zip_extract_cd_info (std::array<uint8_t, BufSize> const& buf, uint32_t& cd_offset, uint32_t& cd_size, uint16_t& cd_entries) noexcept
{
constexpr size_t EOCD_TOTAL_ENTRIES_OFFSET = 10uz;
constexpr size_t EOCD_CD_SIZE_OFFSET = 12uz;
Expand Down Expand Up @@ -558,7 +558,7 @@ EmbeddedAssemblies::zip_extract_cd_info (std::array<uint8_t, BufSize> const& buf

template<class T>
force_inline bool
EmbeddedAssemblies::zip_ensure_valid_params (T const& buf, size_t index, size_t to_read) const noexcept
EmbeddedAssemblies::zip_ensure_valid_params (T const& buf, size_t index, size_t to_read) noexcept
{
if (index + to_read > buf.size ()) {
log_error (LOG_ASSEMBLY, "Buffer too short to read %u bytes of data", to_read);
Expand All @@ -570,7 +570,7 @@ EmbeddedAssemblies::zip_ensure_valid_params (T const& buf, size_t index, size_t

template<ByteArrayContainer T>
bool
EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, uint16_t& dst) const noexcept
EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, uint16_t& dst) noexcept
{
if (!zip_ensure_valid_params (src, source_index, sizeof (dst))) {
return false;
Expand All @@ -583,7 +583,7 @@ EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, uint16_t&

template<ByteArrayContainer T>
bool
EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, uint32_t& dst) const noexcept
EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, uint32_t& dst) noexcept
{
if (!zip_ensure_valid_params (src, source_index, sizeof (dst))) {
return false;
Expand All @@ -600,7 +600,7 @@ EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, uint32_t&

template<ByteArrayContainer T>
bool
EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, std::array<uint8_t, 4>& dst_sig) const noexcept
EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, std::array<uint8_t, 4>& dst_sig) noexcept
{
if (!zip_ensure_valid_params (src, source_index, dst_sig.size ())) {
return false;
Expand All @@ -612,7 +612,7 @@ EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, std::arra

template<ByteArrayContainer T>
bool
EmbeddedAssemblies::zip_read_field (T const& buf, size_t index, size_t count, dynamic_local_string<SENSIBLE_PATH_MAX>& characters) const noexcept
EmbeddedAssemblies::zip_read_field (T const& buf, size_t index, size_t count, dynamic_local_string<SENSIBLE_PATH_MAX>& characters) noexcept
{
if (!zip_ensure_valid_params (buf, index, count)) {
return false;
Expand All @@ -623,7 +623,7 @@ EmbeddedAssemblies::zip_read_field (T const& buf, size_t index, size_t count, dy
}

bool
EmbeddedAssemblies::zip_read_entry_info (std::vector<uint8_t> const& buf, dynamic_local_string<SENSIBLE_PATH_MAX>& file_name, ZipEntryLoadState &state)
EmbeddedAssemblies::zip_read_entry_info (std::vector<uint8_t> const& buf, dynamic_local_string<SENSIBLE_PATH_MAX>& file_name, ZipEntryLoadState &state) noexcept
{
constexpr size_t CD_COMPRESSION_METHOD_OFFSET = 10uz;
constexpr size_t CD_UNCOMPRESSED_SIZE_OFFSET = 24uz;
Expand Down
24 changes: 11 additions & 13 deletions src/native/monodroid/embedded-assemblies.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class MonoGuidString final
char *guid = nullptr;
};

void EmbeddedAssemblies::set_assemblies_prefix (const char *prefix)
void EmbeddedAssemblies::set_assemblies_prefix (const char *prefix) noexcept
{
if (assemblies_prefix_override != nullptr)
delete[] assemblies_prefix_override;
Expand Down Expand Up @@ -493,25 +493,25 @@ MonoAssembly*
EmbeddedAssemblies::open_from_bundles (MonoAssemblyLoadContextGCHandle alc_gchandle, MonoAssemblyName *aname, [[maybe_unused]] char **assemblies_path, [[maybe_unused]] void *user_data, MonoError *error)
{
constexpr bool ref_only = false;
return embeddedAssemblies.open_from_bundles (aname, alc_gchandle, error, ref_only);
return EmbeddedAssemblies::open_from_bundles (aname, alc_gchandle, error, ref_only);
}

MonoAssembly*
EmbeddedAssemblies::open_from_bundles_full (MonoAssemblyName *aname, [[maybe_unused]] char **assemblies_path, [[maybe_unused]] void *user_data)
{
constexpr bool ref_only = false;

return embeddedAssemblies.open_from_bundles (aname, ref_only /* loader_data */, nullptr /* error */, ref_only);
return EmbeddedAssemblies::open_from_bundles (aname, ref_only /* loader_data */, nullptr /* error */, ref_only);
}

void
EmbeddedAssemblies::install_preload_hooks_for_appdomains ()
EmbeddedAssemblies::install_preload_hooks_for_appdomains () noexcept
{
mono_install_assembly_preload_hook (open_from_bundles_full, nullptr);
}

void
EmbeddedAssemblies::install_preload_hooks_for_alc ()
EmbeddedAssemblies::install_preload_hooks_for_alc () noexcept
{
mono_install_assembly_preload_hook_v3 (
open_from_bundles,
Expand Down Expand Up @@ -679,7 +679,7 @@ EmbeddedAssemblies::typemap_java_to_managed (hash_t hash, const MonoString *java
} else {
MonoAssemblyLoadContextGCHandle alc_gchandle = mono_alc_get_default_gchandle ();
MonoError mono_error;
assm = embeddedAssemblies.open_from_bundles (assembly_name, alc_gchandle, &mono_error, false /* ref_only */);
assm = EmbeddedAssemblies::open_from_bundles (assembly_name, alc_gchandle, &mono_error, false /* ref_only */);
}

if (assm == nullptr) {
Expand Down Expand Up @@ -912,7 +912,7 @@ EmbeddedAssemblies::md_mmap_apk_file (int fd, uint32_t offset, size_t size, cons
}

void
EmbeddedAssemblies::gather_bundled_assemblies_from_apk (const char* apk, monodroid_should_register should_register)
EmbeddedAssemblies::gather_bundled_assemblies_from_apk (const char* apk, monodroid_should_register should_register) noexcept
{
int fd;

Expand Down Expand Up @@ -1284,12 +1284,10 @@ EmbeddedAssemblies::register_from_filesystem (const char *lib_dir_path,bool look
}

auto register_fn =
application_config.have_assembly_store ? std::mem_fn (&EmbeddedAssemblies::maybe_register_blob_from_filesystem) :
application_config.have_assembly_store ? &EmbeddedAssemblies::maybe_register_blob_from_filesystem :
(look_for_mangled_names ?
std::mem_fn (&EmbeddedAssemblies::maybe_register_assembly_from_filesystem<true>) :
std::mem_fn (&EmbeddedAssemblies::maybe_register_assembly_from_filesystem<false>
)
);
&EmbeddedAssemblies::maybe_register_assembly_from_filesystem<true> :
&EmbeddedAssemblies::maybe_register_assembly_from_filesystem<false>);

size_t assembly_count = 0uz;
do {
Expand Down Expand Up @@ -1334,7 +1332,7 @@ EmbeddedAssemblies::register_from_filesystem (const char *lib_dir_path,bool look
}

// We get `true` if it's time to terminate
if (register_fn (this, should_register, assembly_count, cur, state)) {
if (register_fn (should_register, assembly_count, cur, state)) {
break;
}
} while (true);
Expand Down
Loading

0 comments on commit 7b7cd13

Please sign in to comment.