diff mbox

translate-all.c: memory walker initial address miscalculation

Message ID 1410159082-24363-1-git-send-email-m.ilin@samsung.com
State New
Headers show

Commit Message

Mikhail Ilin Sept. 8, 2014, 6:51 a.m. UTC
From: Mikhail Ilyin <m.ilin@samsung.com>

The initial base address is miscalculated in walk_memory_regions().
It has to be shifted TARGET_PAGE_BITS more. Holder variables are
extended to target_ulong size otherwise they don't fit for MIPS N32
(a 32-bit ABI with a 64-bit address space) and qemu won't compile.
The issue led to incorrect debug output of memory maps and a
mis-formed coredumped file.

Signed-off-by: Mikhail Ilyin <m.ilin@samsung.com>
---
 include/exec/cpu-all.h |  4 ++--
 linux-user/elfload.c   | 18 +++++++++---------
 translate-all.c        | 27 +++++++++++++--------------
 3 files changed, 24 insertions(+), 25 deletions(-)

Comments

Michael S. Tsirkin Sept. 30, 2014, 2:16 p.m. UTC | #1
On Mon, Sep 08, 2014 at 10:51:22AM +0400, m.ilin@samsung.com wrote:
> From: Mikhail Ilyin <m.ilin@samsung.com>
> 
> The initial base address is miscalculated in walk_memory_regions().
> It has to be shifted TARGET_PAGE_BITS more. Holder variables are
> extended to target_ulong size otherwise they don't fit for MIPS N32
> (a 32-bit ABI with a 64-bit address space) and qemu won't compile.
> The issue led to incorrect debug output of memory maps and a
> mis-formed coredumped file.
> 
> Signed-off-by: Mikhail Ilyin <m.ilin@samsung.com>

I'd prefer for someone who knows about linux-user to
apply this, but it has been too long since this was
posted.

This seems to make sense superficially.

Acked-by: Michael S. Tsirkin <mst@redhat.com>

Riku?

> ---
>  include/exec/cpu-all.h |  4 ++--
>  linux-user/elfload.c   | 18 +++++++++---------
>  translate-all.c        | 27 +++++++++++++--------------
>  3 files changed, 24 insertions(+), 25 deletions(-)
> 
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index f9d132f..c085804 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -232,8 +232,8 @@ extern uintptr_t qemu_host_page_mask;
>  #if defined(CONFIG_USER_ONLY)
>  void page_dump(FILE *f);
>  
> -typedef int (*walk_memory_regions_fn)(void *, abi_ulong,
> -                                      abi_ulong, unsigned long);
> +typedef int (*walk_memory_regions_fn)(void *, target_ulong,
> +                                      target_ulong, unsigned long);
>  int walk_memory_regions(void *, walk_memory_regions_fn);
>  
>  int page_get_flags(target_ulong address);
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index bea803b..1c04fcf 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2355,9 +2355,9 @@ struct elf_note_info {
>  };
>  
>  struct vm_area_struct {
> -    abi_ulong   vma_start;  /* start vaddr of memory region */
> -    abi_ulong   vma_end;    /* end vaddr of memory region */
> -    abi_ulong   vma_flags;  /* protection etc. flags for the region */
> +    target_ulong   vma_start;  /* start vaddr of memory region */
> +    target_ulong   vma_end;    /* end vaddr of memory region */
> +    abi_ulong      vma_flags;  /* protection etc. flags for the region */
>      QTAILQ_ENTRY(vm_area_struct) vma_link;
>  };
>  
> @@ -2368,13 +2368,13 @@ struct mm_struct {
>  
>  static struct mm_struct *vma_init(void);
>  static void vma_delete(struct mm_struct *);
> -static int vma_add_mapping(struct mm_struct *, abi_ulong,
> -                           abi_ulong, abi_ulong);
> +static int vma_add_mapping(struct mm_struct *, target_ulong,
> +                           target_ulong, abi_ulong);
>  static int vma_get_mapping_count(const struct mm_struct *);
>  static struct vm_area_struct *vma_first(const struct mm_struct *);
>  static struct vm_area_struct *vma_next(struct vm_area_struct *);
>  static abi_ulong vma_dump_size(const struct vm_area_struct *);
> -static int vma_walker(void *priv, abi_ulong start, abi_ulong end,
> +static int vma_walker(void *priv, target_ulong start, target_ulong end,
>                        unsigned long flags);
>  
>  static void fill_elf_header(struct elfhdr *, int, uint16_t, uint32_t);
> @@ -2466,8 +2466,8 @@ static void vma_delete(struct mm_struct *mm)
>      g_free(mm);
>  }
>  
> -static int vma_add_mapping(struct mm_struct *mm, abi_ulong start,
> -                           abi_ulong end, abi_ulong flags)
> +static int vma_add_mapping(struct mm_struct *mm, target_ulong start,
> +                           target_ulong end, abi_ulong flags)
>  {
>      struct vm_area_struct *vma;
>  
> @@ -2535,7 +2535,7 @@ static abi_ulong vma_dump_size(const struct vm_area_struct *vma)
>      return (vma->vma_end - vma->vma_start);
>  }
>  
> -static int vma_walker(void *priv, abi_ulong start, abi_ulong end,
> +static int vma_walker(void *priv, target_ulong start, target_ulong end,
>                        unsigned long flags)
>  {
>      struct mm_struct *mm = (struct mm_struct *)priv;
> diff --git a/translate-all.c b/translate-all.c
> index 2e0265a..885a4ec 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -1660,12 +1660,12 @@ void cpu_interrupt(CPUState *cpu, int mask)
>  struct walk_memory_regions_data {
>      walk_memory_regions_fn fn;
>      void *priv;
> -    uintptr_t start;
> +    target_ulong start;
>      int prot;
>  };
>  
>  static int walk_memory_regions_end(struct walk_memory_regions_data *data,
> -                                   abi_ulong end, int new_prot)
> +                                   target_ulong end, int new_prot)
>  {
>      if (data->start != -1ul) {
>          int rc = data->fn(data->priv, data->start, end, data->prot);
> @@ -1681,9 +1681,9 @@ static int walk_memory_regions_end(struct walk_memory_regions_data *data,
>  }
>  
>  static int walk_memory_regions_1(struct walk_memory_regions_data *data,
> -                                 abi_ulong base, int level, void **lp)
> +                                 target_ulong base, int level, void **lp)
>  {
> -    abi_ulong pa;
> +    target_ulong pa;
>      int i, rc;
>  
>      if (*lp == NULL) {
> @@ -1708,7 +1708,7 @@ static int walk_memory_regions_1(struct walk_memory_regions_data *data,
>          void **pp = *lp;
>  
>          for (i = 0; i < V_L2_SIZE; ++i) {
> -            pa = base | ((abi_ulong)i <<
> +            pa = base | ((target_ulong)i <<
>                  (TARGET_PAGE_BITS + V_L2_BITS * level));
>              rc = walk_memory_regions_1(data, pa, level - 1, pp + i);
>              if (rc != 0) {
> @@ -1731,9 +1731,8 @@ int walk_memory_regions(void *priv, walk_memory_regions_fn fn)
>      data.prot = 0;
>  
>      for (i = 0; i < V_L1_SIZE; i++) {
> -        int rc = walk_memory_regions_1(&data, (abi_ulong)i << V_L1_SHIFT,
> +        int rc = walk_memory_regions_1(&data, (target_ulong)i << (V_L1_SHIFT + TARGET_PAGE_BITS),
>                                         V_L1_SHIFT / V_L2_BITS - 1, l1_map + i);
> -
>          if (rc != 0) {
>              return rc;
>          }
> @@ -1742,13 +1741,13 @@ int walk_memory_regions(void *priv, walk_memory_regions_fn fn)
>      return walk_memory_regions_end(&data, 0, 0);
>  }
>  
> -static int dump_region(void *priv, abi_ulong start,
> -    abi_ulong end, unsigned long prot)
> +static int dump_region(void *priv, target_ulong start,
> +    target_ulong end, unsigned long prot)
>  {
>      FILE *f = (FILE *)priv;
>  
> -    (void) fprintf(f, TARGET_ABI_FMT_lx"-"TARGET_ABI_FMT_lx
> -        " "TARGET_ABI_FMT_lx" %c%c%c\n",
> +    (void) fprintf(f, TARGET_FMT_lx"-"TARGET_FMT_lx
> +        " "TARGET_FMT_lx" %c%c%c\n",
>          start, end, end - start,
>          ((prot & PAGE_READ) ? 'r' : '-'),
>          ((prot & PAGE_WRITE) ? 'w' : '-'),
> @@ -1760,7 +1759,7 @@ static int dump_region(void *priv, abi_ulong start,
>  /* dump memory mappings */
>  void page_dump(FILE *f)
>  {
> -    const int length = sizeof(abi_ulong) * 2;
> +    const int length = sizeof(target_ulong) * 2;
>      (void) fprintf(f, "%-*s %-*s %-*s %s\n",
>              length, "start", length, "end", length, "size", "prot");
>      walk_memory_regions(f, dump_region);
> @@ -1788,7 +1787,7 @@ void page_set_flags(target_ulong start, target_ulong end, int flags)
>         guest address space.  If this assert fires, it probably indicates
>         a missing call to h2g_valid.  */
>  #if TARGET_ABI_BITS > L1_MAP_ADDR_SPACE_BITS
> -    assert(end < ((abi_ulong)1 << L1_MAP_ADDR_SPACE_BITS));
> +    assert(end < ((target_ulong)1 << L1_MAP_ADDR_SPACE_BITS));
>  #endif
>      assert(start < end);
>  
> @@ -1825,7 +1824,7 @@ int page_check_range(target_ulong start, target_ulong len, int flags)
>         guest address space.  If this assert fires, it probably indicates
>         a missing call to h2g_valid.  */
>  #if TARGET_ABI_BITS > L1_MAP_ADDR_SPACE_BITS
> -    assert(start < ((abi_ulong)1 << L1_MAP_ADDR_SPACE_BITS));
> +    assert(start < ((target_ulong)1 << L1_MAP_ADDR_SPACE_BITS));
>  #endif
>  
>      if (len == 0) {
> -- 
> 1.9.1
diff mbox

Patch

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index f9d132f..c085804 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -232,8 +232,8 @@  extern uintptr_t qemu_host_page_mask;
 #if defined(CONFIG_USER_ONLY)
 void page_dump(FILE *f);
 
-typedef int (*walk_memory_regions_fn)(void *, abi_ulong,
-                                      abi_ulong, unsigned long);
+typedef int (*walk_memory_regions_fn)(void *, target_ulong,
+                                      target_ulong, unsigned long);
 int walk_memory_regions(void *, walk_memory_regions_fn);
 
 int page_get_flags(target_ulong address);
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index bea803b..1c04fcf 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2355,9 +2355,9 @@  struct elf_note_info {
 };
 
 struct vm_area_struct {
-    abi_ulong   vma_start;  /* start vaddr of memory region */
-    abi_ulong   vma_end;    /* end vaddr of memory region */
-    abi_ulong   vma_flags;  /* protection etc. flags for the region */
+    target_ulong   vma_start;  /* start vaddr of memory region */
+    target_ulong   vma_end;    /* end vaddr of memory region */
+    abi_ulong      vma_flags;  /* protection etc. flags for the region */
     QTAILQ_ENTRY(vm_area_struct) vma_link;
 };
 
@@ -2368,13 +2368,13 @@  struct mm_struct {
 
 static struct mm_struct *vma_init(void);
 static void vma_delete(struct mm_struct *);
-static int vma_add_mapping(struct mm_struct *, abi_ulong,
-                           abi_ulong, abi_ulong);
+static int vma_add_mapping(struct mm_struct *, target_ulong,
+                           target_ulong, abi_ulong);
 static int vma_get_mapping_count(const struct mm_struct *);
 static struct vm_area_struct *vma_first(const struct mm_struct *);
 static struct vm_area_struct *vma_next(struct vm_area_struct *);
 static abi_ulong vma_dump_size(const struct vm_area_struct *);
-static int vma_walker(void *priv, abi_ulong start, abi_ulong end,
+static int vma_walker(void *priv, target_ulong start, target_ulong end,
                       unsigned long flags);
 
 static void fill_elf_header(struct elfhdr *, int, uint16_t, uint32_t);
@@ -2466,8 +2466,8 @@  static void vma_delete(struct mm_struct *mm)
     g_free(mm);
 }
 
-static int vma_add_mapping(struct mm_struct *mm, abi_ulong start,
-                           abi_ulong end, abi_ulong flags)
+static int vma_add_mapping(struct mm_struct *mm, target_ulong start,
+                           target_ulong end, abi_ulong flags)
 {
     struct vm_area_struct *vma;
 
@@ -2535,7 +2535,7 @@  static abi_ulong vma_dump_size(const struct vm_area_struct *vma)
     return (vma->vma_end - vma->vma_start);
 }
 
-static int vma_walker(void *priv, abi_ulong start, abi_ulong end,
+static int vma_walker(void *priv, target_ulong start, target_ulong end,
                       unsigned long flags)
 {
     struct mm_struct *mm = (struct mm_struct *)priv;
diff --git a/translate-all.c b/translate-all.c
index 2e0265a..885a4ec 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1660,12 +1660,12 @@  void cpu_interrupt(CPUState *cpu, int mask)
 struct walk_memory_regions_data {
     walk_memory_regions_fn fn;
     void *priv;
-    uintptr_t start;
+    target_ulong start;
     int prot;
 };
 
 static int walk_memory_regions_end(struct walk_memory_regions_data *data,
-                                   abi_ulong end, int new_prot)
+                                   target_ulong end, int new_prot)
 {
     if (data->start != -1ul) {
         int rc = data->fn(data->priv, data->start, end, data->prot);
@@ -1681,9 +1681,9 @@  static int walk_memory_regions_end(struct walk_memory_regions_data *data,
 }
 
 static int walk_memory_regions_1(struct walk_memory_regions_data *data,
-                                 abi_ulong base, int level, void **lp)
+                                 target_ulong base, int level, void **lp)
 {
-    abi_ulong pa;
+    target_ulong pa;
     int i, rc;
 
     if (*lp == NULL) {
@@ -1708,7 +1708,7 @@  static int walk_memory_regions_1(struct walk_memory_regions_data *data,
         void **pp = *lp;
 
         for (i = 0; i < V_L2_SIZE; ++i) {
-            pa = base | ((abi_ulong)i <<
+            pa = base | ((target_ulong)i <<
                 (TARGET_PAGE_BITS + V_L2_BITS * level));
             rc = walk_memory_regions_1(data, pa, level - 1, pp + i);
             if (rc != 0) {
@@ -1731,9 +1731,8 @@  int walk_memory_regions(void *priv, walk_memory_regions_fn fn)
     data.prot = 0;
 
     for (i = 0; i < V_L1_SIZE; i++) {
-        int rc = walk_memory_regions_1(&data, (abi_ulong)i << V_L1_SHIFT,
+        int rc = walk_memory_regions_1(&data, (target_ulong)i << (V_L1_SHIFT + TARGET_PAGE_BITS),
                                        V_L1_SHIFT / V_L2_BITS - 1, l1_map + i);
-
         if (rc != 0) {
             return rc;
         }
@@ -1742,13 +1741,13 @@  int walk_memory_regions(void *priv, walk_memory_regions_fn fn)
     return walk_memory_regions_end(&data, 0, 0);
 }
 
-static int dump_region(void *priv, abi_ulong start,
-    abi_ulong end, unsigned long prot)
+static int dump_region(void *priv, target_ulong start,
+    target_ulong end, unsigned long prot)
 {
     FILE *f = (FILE *)priv;
 
-    (void) fprintf(f, TARGET_ABI_FMT_lx"-"TARGET_ABI_FMT_lx
-        " "TARGET_ABI_FMT_lx" %c%c%c\n",
+    (void) fprintf(f, TARGET_FMT_lx"-"TARGET_FMT_lx
+        " "TARGET_FMT_lx" %c%c%c\n",
         start, end, end - start,
         ((prot & PAGE_READ) ? 'r' : '-'),
         ((prot & PAGE_WRITE) ? 'w' : '-'),
@@ -1760,7 +1759,7 @@  static int dump_region(void *priv, abi_ulong start,
 /* dump memory mappings */
 void page_dump(FILE *f)
 {
-    const int length = sizeof(abi_ulong) * 2;
+    const int length = sizeof(target_ulong) * 2;
     (void) fprintf(f, "%-*s %-*s %-*s %s\n",
             length, "start", length, "end", length, "size", "prot");
     walk_memory_regions(f, dump_region);
@@ -1788,7 +1787,7 @@  void page_set_flags(target_ulong start, target_ulong end, int flags)
        guest address space.  If this assert fires, it probably indicates
        a missing call to h2g_valid.  */
 #if TARGET_ABI_BITS > L1_MAP_ADDR_SPACE_BITS
-    assert(end < ((abi_ulong)1 << L1_MAP_ADDR_SPACE_BITS));
+    assert(end < ((target_ulong)1 << L1_MAP_ADDR_SPACE_BITS));
 #endif
     assert(start < end);
 
@@ -1825,7 +1824,7 @@  int page_check_range(target_ulong start, target_ulong len, int flags)
        guest address space.  If this assert fires, it probably indicates
        a missing call to h2g_valid.  */
 #if TARGET_ABI_BITS > L1_MAP_ADDR_SPACE_BITS
-    assert(start < ((abi_ulong)1 << L1_MAP_ADDR_SPACE_BITS));
+    assert(start < ((target_ulong)1 << L1_MAP_ADDR_SPACE_BITS));
 #endif
 
     if (len == 0) {