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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Hi Helge,

On 2/8/23 01:27, Helge Deller wrote:
> Show the memory location of the heap in the /proc/pid/maps file inside
> the guest. Store the heap address in ts->heap_base, which requires to
> make that variable accessible for all guest architectures, not just
> architectures for semihosted binaries (arm, m68k, riscv).
> 
> Note that /proc/pid/maps in the guest needs to show target-aligned
> addresses. This is fixed in this patch, so now the heap and stack
> address for architectures like sparc64 and alpha now show up in that
> output as well.
> 
> Show 32- and 64-bit pointers with 8 digits and leading zeros (%08x/%08lx).
> For 64-bit we could use %16lx, but we mimic the Linux kernel, which shows
> even 64-bit addresses with %08lx.

You are describing 3 changes, do you mind splitting in 3 patches?
Otherwise LGTM.

> Example:
> 
> user@machine:/# uname -a
> Linux paq 5.15.88+ #47 SMP Sun Jan 15 12:53:11 CET 2023 aarch64 GNU/Linux
> 
> user@machine:/# cat /proc/self/maps
> Linux p100 6.4.4-200.fc38.x86_64 #1 SMP PREEMPT_DYNAMIC Wed Jul 19 16:32:49 UTC 2023 aarch64 GNU/Linux
> 5500000000-5500009000 r-xp 00000000 fd:00 570430                         /usr/bin/cat
> 5500009000-550001f000 ---p 00000000 00:00 0
> 550001f000-5500020000 r--p 0000f000 fd:00 570430                         /usr/bin/cat
> 5500020000-5500021000 rw-p 00010000 fd:00 570430                         /usr/bin/cat
> 5500021000-5500042000 rw-p 00000000 00:00 0                              [heap]
> 7000000000-7000001000 ---p 00000000 00:00 0
> 7000001000-7000801000 rw-p 00000000 00:00 0                              [stack]
> 7000801000-7000827000 r-xp 00000000 fd:00 571555                         /usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1
> 7000827000-700083f000 ---p 00000000 00:00 0
> 700083f000-7000841000 r--p 0002e000 fd:00 571555                         /usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1
> 7000841000-7000843000 rw-p 00030000 fd:00 571555                         /usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1
> 7000843000-7000844000 r-xp 00000000 00:00 0
> 7000844000-7000846000 rw-p 00000000 00:00 0
> 7000850000-70009d7000 r-xp 00000000 fd:00 571558                         /usr/lib/aarch64-linux-gnu/libc.so.6
> 70009d7000-70009ed000 ---p 00187000 fd:00 571558                         /usr/lib/aarch64-linux-gnu/libc.so.6
> 70009ed000-70009f0000 r--p 0018d000 fd:00 571558                         /usr/lib/aarch64-linux-gnu/libc.so.6
> 70009f0000-70009f2000 rw-p 00190000 fd:00 571558                         /usr/lib/aarch64-linux-gnu/libc.so.6
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> ---
>   include/exec/cpu_ldst.h |  4 ++--
>   linux-user/main.c       |  2 ++
>   linux-user/qemu.h       |  4 ++--
>   linux-user/syscall.c    | 13 +++++++++----
>   4 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
> index 645476f0e5..f1e6f31e88 100644
> --- a/include/exec/cpu_ldst.h
> +++ b/include/exec/cpu_ldst.h
> @@ -72,10 +72,10 @@
>    */
>   #if TARGET_VIRT_ADDR_SPACE_BITS <= 32
>   typedef uint32_t abi_ptr;
> -#define TARGET_ABI_FMT_ptr "%x"
> +#define TARGET_ABI_FMT_ptr "%08x"
>   #else
>   typedef uint64_t abi_ptr;
> -#define TARGET_ABI_FMT_ptr "%"PRIx64
> +#define TARGET_ABI_FMT_ptr "%08"PRIx64
>   #endif

This is patch #1,

> 
>   #ifndef TARGET_TAGGED_ADDRESSES
> diff --git a/linux-user/main.c b/linux-user/main.c
> index dba67ffa36..fa6e47510f 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -946,6 +946,7 @@ int main(int argc, char **argv, char **envp)
>           }
>       }
> 
> +    info->brk = TARGET_PAGE_ALIGN(info->brk);

Patch #3,

>       target_set_brk(info->brk);
>       syscall_init();
>       signal_init();
> @@ -955,6 +956,7 @@ int main(int argc, char **argv, char **envp)
>          the real value of GUEST_BASE into account.  */
>       tcg_prologue_init(tcg_ctx);
> 
> +    ts->heap_base = info->brk;

Patch #3,

>       target_cpu_copy_regs(env, regs);
> 
>       if (gdbstub) {
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index 802794db63..7a6adac637 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -121,11 +121,11 @@ typedef struct TaskState {
>   #ifdef TARGET_M68K
>       abi_ulong tp_value;
>   #endif
> -#if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_RISCV)
> +

Patch #3,

>       /* Extra fields for semihosted binaries.  */
>       abi_ulong heap_base;
>       abi_ulong heap_limit;
> -#endif
> +
>       abi_ulong stack_base;
>       int used; /* non zero if used */
>       struct image_info *info;
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 475260b7ce..dc8266c073 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -8078,8 +8078,9 @@ static int open_self_maps_1(CPUArchState *cpu_env, int fd, bool smaps)
>           MapInfo *e = (MapInfo *) s->data;
> 
>           if (h2g_valid(e->start)) {
> -            unsigned long min = e->start;
> -            unsigned long max = e->end;
> +            /* show page granularity of guest in /proc/pid/maps */
> +            unsigned long min = TARGET_PAGE_ALIGN(e->start);
> +            unsigned long max = TARGET_PAGE_ALIGN(e->end);

Patch #2,

>               int flags = page_get_flags(h2g(min));
>               const char *path;
> 
> @@ -8090,14 +8091,18 @@ static int open_self_maps_1(CPUArchState *cpu_env, int fd, bool smaps)
>                   continue;
>               }
> 
> +            path = e->path;
> +
> +            if (ts->heap_base && h2g(min) == ts->heap_base) {
> +                path = "[heap]";
> +            }

Patch #3 but see below,

>   #ifdef TARGET_HPPA
>               if (h2g(max) == ts->info->stack_limit) {
>   #else
>               if (h2g(min) == ts->info->stack_limit) {
>   #endif
>                   path = "[stack]";

                } else if (ts->heap_base && h2g(min) == ts->heap_base) {
                     path = "[heap]";

> -            } else {
> -                path = e->path;
>               }
> 
>               count = dprintf(fd, TARGET_ABI_FMT_ptr "-" TARGET_ABI_FMT_ptr
> --
> 2.41.0
> 
>