[PATCH 0/6] linux-user: Rewrite open_self_maps

[PATCH 0/6] linux-user: Rewrite open_self_maps

[Richard Henderson]

Based-on: 20230816180338.572576-1-richard.henderson@linaro.org
("[PATCH v4 00/18] linux-user: Implement VDSOs")

As promised, a rewrite of /proc/self/{maps,smaps} emulation
using interval trees.

Incorporate Helge's change to mark [heap], and also mark [vdso].


r~


Richard Henderson (6):
  util/selfmap: Use dev_t and ino_t in MapInfo
  linux-user: Use walk_memory_regions for open_self_maps
  linux-user: Adjust brk for load_bias
  linux-user: Show heap address in /proc/pid/maps
  linux-user: Remove ELF_START_MMAP and image_info.start_mmap
  linux-user: Show vdso address in /proc/pid/maps

 include/qemu/selfmap.h |   4 +-
 linux-user/qemu.h      |   2 +-
 linux-user/elfload.c   |  41 +--------
 linux-user/syscall.c   | 194 +++++++++++++++++++++++++----------------
 util/selfmap.c         |  12 +--
 5 files changed, 131 insertions(+), 122 deletions(-)

-- 
2.34.1

[PATCH 1/6] util/selfmap: Use dev_t and ino_t in MapInfo

[Richard Henderson]

Use dev_t instead of a string, and ino_t instead of uint64_t.
The latter is likely to be identical on modern systems but is
more type-correct for usage.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/qemu/selfmap.h |  4 ++--
 linux-user/syscall.c   |  6 ++++--
 util/selfmap.c         | 12 +++++++-----
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/include/qemu/selfmap.h b/include/qemu/selfmap.h
index 7d938945cb..1690a74f4b 100644
--- a/include/qemu/selfmap.h
+++ b/include/qemu/selfmap.h
@@ -20,10 +20,10 @@ typedef struct {
     bool is_exec;
     bool is_priv;
 
+    dev_t dev;
+    ino_t inode;
     uint64_t offset;
-    uint64_t inode;
     const char *path;
-    char dev[];
 } MapInfo;
 
 /**
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9353268cc1..074262b3ac 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8160,13 +8160,15 @@ static int open_self_maps_1(CPUArchState *cpu_env, int fd, bool smaps)
             }
 
             count = dprintf(fd, TARGET_ABI_FMT_ptr "-" TARGET_ABI_FMT_ptr
-                            " %c%c%c%c %08" PRIx64 " %s %"PRId64,
+                            " %c%c%c%c %08" PRIx64 " %02x:%02x %"PRId64,
                             h2g(min), h2g(max - 1) + 1,
                             (flags & PAGE_READ) ? 'r' : '-',
                             (flags & PAGE_WRITE_ORG) ? 'w' : '-',
                             (flags & PAGE_EXEC) ? 'x' : '-',
                             e->is_priv ? 'p' : 's',
-                            (uint64_t) e->offset, e->dev, e->inode);
+                            (uint64_t)e->offset,
+                            major(e->dev), minor(e->dev),
+                            (uint64_t)e->inode);
             if (path) {
                 dprintf(fd, "%*s%s\n", 73 - count, "", path);
             } else {
diff --git a/util/selfmap.c b/util/selfmap.c
index 4db5b42651..483cb617e2 100644
--- a/util/selfmap.c
+++ b/util/selfmap.c
@@ -30,19 +30,21 @@ IntervalTreeRoot *read_self_maps(void)
 
         if (nfields > 4) {
             uint64_t start, end, offset, inode;
+            unsigned dev_maj, dev_min;
             int errors = 0;
             const char *p;
 
             errors |= qemu_strtou64(fields[0], &p, 16, &start);
             errors |= qemu_strtou64(p + 1, NULL, 16, &end);
             errors |= qemu_strtou64(fields[2], NULL, 16, &offset);
+            errors |= qemu_strtoui(fields[3], &p, 16, &dev_maj);
+            errors |= qemu_strtoui(p + 1, NULL, 16, &dev_min);
             errors |= qemu_strtou64(fields[4], NULL, 10, &inode);
 
             if (!errors) {
-                size_t dev_len, path_len;
+                size_t path_len;
                 MapInfo *e;
 
-                dev_len = strlen(fields[3]) + 1;
                 if (nfields == 6) {
                     p = fields[5];
                     p += strspn(p, " ");
@@ -52,11 +54,12 @@ IntervalTreeRoot *read_self_maps(void)
                     path_len = 0;
                 }
 
-                e = g_malloc0(sizeof(*e) + dev_len + path_len);
+                e = g_malloc0(sizeof(*e) + path_len);
 
                 e->itree.start = start;
                 e->itree.last = end - 1;
                 e->offset = offset;
+                e->dev = makedev(dev_maj, dev_min);
                 e->inode = inode;
 
                 e->is_read  = fields[1][0] == 'r';
@@ -64,9 +67,8 @@ IntervalTreeRoot *read_self_maps(void)
                 e->is_exec  = fields[1][2] == 'x';
                 e->is_priv  = fields[1][3] == 'p';
 
-                memcpy(e->dev, fields[3], dev_len);
                 if (path_len) {
-                    e->path = memcpy(e->dev + dev_len, p, path_len);
+                    e->path = memcpy(e + 1, p, path_len);
                 }
 
                 interval_tree_insert(&e->itree, root);
-- 
2.34.1

Re: [PATCH 1/6] util/selfmap: Use dev_t and ino_t in MapInfo

[Philippe Mathieu-Daudé]

On 16/8/23 20:14, Richard Henderson wrote:
> Use dev_t instead of a string, and ino_t instead of uint64_t.
> The latter is likely to be identical on modern systems but is
> more type-correct for usage.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/qemu/selfmap.h |  4 ++--
>   linux-user/syscall.c   |  6 ++++--
>   util/selfmap.c         | 12 +++++++-----
>   3 files changed, 13 insertions(+), 9 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 2/6] linux-user: Use walk_memory_regions for open_self_maps

[Richard Henderson]

Replace the by-hand method of region identification with
the official user-exec interface.  Cross-check the region
provided to the callback with the interval tree from
read_self_maps().

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/syscall.c | 192 ++++++++++++++++++++++++++-----------------
 1 file changed, 115 insertions(+), 77 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 074262b3ac..658c276e39 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8095,12 +8095,66 @@ static int open_self_cmdline(CPUArchState *cpu_env, int fd)
     return 0;
 }
 
-static void show_smaps(int fd, unsigned long size)
-{
-    unsigned long page_size_kb = TARGET_PAGE_SIZE >> 10;
-    unsigned long size_kb = size >> 10;
+struct open_self_maps_data {
+    TaskState *ts;
+    IntervalTreeRoot *host_maps;
+    int fd;
+    bool smaps;
+};
 
-    dprintf(fd, "Size:                  %lu kB\n"
+/*
+ * Subroutine to output one line of /proc/self/maps,
+ * or one region of /proc/self/smaps.
+ */
+
+#ifdef TARGET_HPPA
+# define test_stack(S, E, L)  (E == L)
+#else
+# define test_stack(S, E, L)  (S == L)
+#endif
+
+static void open_self_maps_4(const struct open_self_maps_data *d,
+                             const MapInfo *mi, abi_ptr start,
+                             abi_ptr end, unsigned flags)
+{
+    const struct image_info *info = d->ts->info;
+    const char *path = mi->path;
+    uint64_t offset;
+    int fd = d->fd;
+    int count;
+
+    if (test_stack(start, end, info->stack_limit)) {
+        path = "[stack]";
+    }
+
+    /* Except null device (MAP_ANON), adjust offset for this fragment. */
+    offset = mi->offset;
+    if (mi->dev) {
+        uintptr_t hstart = (uintptr_t)g2h_untagged(start);
+        offset += hstart - mi->itree.start;
+    }
+
+    count = dprintf(fd, TARGET_ABI_FMT_ptr "-" TARGET_ABI_FMT_ptr
+                    " %c%c%c%c %08" PRIx64 " %02x:%02x %"PRId64,
+                    start, end,
+                    (flags & PAGE_READ) ? 'r' : '-',
+                    (flags & PAGE_WRITE_ORG) ? 'w' : '-',
+                    (flags & PAGE_EXEC) ? 'x' : '-',
+                    mi->is_priv ? 'p' : 's',
+                    offset, major(mi->dev), minor(mi->dev),
+                    (uint64_t)mi->inode);
+    if (path) {
+        dprintf(fd, "%*s%s\n", 73 - count, "", path);
+    } else {
+        dprintf(fd, "\n");
+    }
+
+    if (d->smaps) {
+        unsigned long size = end - start;
+        unsigned long page_size_kb = TARGET_PAGE_SIZE >> 10;
+        unsigned long size_kb = size >> 10;
+
+        dprintf(fd, "Size:                  %lu kB\n"
                 "KernelPageSize:        %lu kB\n"
                 "MMUPageSize:           %lu kB\n"
                 "Rss:                   0 kB\n"
@@ -8121,91 +8175,75 @@ static void show_smaps(int fd, unsigned long size)
                 "Swap:                  0 kB\n"
                 "SwapPss:               0 kB\n"
                 "Locked:                0 kB\n"
-                "THPeligible:    0\n", size_kb, page_size_kb, page_size_kb);
+                "THPeligible:    0\n"
+                "VmFlags:%s%s%s%s%s%s%s%s\n",
+                size_kb, page_size_kb, page_size_kb,
+                (flags & PAGE_READ) ? " rd" : "",
+                (flags & PAGE_WRITE_ORG) ? " wr" : "",
+                (flags & PAGE_EXEC) ? " ex" : "",
+                mi->is_priv ? "" : " sh",
+                (flags & PAGE_READ) ? " mr" : "",
+                (flags & PAGE_WRITE_ORG) ? " mw" : "",
+                (flags & PAGE_EXEC) ? " me" : "",
+                mi->is_priv ? "" : " ms");
+    }
 }
 
-static int open_self_maps_1(CPUArchState *cpu_env, int fd, bool smaps)
+/*
+ * Callback for walk_memory_regions, when read_self_maps() fails.
+ * Proceed without the benefit of host /proc/self/maps cross-check.
+ */
+static int open_self_maps_3(void *opaque, target_ulong guest_start,
+                            target_ulong guest_end, unsigned long flags)
 {
-    CPUState *cpu = env_cpu(cpu_env);
-    TaskState *ts = cpu->opaque;
-    IntervalTreeRoot *map_info = read_self_maps();
-    IntervalTreeNode *s;
-    int count;
+    static const MapInfo mi = { .is_priv = true };
 
-    for (s = interval_tree_iter_first(map_info, 0, -1); s;
-         s = interval_tree_iter_next(s, 0, -1)) {
-        MapInfo *e = container_of(s, MapInfo, itree);
+    open_self_maps_4(opaque, &mi, guest_start, guest_end, flags);
+    return 0;
+}
 
-        if (h2g_valid(e->itree.start)) {
-            unsigned long min = e->itree.start;
-            unsigned long max = e->itree.last + 1;
-            int flags = page_get_flags(h2g(min));
-            const char *path;
+/*
+ * Callback for walk_memory_regions, when read_self_maps() succeeds.
+ */
+static int open_self_maps_2(void *opaque, target_ulong guest_start,
+                            target_ulong guest_end, unsigned long flags)
+{
+    const struct open_self_maps_data *d = opaque;
+    uintptr_t host_start = (uintptr_t)g2h_untagged(guest_start);
+    uintptr_t host_last = (uintptr_t)g2h_untagged(guest_end - 1);
 
-            max = h2g_valid(max - 1) ?
-                max : (uintptr_t) g2h_untagged(GUEST_ADDR_MAX) + 1;
+    while (1) {
+        IntervalTreeNode *n =
+            interval_tree_iter_first(d->host_maps, host_start, host_start);
+        MapInfo *mi = container_of(n, MapInfo, itree);
+        uintptr_t this_hlast = MIN(host_last, n->last);
+        target_ulong this_gend = h2g(this_hlast) + 1;
 
-            if (!page_check_range(h2g(min), max - min, flags)) {
-                continue;
-            }
+        open_self_maps_4(d, mi, guest_start, this_gend, flags);
 
-#ifdef TARGET_HPPA
-            if (h2g(max) == ts->info->stack_limit) {
-#else
-            if (h2g(min) == ts->info->stack_limit) {
-#endif
-                path = "[stack]";
-            } else {
-                path = e->path;
-            }
-
-            count = dprintf(fd, TARGET_ABI_FMT_ptr "-" TARGET_ABI_FMT_ptr
-                            " %c%c%c%c %08" PRIx64 " %02x:%02x %"PRId64,
-                            h2g(min), h2g(max - 1) + 1,
-                            (flags & PAGE_READ) ? 'r' : '-',
-                            (flags & PAGE_WRITE_ORG) ? 'w' : '-',
-                            (flags & PAGE_EXEC) ? 'x' : '-',
-                            e->is_priv ? 'p' : 's',
-                            (uint64_t)e->offset,
-                            major(e->dev), minor(e->dev),
-                            (uint64_t)e->inode);
-            if (path) {
-                dprintf(fd, "%*s%s\n", 73 - count, "", path);
-            } else {
-                dprintf(fd, "\n");
-            }
-            if (smaps) {
-                show_smaps(fd, max - min);
-                dprintf(fd, "VmFlags:%s%s%s%s%s%s%s%s\n",
-                        (flags & PAGE_READ) ? " rd" : "",
-                        (flags & PAGE_WRITE_ORG) ? " wr" : "",
-                        (flags & PAGE_EXEC) ? " ex" : "",
-                        e->is_priv ? "" : " sh",
-                        (flags & PAGE_READ) ? " mr" : "",
-                        (flags & PAGE_WRITE_ORG) ? " mw" : "",
-                        (flags & PAGE_EXEC) ? " me" : "",
-                        e->is_priv ? "" : " ms");
-            }
+        if (this_hlast == host_last) {
+            return 0;
         }
+        host_start = this_hlast + 1;
+        guest_start = h2g(host_start);
     }
+}
 
-    free_self_maps(map_info);
+static int open_self_maps_1(CPUArchState *env, int fd, bool smaps)
+{
+    struct open_self_maps_data d = {
+        .ts = env_cpu(env)->opaque,
+        .host_maps = read_self_maps(),
+        .fd = fd,
+        .smaps = smaps
+    };
 
-#ifdef TARGET_VSYSCALL_PAGE
-    /*
-     * We only support execution from the vsyscall page.
-     * This is as if CONFIG_LEGACY_VSYSCALL_XONLY=y from v5.3.
-     */
-    count = dprintf(fd, TARGET_FMT_lx "-" TARGET_FMT_lx
-                    " --xp 00000000 00:00 0",
-                    TARGET_VSYSCALL_PAGE, TARGET_VSYSCALL_PAGE + TARGET_PAGE_SIZE);
-    dprintf(fd, "%*s%s\n", 73 - count, "",  "[vsyscall]");
-    if (smaps) {
-        show_smaps(fd, TARGET_PAGE_SIZE);
-        dprintf(fd, "VmFlags: ex\n");
+    if (d.host_maps) {
+        walk_memory_regions(&d, open_self_maps_2);
+        free_self_maps(d.host_maps);
+    } else {
+        walk_memory_regions(&d, open_self_maps_3);
     }
-#endif
-
     return 0;
 }
 
-- 
2.34.1

[PATCH 3/6] linux-user: Adjust brk for load_bias

[Richard Henderson]

PIE executables are usually linked at offset 0 and are
relocated somewhere during load.  The hiaddr needs to
be adjusted to keep the brk next to the executable.

Cc: qemu-stable@nongnu.org
Fixes: 1f356e8c013 ("linux-user: Adjust initial brk when interpreter is close to executable")
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/elfload.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index ccfbf82836..ab11f141c3 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -3278,7 +3278,7 @@ static void load_elf_image(const char *image_name, const ImageSource *src,
     info->start_data = -1;
     info->end_data = 0;
     /* Usual start for brk is after all sections of the main executable. */
-    info->brk = TARGET_PAGE_ALIGN(hiaddr);
+    info->brk = TARGET_PAGE_ALIGN(hiaddr + load_bias);
     info->elf_flags = ehdr->e_flags;
 
     prot_exec = PROT_EXEC;
-- 
2.34.1

Re: [PATCH 3/6] linux-user: Adjust brk for load_bias

[Philippe Mathieu-Daudé]

On 16/8/23 20:14, Richard Henderson wrote:
> PIE executables are usually linked at offset 0 and are
> relocated somewhere during load.  The hiaddr needs to
> be adjusted to keep the brk next to the executable.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 1f356e8c013 ("linux-user: Adjust initial brk when interpreter is close to executable")
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/elfload.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index ccfbf82836..ab11f141c3 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -3278,7 +3278,7 @@ static void load_elf_image(const char *image_name, const ImageSource *src,
>       info->start_data = -1;
>       info->end_data = 0;
>       /* Usual start for brk is after all sections of the main executable. */
> -    info->brk = TARGET_PAGE_ALIGN(hiaddr);
> +    info->brk = TARGET_PAGE_ALIGN(hiaddr + load_bias);

Did you got some odd behavior or figured that by
code review?

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 3/6] linux-user: Adjust brk for load_bias

[Richard Henderson]

On 8/17/23 01:53, Philippe Mathieu-Daudé wrote:
> On 16/8/23 20:14, Richard Henderson wrote:
>> PIE executables are usually linked at offset 0 and are
>> relocated somewhere during load.  The hiaddr needs to
>> be adjusted to keep the brk next to the executable.
>>
>> Cc: qemu-stable@nongnu.org
>> Fixes: 1f356e8c013 ("linux-user: Adjust initial brk when interpreter is close to 
>> executable")
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   linux-user/elfload.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>> index ccfbf82836..ab11f141c3 100644
>> --- a/linux-user/elfload.c
>> +++ b/linux-user/elfload.c
>> @@ -3278,7 +3278,7 @@ static void load_elf_image(const char *image_name, const 
>> ImageSource *src,
>>       info->start_data = -1;
>>       info->end_data = 0;
>>       /* Usual start for brk is after all sections of the main executable. */
>> -    info->brk = TARGET_PAGE_ALIGN(hiaddr);
>> +    info->brk = TARGET_PAGE_ALIGN(hiaddr + load_bias);
> 
> Did you got some odd behavior or figured that by
> code review?
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Odd behaviour, easily seen by [heap] being weird or missing.


r~

Re: [PATCH 3/6] linux-user: Adjust brk for load_bias

[Michael Tokarev]

16.08.2023 21:14, Richard Henderson wrote:
> PIE executables are usually linked at offset 0 and are
> relocated somewhere during load.  The hiaddr needs to
> be adjusted to keep the brk next to the executable.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 1f356e8c013 ("linux-user: Adjust initial brk when interpreter is close to executable")

FWIW, 1f356e8c013 is v8.1.0-rc2-86, - why did you Cc qemu-stable@?

If this "Adjust brk for load_bias" fix isn't supposed to be part of 8.1.0 release,
sure thing I'll pick it up for stable-8.1, but it looks like it should be in 8.1.0.

Or are you saying 1f356e8c013 should be picked for stable-8.0, together with this one?

(We're yet to decide if stable-8.0 should have any recent linux-user changes).

/mjt

Re: [PATCH 3/6] linux-user: Adjust brk for load_bias

[Richard Henderson]

On 8/17/23 09:04, Michael Tokarev wrote:
> 16.08.2023 21:14, Richard Henderson wrote:
>> PIE executables are usually linked at offset 0 and are
>> relocated somewhere during load.  The hiaddr needs to
>> be adjusted to keep the brk next to the executable.
>>
>> Cc: qemu-stable@nongnu.org
>> Fixes: 1f356e8c013 ("linux-user: Adjust initial brk when interpreter is close to 
>> executable")
> 
> FWIW, 1f356e8c013 is v8.1.0-rc2-86, - why did you Cc qemu-stable@?
> 
> If this "Adjust brk for load_bias" fix isn't supposed to be part of 8.1.0 release,
> sure thing I'll pick it up for stable-8.1, but it looks like it should be in 8.1.0.
> 
> Or are you saying 1f356e8c013 should be picked for stable-8.0, together with this one?
> 
> (We're yet to decide if stable-8.0 should have any recent linux-user changes).

This has missed 8.1.0-rc4 and therefore will not be in 8.1.0.
I have tagged it stable for stable-8.1 for 8.1.1.


r~

Re: [PATCH 3/6] linux-user: Adjust brk for load_bias [regression]

[Dominique MARTINET]

This commit is fairly old, but this appears to cause a segfault for
older versions of ldconfig:
```
$ docker run --rm --platform linux/arm64/v8 -ti docker.io/debian:bullseye-slim ldconfig
qemu: uncaught target signal 11 (Segmentation fault) - core dumped
Segmentation fault (core dumped)
```

The segfault happens inside ldconfig code (code_gen_buffer in qemu's
backtrace), so I'm not sure how to debug that further, but it doesn't
reproduce in bookworm's ldconfig so that is something that was "fixed"
in glibc at some point.

If someone needs to run older debian releases with a newer qemu that
might be a problem in the future?

[we might need to run old containers once every few years to rebuild old
projects in a similar environment they were built on, so would
eventually need to work around this problem somehow]


The failure can be reproduced just running `qemu-aarch64
./path/to/ldconfig` on an extracted container so it was easy to bisect
and I've got down to this commit; hence replying here directly with
involved people.
------
commit aec338d63bc28f1f13d5e64c561d7f1dd0e4b07e
Author: Richard Henderson <richard.henderson@linaro.org>
Date:   Wed Aug 16 10:32:18 2023 -0700

    linux-user: Adjust brk for load_bias
    
    PIE executables are usually linked at offset 0 and are
    relocated somewhere during load.  The hiaddr needs to
    be adjusted to keep the brk next to the executable.
    
    Cc: qemu-stable@nongnu.org
    Fixes: 1f356e8c013 ("linux-user: Adjust initial brk when interpreter is close to executable")
    Tested-by: Helge Deller <deller@gmx.de>
    Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>
    Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
    Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
------

I've done my share of debugging linux-user last week[1] so I'll leave this
as is for now, I've downgraded to (a non-static-pie build of) 7.1 for
our build machine and am not in immediate trouble.
[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1053101

If this doesn't get much interest I might try to pick at it further in
a couple of weeks, assuming it's something we can/want to fix on qemu
side.

Thanks,
-- 
Dominique

Re: [PATCH 3/6] linux-user: Adjust brk for load_bias [regression]

[Ilya Leoshkevich]

On Tue, 2024-11-26 at 16:11 +0900, Dominique MARTINET wrote:
> This commit is fairly old, but this appears to cause a segfault for
> older versions of ldconfig:
> ```
> $ docker run --rm --platform linux/arm64/v8 -ti
> docker.io/debian:bullseye-slim ldconfig
> qemu: uncaught target signal 11 (Segmentation fault) - core dumped
> Segmentation fault (core dumped)
> ```
> 
> The segfault happens inside ldconfig code (code_gen_buffer in qemu's
> backtrace), so I'm not sure how to debug that further, but it doesn't
> reproduce in bookworm's ldconfig so that is something that was
> "fixed"
> in glibc at some point.
> 
> If someone needs to run older debian releases with a newer qemu that
> might be a problem in the future?
> 
> [we might need to run old containers once every few years to rebuild
> old
> projects in a similar environment they were built on, so would
> eventually need to work around this problem somehow]
> 
> 
> The failure can be reproduced just running `qemu-aarch64
> ./path/to/ldconfig` on an extracted container so it was easy to
> bisect
> and I've got down to this commit; hence replying here directly with
> involved people.
> ------
> commit aec338d63bc28f1f13d5e64c561d7f1dd0e4b07e
> Author: Richard Henderson <richard.henderson@linaro.org>
> Date:   Wed Aug 16 10:32:18 2023 -0700
> 
>     linux-user: Adjust brk for load_bias
>     
>     PIE executables are usually linked at offset 0 and are
>     relocated somewhere during load.  The hiaddr needs to
>     be adjusted to keep the brk next to the executable.
>     
>     Cc: qemu-stable@nongnu.org
>     Fixes: 1f356e8c013 ("linux-user: Adjust initial brk when
> interpreter is close to executable")
>     Tested-by: Helge Deller <deller@gmx.de>
>     Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>
>     Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>     Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ------
> 
> I've done my share of debugging linux-user last week[1] so I'll leave
> this
> as is for now, I've downgraded to (a non-static-pie build of) 7.1 for
> our build machine and am not in immediate trouble.
> [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1053101
> 
> If this doesn't get much interest I might try to pick at it further
> in
> a couple of weeks, assuming it's something we can/want to fix on qemu
> side.
> 
> Thanks,

Hi,

I think this is
https://gitlab.com/qemu-project/qemu/-/issues/1913

Best regards,
Ilya

Re: [PATCH 3/6] linux-user: Adjust brk for load_bias [regression]

[Dominique MARTINET]

Ilya Leoshkevich wrote on Tue, Nov 26, 2024 at 10:24:12AM +0100:
> I think this is
> https://gitlab.com/qemu-project/qemu/-/issues/1913

Thank you, I should have looked there first !

I'll continue to follow-up on the issue if time permits as it doesn't
look like this has had progress in the past few months

-- 
Dominique

[PATCH 4/6] linux-user: Show heap address in /proc/pid/maps

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/syscall.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 658c276e39..5c0fb20e19 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8125,6 +8125,8 @@ static void open_self_maps_4(const struct open_self_maps_data *d,
 
     if (test_stack(start, end, info->stack_limit)) {
         path = "[stack]";
+    } else if (start == info->brk) {
+        path = "[heap]";
     }
 
     /* Except null device (MAP_ANON), adjust offset for this fragment. */
-- 
2.34.1

Re: [PATCH 4/6] linux-user: Show heap address in /proc/pid/maps

[Philippe Mathieu-Daudé]

On 16/8/23 20:14, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/syscall.c | 2 ++
>   1 file changed, 2 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 5/6] linux-user: Remove ELF_START_MMAP and image_info.start_mmap

[Richard Henderson]

The start_mmap value is write-only.
Remove the field and the defines that populated it.
Logically, this has been replaced by task_unmapped_base.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/qemu.h    |  1 -
 linux-user/elfload.c | 38 --------------------------------------
 2 files changed, 39 deletions(-)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 4f8b55e2fb..12f638336a 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -30,7 +30,6 @@ struct image_info {
         abi_ulong       start_data;
         abi_ulong       end_data;
         abi_ulong       brk;
-        abi_ulong       start_mmap;
         abi_ulong       start_stack;
         abi_ulong       stack_limit;
         abi_ulong       entry;
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index ab11f141c3..a670a7817a 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -156,8 +156,6 @@ static uint32_t get_elf_hwcap(void)
 }
 
 #ifdef TARGET_X86_64
-#define ELF_START_MMAP 0x2aaaaab000ULL
-
 #define ELF_CLASS      ELFCLASS64
 #define ELF_ARCH       EM_X86_64
 
@@ -234,8 +232,6 @@ static bool init_guest_commpage(void)
 #endif
 #else
 
-#define ELF_START_MMAP 0x80000000
-
 /*
  * This is used to ensure we don't load something for the wrong architecture.
  */
@@ -333,8 +329,6 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUX86State *en
 #ifndef TARGET_AARCH64
 /* 32 bit ARM definitions */
 
-#define ELF_START_MMAP 0x80000000
-
 #define ELF_ARCH        EM_ARM
 #define ELF_CLASS       ELFCLASS32
 #define EXSTACK_DEFAULT true
@@ -606,7 +600,6 @@ static const VdsoImageInfo *vdso_image_info(void)
 
 #else
 /* 64 bit ARM definitions */
-#define ELF_START_MMAP 0x80000000
 
 #define ELF_ARCH        EM_AARCH64
 #define ELF_CLASS       ELFCLASS64
@@ -802,7 +795,6 @@ static uint32_t get_elf_hwcap2(void)
 #ifdef TARGET_SPARC
 #ifdef TARGET_SPARC64
 
-#define ELF_START_MMAP 0x80000000
 #define ELF_HWCAP  (HWCAP_SPARC_FLUSH | HWCAP_SPARC_STBAR | HWCAP_SPARC_SWAP \
                     | HWCAP_SPARC_MULDIV | HWCAP_SPARC_V9)
 #ifndef TARGET_ABI32
@@ -814,7 +806,6 @@ static uint32_t get_elf_hwcap2(void)
 #define ELF_CLASS   ELFCLASS64
 #define ELF_ARCH    EM_SPARCV9
 #else
-#define ELF_START_MMAP 0x80000000
 #define ELF_HWCAP  (HWCAP_SPARC_FLUSH | HWCAP_SPARC_STBAR | HWCAP_SPARC_SWAP \
                     | HWCAP_SPARC_MULDIV)
 #define ELF_CLASS   ELFCLASS32
@@ -836,7 +827,6 @@ static inline void init_thread(struct target_pt_regs *regs,
 #ifdef TARGET_PPC
 
 #define ELF_MACHINE    PPC_ELF_MACHINE
-#define ELF_START_MMAP 0x80000000
 
 #if defined(TARGET_PPC64)
 
@@ -1048,8 +1038,6 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUPPCState *en
 
 #ifdef TARGET_LOONGARCH64
 
-#define ELF_START_MMAP 0x80000000
-
 #define ELF_CLASS   ELFCLASS64
 #define ELF_ARCH    EM_LOONGARCH
 #define EXSTACK_DEFAULT true
@@ -1144,8 +1132,6 @@ static uint32_t get_elf_hwcap(void)
 
 #ifdef TARGET_MIPS
 
-#define ELF_START_MMAP 0x80000000
-
 #ifdef TARGET_MIPS64
 #define ELF_CLASS   ELFCLASS64
 #else
@@ -1303,8 +1289,6 @@ static uint32_t get_elf_hwcap(void)
 
 #ifdef TARGET_MICROBLAZE
 
-#define ELF_START_MMAP 0x80000000
-
 #define elf_check_arch(x) ( (x) == EM_MICROBLAZE || (x) == EM_MICROBLAZE_OLD)
 
 #define ELF_CLASS   ELFCLASS32
@@ -1345,8 +1329,6 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUMBState *env
 
 #ifdef TARGET_NIOS2
 
-#define ELF_START_MMAP 0x80000000
-
 #define elf_check_arch(x) ((x) == EM_ALTERA_NIOS2)
 
 #define ELF_CLASS   ELFCLASS32
@@ -1442,8 +1424,6 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs,
 
 #ifdef TARGET_OPENRISC
 
-#define ELF_START_MMAP 0x08000000
-
 #define ELF_ARCH EM_OPENRISC
 #define ELF_CLASS ELFCLASS32
 #define ELF_DATA  ELFDATA2MSB
@@ -1480,8 +1460,6 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs,
 
 #ifdef TARGET_SH4
 
-#define ELF_START_MMAP 0x80000000
-
 #define ELF_CLASS ELFCLASS32
 #define ELF_ARCH  EM_SH
 
@@ -1562,8 +1540,6 @@ static uint32_t get_elf_hwcap(void)
 
 #ifdef TARGET_CRIS
 
-#define ELF_START_MMAP 0x80000000
-
 #define ELF_CLASS ELFCLASS32
 #define ELF_ARCH  EM_CRIS
 
@@ -1579,8 +1555,6 @@ static inline void init_thread(struct target_pt_regs *regs,
 
 #ifdef TARGET_M68K
 
-#define ELF_START_MMAP 0x80000000
-
 #define ELF_CLASS       ELFCLASS32
 #define ELF_ARCH        EM_68K
 
@@ -1630,8 +1604,6 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUM68KState *e
 
 #ifdef TARGET_ALPHA
 
-#define ELF_START_MMAP (0x30000000000ULL)
-
 #define ELF_CLASS      ELFCLASS64
 #define ELF_ARCH       EM_ALPHA
 
@@ -1649,8 +1621,6 @@ static inline void init_thread(struct target_pt_regs *regs,
 
 #ifdef TARGET_S390X
 
-#define ELF_START_MMAP (0x20000000000ULL)
-
 #define ELF_CLASS	ELFCLASS64
 #define ELF_DATA	ELFDATA2MSB
 #define ELF_ARCH	EM_S390
@@ -1763,7 +1733,6 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs,
 
 #ifdef TARGET_RISCV
 
-#define ELF_START_MMAP 0x80000000
 #define ELF_ARCH  EM_RISCV
 
 #ifdef TARGET_RISCV32
@@ -1803,7 +1772,6 @@ static inline void init_thread(struct target_pt_regs *regs,
 
 #ifdef TARGET_HPPA
 
-#define ELF_START_MMAP  0x80000000
 #define ELF_CLASS       ELFCLASS32
 #define ELF_ARCH        EM_PARISC
 #define ELF_PLATFORM    "PARISC"
@@ -1859,8 +1827,6 @@ static bool init_guest_commpage(void)
 
 #ifdef TARGET_XTENSA
 
-#define ELF_START_MMAP 0x20000000
-
 #define ELF_CLASS       ELFCLASS32
 #define ELF_ARCH        EM_XTENSA
 
@@ -1926,8 +1892,6 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs,
 
 #ifdef TARGET_HEXAGON
 
-#define ELF_START_MMAP 0x20000000
-
 #define ELF_CLASS       ELFCLASS32
 #define ELF_ARCH        EM_HEXAGON
 
@@ -3684,8 +3648,6 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info)
     interp_info.fp_abi = MIPS_ABI_FP_UNKNOWN;
 #endif
 
-    info->start_mmap = (abi_ulong)ELF_START_MMAP;
-
     load_elf_image(bprm->filename, &bprm->src, info, &ehdr, &elf_interpreter);
 
     /* Do this so that we can load the interpreter, if need be.  We will
-- 
2.34.1

Re: [PATCH 5/6] linux-user: Remove ELF_START_MMAP and image_info.start_mmap

[Philippe Mathieu-Daudé]

On 16/8/23 20:14, Richard Henderson wrote:
> The start_mmap value is write-only.
> Remove the field and the defines that populated it.
> Logically, this has been replaced by task_unmapped_base.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/qemu.h    |  1 -
>   linux-user/elfload.c | 38 --------------------------------------
>   2 files changed, 39 deletions(-)

Can we squash similar removal in bsd-user?
Either that or in a different patch:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 5/6] linux-user: Remove ELF_START_MMAP and image_info.start_mmap

[Richard Henderson]

On 8/17/23 02:00, Philippe Mathieu-Daudé wrote:
> On 16/8/23 20:14, Richard Henderson wrote:
>> The start_mmap value is write-only.
>> Remove the field and the defines that populated it.
>> Logically, this has been replaced by task_unmapped_base.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   linux-user/qemu.h    |  1 -
>>   linux-user/elfload.c | 38 --------------------------------------
>>   2 files changed, 39 deletions(-)
> 
> Can we squash similar removal in bsd-user?
> Either that or in a different patch:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 

A different patch, for sure.  I don't want trivial patches to interfere with the ongoing 
merge process.


r~

Re: [PATCH 5/6] linux-user: Remove ELF_START_MMAP and image_info.start_mmap

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 1048 bytes --]

On Thu, Aug 17, 2023 at 6:19 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 8/17/23 02:00, Philippe Mathieu-Daudé wrote:
> > On 16/8/23 20:14, Richard Henderson wrote:
> >> The start_mmap value is write-only.
> >> Remove the field and the defines that populated it.
> >> Logically, this has been replaced by task_unmapped_base.
> >>
> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >> ---
> >>   linux-user/qemu.h    |  1 -
> >>   linux-user/elfload.c | 38 --------------------------------------
> >>   2 files changed, 39 deletions(-)
> >
> > Can we squash similar removal in bsd-user?
> > Either that or in a different patch:
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >
>
> A different patch, for sure.  I don't want trivial patches to interfere
> with the ongoing
> merge process.
>

CC me on the patch. I'll queue it with the other patches that have been
reviewed and act
as conductor to make sure there's no interference with ongoing work.

Warner

[-- Attachment #2: Type: text/html, Size: 1668 bytes --]

[PATCH 6/6] linux-user: Show vdso address in /proc/pid/maps

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/qemu.h    | 1 +
 linux-user/elfload.c | 1 +
 linux-user/syscall.c | 2 ++
 3 files changed, 4 insertions(+)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 12f638336a..4de9ec783f 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -32,6 +32,7 @@ struct image_info {
         abi_ulong       brk;
         abi_ulong       start_stack;
         abi_ulong       stack_limit;
+        abi_ulong       vdso;
         abi_ulong       entry;
         abi_ulong       code_offset;
         abi_ulong       data_offset;
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index a670a7817a..12285eae82 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -3726,6 +3726,7 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info)
     const VdsoImageInfo *vdso = vdso_image_info();
     if (vdso) {
         load_elf_vdso(&vdso_info, vdso);
+        info->vdso = vdso_info.load_bias;
     } else if (TARGET_ARCH_HAS_SIGTRAMP_PAGE) {
         abi_long tramp_page = target_mmap(0, TARGET_PAGE_SIZE,
                                           PROT_READ | PROT_WRITE,
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 5c0fb20e19..c85cf6ffb9 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8127,6 +8127,8 @@ static void open_self_maps_4(const struct open_self_maps_data *d,
         path = "[stack]";
     } else if (start == info->brk) {
         path = "[heap]";
+    } else if (start == info->vdso) {
+        path = "[vdso]";
     }
 
     /* Except null device (MAP_ANON), adjust offset for this fragment. */
-- 
2.34.1

Re: [PATCH 6/6] linux-user: Show vdso address in /proc/pid/maps

[Philippe Mathieu-Daudé]

On 16/8/23 20:14, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/qemu.h    | 1 +
>   linux-user/elfload.c | 1 +
>   linux-user/syscall.c | 2 ++
>   3 files changed, 4 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 0/6] linux-user: Rewrite open_self_maps

[Helge Deller]

Hi Richard,

On 8/16/23 20:14, Richard Henderson wrote:
> Based-on: 20230816180338.572576-1-richard.henderson@linaro.org
> ("[PATCH v4 00/18] linux-user: Implement VDSOs")
>
> As promised, a rewrite of /proc/self/{maps,smaps} emulation
> using interval trees.
>
> Incorporate Helge's change to mark [heap], and also mark [vdso].

Series looks good, so you may add

Tested-by: Helge Deller <deller@gmx.de>

to this series and the previous one (linux-user: Implement VDSOs).


The only thing I noticed is, that mips64el doesn't seem to have heap?

mips64el-chroot
Linux p100 6.4.10-200.fc38.x86_64 #1 SMP PREEMPT_DYNAMIC Fri Aug 11 12:20:29 UTC 2023 mips64 GNU/Linux
555555556000-555555557000 ---p 00000000 00:00 0
555555557000-555555d57000 rwxp 00000000 00:00 0                          [stack]
555555d57000-555555d84000 r-xp 00000000 fd:00 806056                     /usr/lib/mips64el-linux-gnuabi64/ld.so.1
555555d84000-555555d96000 ---p 00000000 00:00 0
555555d96000-555555d97000 r--p 0002f000 fd:00 806056                     /usr/lib/mips64el-linux-gnuabi64/ld.so.1
555555d97000-555555d99000 rw-p 00030000 fd:00 806056                     /usr/lib/mips64el-linux-gnuabi64/ld.so.1
555555d99000-555555d9a000 r-xp 00000000 00:00 0
555555d9a000-555555d9c000 rw-p 00000000 00:00 0
555555da0000-555555f8a000 r-xp 00000000 fd:00 806059                     /usr/lib/mips64el-linux-gnuabi64/libc.so.6
555555f8a000-555555f9a000 ---p 001ea000 fd:00 806059                     /usr/lib/mips64el-linux-gnuabi64/libc.so.6
555555f9a000-555555fa0000 r--p 001ea000 fd:00 806059                     /usr/lib/mips64el-linux-gnuabi64/libc.so.6
555555fa0000-555555fa5000 rw-p 001f0000 fd:00 806059                     /usr/lib/mips64el-linux-gnuabi64/libc.so.6
555555fa5000-555555fb2000 rw-p 00000000 00:00 0
555555fbe000-5555560c0000 rw-p 00000000 00:00 0
7f9bc9987000-7f9bc9992000 r-xp 00000000 fd:00 811277                     /usr/bin/cat
7f9bc9992000-7f9bc99a6000 ---p 00000000 00:00 0
7f9bc99a6000-7f9bc99a7000 r--p 0000f000 fd:00 811277                     /usr/bin/cat
7f9bc99a7000-7f9bc99a8000 rw-p 00010000 fd:00 811277                     /usr/bin/cat

Helge

Re: [PATCH 0/6] linux-user: Rewrite open_self_maps

[Ilya Leoshkevich]

On Wed, Aug 16, 2023 at 11:14:31AM -0700, Richard Henderson wrote:
> Based-on: 20230816180338.572576-1-richard.henderson@linaro.org
> ("[PATCH v4 00/18] linux-user: Implement VDSOs")
> 
> As promised, a rewrite of /proc/self/{maps,smaps} emulation
> using interval trees.
> 
> Incorporate Helge's change to mark [heap], and also mark [vdso].
> 
> 
> r~
> 
> 
> Richard Henderson (6):
>   util/selfmap: Use dev_t and ino_t in MapInfo
>   linux-user: Use walk_memory_regions for open_self_maps
>   linux-user: Adjust brk for load_bias
>   linux-user: Show heap address in /proc/pid/maps
>   linux-user: Remove ELF_START_MMAP and image_info.start_mmap
>   linux-user: Show vdso address in /proc/pid/maps
> 
>  include/qemu/selfmap.h |   4 +-
>  linux-user/qemu.h      |   2 +-
>  linux-user/elfload.c   |  41 +--------
>  linux-user/syscall.c   | 194 +++++++++++++++++++++++++----------------
>  util/selfmap.c         |  12 +--
>  5 files changed, 131 insertions(+), 122 deletions(-)
> 
> -- 
> 2.34.1

As expected, this improved the situation with mappings on ppc64le.
Handling the errors from read_self_maps() is also a nice addition.

Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>