From 56e2facd99af3ba283f678195fb48bc3ef71ead8 Mon Sep 17 00:00:00 2001 From: mathieubourgeois Date: Thu, 3 Jan 2019 16:26:40 -0500 Subject: [PATCH] [monodroid] mmap files separately, not the entire apk (#2570) Fixes: https://github.com/xamarin/xamarin-android/issues/1673 During process startup, Xamarin.Android looks through the `.apk` for various files to register within mono and itself, including: * Assemblies (`.dll`/`.exe`) * Assembly configuration files (`.dll.config`) * Debug symbols (`.mdb`/`.pdb`) * Type Map files (`.jm`/`.mj`) Such files are stored *uncompressed* within the `.apk` and are aligned on 4-byte boundaries via `zipalign`. In order to *use* these files, the entire `.apk` would be **mmap**(2)'d into the process address space, so that e.g. the assemblies could be registered directly with mono without copying the assembly contents "elsewhere" (e.g. disk, RAM), and instead would be demand-paged *into* RAM from disk as needed. There is an unfortunate downside to this approach: `.apk` files can contain lots of content which *isn't* of interest to mono/etc., such as Android Assets and Resources (large MP4 video files?), and the "`mmap()` everything!" approach means that all this *unneeded* data is *also* `mmap()`ed into the process address space. Which is what Issue #1673 triggered: a 930MB `.apk` file with lots of "game assets" could not load within a 32-bit address space, because there wasn't 930MB of free contiguous address space to use: I/monodroid-assembly(14915): start: 0xffffffff end: 0x3da16747 len: 1033987912 apk: /data/app/-1/base.apk A/libc(14915): Fatal signal 11 (SIGSEGV), code 1, fault addr 0x3da16343 in tid 14915 `mmap()`ing the whole `.apk`, while effective, is overkill. Update `gather_bundled_assemblies_from_apk()` so that instead of loading the entire `.apk` with one `mmap()` call, we instead use separate `mmap()` calls for each distinct file of interest within the `.apk` -- on OS page-aligned memory -- so that we don't excessively use process address space. --- src/monodroid/jni/embedded-assemblies.cc | 101 +++++++++++++---------- src/monodroid/jni/monodroid-glue.h | 1 + 2 files changed, 60 insertions(+), 42 deletions(-) diff --git a/src/monodroid/jni/embedded-assemblies.cc b/src/monodroid/jni/embedded-assemblies.cc index d740775f822..0e68779dd5a 100644 --- a/src/monodroid/jni/embedded-assemblies.cc +++ b/src/monodroid/jni/embedded-assemblies.cc @@ -7,6 +7,7 @@ #include #include #include +#include extern "C" { #include "java-interop-util.h" @@ -19,6 +20,7 @@ extern "C" { #include "ioapi.h" #include "embedded-assemblies.h" #include "globals.h" +#include "monodroid-glue.h" using namespace xamarin::android; using namespace xamarin::android::internal; @@ -243,6 +245,34 @@ struct md_mmap_info { size_t size; }; +static md_mmap_info +md_mmap_apk_file (int fd, uLong offset, uLong size, const char* filename, const char* apk) +{ + md_mmap_info file_info; + md_mmap_info mmap_info; + + int pageSize = monodroid_getpagesize(); + uLong offsetFromPage = offset % pageSize; + uLong offsetPage = offset - offsetFromPage; + uLong offsetSize = size + offsetFromPage; + + mmap_info.area = mmap (NULL, offsetSize, PROT_READ, MAP_PRIVATE, fd, offsetPage); + if (mmap_info.area == MAP_FAILED) { + log_fatal (LOG_DEFAULT, "Could not `mmap` apk `%s` entry `%s`: %s", apk, filename, strerror (errno)); + exit (FATAL_EXIT_CANNOT_FIND_APK); + } + + mmap_info.size = offsetSize; + file_info.area = (void*)((const char*)mmap_info.area + offsetFromPage); + file_info.size = size; + + log_info (LOG_ASSEMBLY, " mmap_start: %08p mmap_end: %08p mmap_len: % 12u file_start: %08p file_end: %08p file_len: % 12u apk: %s file: %s", + mmap_info.area, reinterpret_cast (mmap_info.area) + mmap_info.size, (unsigned int) mmap_info.size, + file_info.area, reinterpret_cast (file_info.area) + file_info.size, (unsigned int) file_info.size, apk, filename); + + return file_info; +} + static void* md_mmap_open_file (void *opaque, const char *filename, int mode) { @@ -254,37 +284,31 @@ md_mmap_open_file (void *opaque, const char *filename, int mode) static uLong md_mmap_read_file (void *opaque, void *stream, void *buf, uLong size) { - int *offset = reinterpret_cast (stream); - struct md_mmap_info *info = reinterpret_cast (opaque); - - memcpy (buf, ((const char*) info->area) + *offset, size); - *offset += size; - - return size; + int fd = *reinterpret_cast(opaque); + return read (fd, buf, size); } static long md_mmap_tell_file (void *opaque, void *stream) { - int *offset = reinterpret_cast (stream); - return *offset; + int fd = *reinterpret_cast(opaque); + return lseek (fd, 0, SEEK_CUR); } static long md_mmap_seek_file (void *opaque, void *stream, uLong offset, int origin) { - int *pos = reinterpret_cast (stream); - struct md_mmap_info *info = reinterpret_cast (opaque); + int fd = *reinterpret_cast(opaque); switch (origin) { case ZLIB_FILEFUNC_SEEK_END: - *pos = info->size; - /* goto case ZLIB_FILEFUNC_SEEK_CUR */ + lseek (fd, offset, SEEK_END); + break; case ZLIB_FILEFUNC_SEEK_CUR: - *pos += (int) offset; + lseek (fd, offset, SEEK_CUR); break; case ZLIB_FILEFUNC_SEEK_SET: - *pos = (int) offset; + lseek (fd, offset, SEEK_SET); break; default: return -1; @@ -351,8 +375,6 @@ gather_bundled_assemblies_from_apk ( int *bundle_count) { int fd; - struct stat buf; - struct md_mmap_info mmap_info; unzFile file; zlib_filefunc_def funcs = { @@ -371,21 +393,8 @@ gather_bundled_assemblies_from_apk ( // TODO: throw return -1; } - if (fstat (fd, &buf) < 0) { - close (fd); - // TODO: throw - return -1; - } - - mmap_info.area = mmap (NULL, buf.st_size, PROT_READ, MAP_PRIVATE, fd, 0); - mmap_info.size = buf.st_size; - - log_info (LOG_ASSEMBLY, " start: %08p end: %08p len: % 12u apk: %s", - mmap_info.area, reinterpret_cast (mmap_info.area) + mmap_info.size, (unsigned int) mmap_info.size, apk); - - close (fd); - funcs.opaque = &mmap_info; + funcs.opaque = &fd; if ((file = unzOpen2 (NULL, &funcs)) != NULL) { do { @@ -405,11 +414,13 @@ gather_bundled_assemblies_from_apk ( } if (utils.ends_with (cur_entry_name, ".jm")) { - add_type_mapping (&java_to_managed_maps, apk, cur_entry_name, ((const char*) mmap_info.area) + offset); + md_mmap_info map_info = md_mmap_apk_file (fd, offset, info.uncompressed_size, cur_entry_name, apk); + add_type_mapping (&java_to_managed_maps, apk, cur_entry_name, (const char*)map_info.area); continue; } if (utils.ends_with (cur_entry_name, ".mj")) { - add_type_mapping (&managed_to_java_maps, apk, cur_entry_name, ((const char*) mmap_info.area) + offset); + md_mmap_info map_info = md_mmap_apk_file (fd, offset, info.uncompressed_size, cur_entry_name, apk); + add_type_mapping (&managed_to_java_maps, apk, cur_entry_name, (const char*)map_info.area); continue; } @@ -419,8 +430,8 @@ gather_bundled_assemblies_from_apk ( // assemblies must be 4-byte aligned, or Bad Things happen if ((offset & 0x3) != 0) { - log_fatal (LOG_ASSEMBLY, "Assembly '%s' is located at a bad address %p\n", cur_entry_name, - ((const unsigned char*) mmap_info.area) + offset); + log_fatal (LOG_ASSEMBLY, "Assembly '%s' is located at bad offset %lu within the .apk\n", cur_entry_name, + offset); log_fatal (LOG_ASSEMBLY, "You MUST run `zipalign` on %s\n", strrchr (apk, '/') + 1); exit (FATAL_EXIT_MISSING_ZIPALIGN); } @@ -431,19 +442,22 @@ gather_bundled_assemblies_from_apk ( if ((utils.ends_with (cur_entry_name, ".mdb") || utils.ends_with (cur_entry_name, ".pdb")) && register_debug_symbols && !entry_is_overridden && - *bundle != NULL && - register_debug_symbols_for_assembly (mono, cur_entry_name, (*bundle) [*bundle_count-1], - ((const mono_byte*) mmap_info.area) + offset, + *bundle != NULL) { + md_mmap_info map_info = md_mmap_apk_file(fd, offset, info.uncompressed_size, cur_entry_name, apk); + if (register_debug_symbols_for_assembly (mono, cur_entry_name, (*bundle) [*bundle_count-1], + (const mono_byte*)map_info.area, info.uncompressed_size)) - continue; + continue; + } if (utils.ends_with (cur_entry_name, ".config") && *bundle != NULL) { char *assembly_name = utils.monodroid_strdup_printf ("%s", basename (cur_entry_name)); // Remove '.config' suffix *strrchr (assembly_name, '.') = '\0'; - - mono->register_config_for_assembly (assembly_name, ((const char*) mmap_info.area) + offset); + + md_mmap_info map_info = md_mmap_apk_file(fd, offset, info.uncompressed_size, cur_entry_name, apk); + mono->register_config_for_assembly (assembly_name, (const char*)map_info.area); continue; } @@ -458,8 +472,9 @@ gather_bundled_assemblies_from_apk ( cur = (*bundle) [*bundle_count] = reinterpret_cast (utils.xcalloc (1, sizeof (MonoBundledAssembly))); ++*bundle_count; + md_mmap_info map_info = md_mmap_apk_file(fd, offset, info.uncompressed_size, cur_entry_name, apk); cur->name = utils.monodroid_strdup_printf ("%s", strstr (cur_entry_name, prefix) + strlen (prefix)); - cur->data = ((const unsigned char*) mmap_info.area) + offset; + cur->data = (const unsigned char*)map_info.area; // MonoBundledAssembly::size is const?! psize = (unsigned int*) &cur->size; @@ -483,6 +498,8 @@ gather_bundled_assemblies_from_apk ( } while (unzGoToNextFile (file) == UNZ_OK); unzClose (file); } + + close(fd); return 0; } diff --git a/src/monodroid/jni/monodroid-glue.h b/src/monodroid/jni/monodroid-glue.h index 5dd18065b14..633c187d1e4 100644 --- a/src/monodroid/jni/monodroid-glue.h +++ b/src/monodroid/jni/monodroid-glue.h @@ -11,6 +11,7 @@ extern "C" { #endif MONO_API int monodroid_get_system_property (const char *name, char **value); + MONO_API int monodroid_getpagesize (void); #ifdef __cplusplus } #endif