On 8/2/23 20:25, Richard Henderson wrote:
On 8/1/23 16:27, Helge Deller wrote:
Reorganize the guest memory layout to get as much memory as possible for
heap for the guest application.

This patch optimizes the memory layout by loading pie executables
into lower memory and shared libs into higher memory (at
TASK_UNMAPPED_BASE). This leaves a bigger memory area usable for heap
space which will be located directly after the executable.
Up to now, pie executable and shared libs were loaded directly behind
each other in the area at TASK_UNMAPPED_BASE, which leaves very little
space for heap.

I tested this patchset with chroots of alpha, arm, armel, arm64, hppa, m68k,
mips64el, mipsel, powerpc, ppc64, ppc64el, s390x, sh4 and sparc64 on a x86-64
host, and with a static armhf binary (which fails to run without this patch).

This patch temporarily breaks the Thread Sanitizer (TSan) application
which expects specific boundary definitions for memory mappings on
different platforms [1], see commit aab613fb9597 ("linux-user: Update
TASK_UNMAPPED_BASE for aarch64") for aarch64. The follow-up patch fixes it
again.

[1] 
https://github.com/llvm/llvm-project/blob/master/compiler-rt/lib/tsan/rtl/tsan_platform.h

Signed-off-by: Helge Deller <del...@gmx.de>
---
  linux-user/elfload.c | 55 +++++++++++++-------------------------------
  linux-user/mmap.c    |  8 ++++---
  2 files changed, 21 insertions(+), 42 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 2aee2298ec..47a118e430 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -3023,6 +3023,7 @@ static void load_elf_image(const char *image_name, int 
image_fd,
      abi_ulong load_addr, load_bias, loaddr, hiaddr, error;
      int i, retval, prot_exec;
      Error *err = NULL;
+    bool is_main_executable;

      /* First of all, some simple consistency checks */
      if (!elf_check_ident(ehdr)) {
@@ -3106,28 +3107,8 @@ static void load_elf_image(const char *image_name, int 
image_fd,
          }
      }

-    if (pinterp_name != NULL) {
-        /*
-         * This is the main executable.
-         *
-         * Reserve extra space for brk.
-         * We hold on to this space while placing the interpreter
-         * and the stack, lest they be placed immediately after
-         * the data segment and block allocation from the brk.
-         *
-         * 16MB is chosen as "large enough" without being so large as
-         * to allow the result to not fit with a 32-bit guest on a
-         * 32-bit host. However some 64 bit guests (e.g. s390x)
-         * attempt to place their heap further ahead and currently
-         * nothing stops them smashing into QEMUs address space.
-         */
-#if TARGET_LONG_BITS == 64
-        info->reserve_brk = 32 * MiB;
-#else
-        info->reserve_brk = 16 * MiB;
-#endif
-        hiaddr += info->reserve_brk;
-
+    is_main_executable = (pinterp_name != NULL);

This will be false for static main executables.

[deller@p100 qemu-helge-user-armhf]$ file fstype
fstype: ELF 32-bit LSB executable, ARM, EABI5 version 1 (SYSV), statically 
linked, interpreter /lib/klibc-PupSAGgtpafMlSLXOLgje1kXFo8.so, 
BuildID[sha1]=45aac32edcd204fd6fa06febf3abff274ab39b26, stripped

And for real static binaries (without interpreter), the "loadaddr" is set in 
the mmap below,
and MAP_FIX isn't needed then.

+    if (is_main_executable) {
          if (ehdr->e_type == ET_EXEC) {
              /*
               * Make sure that the low address does not conflict with
@@ -3136,7 +3117,7 @@ static void load_elf_image(const char *image_name, int 
image_fd,
              probe_guest_base(image_name, loaddr, hiaddr);
          } else {
              /*
-             * The binary is dynamic, but we still need to
+             * The binary is dynamic (pie-executabe), but we still need to
               * select guest_base.  In this case we pass a size.
               */
              probe_guest_base(image_name, 0, hiaddr - loaddr);
@@ -3159,7 +3140,7 @@ static void load_elf_image(const char *image_name, int 
image_fd,
       */
      load_addr = target_mmap(loaddr, (size_t)hiaddr - loaddr + 1, PROT_NONE,
                              MAP_PRIVATE | MAP_ANON | MAP_NORESERVE |
-                            (ehdr->e_type == ET_EXEC ? MAP_FIXED : 0),
+                            (is_main_executable ? MAP_FIXED : 0),

This is definitely wrong, as all ET_EXEC require FIXED.

Not if the PIE flag is set too and even here the loadaddr defines where
it will be loaded then.

(In addition to static, it is possible to write an ET_EXEC interpreter.)

--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -299,14 +299,16 @@ static bool mmap_frag(abi_ulong real_start, abi_ulong 
start, abi_ulong last,
  #ifdef TARGET_AARCH64
  # define TASK_UNMAPPED_BASE  0x5500000000
  #else
-# define TASK_UNMAPPED_BASE  (1ul << 38)
+# define TASK_UNMAPPED_BASE  0x4000000000
  #endif
-#else
+#elif HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 32
  #ifdef TARGET_HPPA
  # define TASK_UNMAPPED_BASE  0xfa000000
  #else
-# define TASK_UNMAPPED_BASE  0x40000000
+# define TASK_UNMAPPED_BASE  0xe0000000
  #endif
+#else /* HOST_LONG_BITS == 32 && TARGET_ABI_BITS == 32 */
+# define TASK_UNMAPPED_BASE  0x40000000
  #endif
  abi_ulong mmap_next_start = TASK_UNMAPPED_BASE;

This should be a separate change.

While we're at it, we should move this to e.g.
linux-user/$GUEST/target_mman.h, and make this match each guest
kernel's TASK_UNMAPPED_BASE.  It really shouldn't depend on the host
at all.

I think it matters if you want to run a 32-bit guest on 32-bit host?
Those TASK_UNMAPPED_BASE are actually TARGET_TASK_UNMAPPED_BASE values.

Helge

Reply via email to