[Qemu-devel] [PATCH v3] dump: Set correct vaddr for ELF dump

Jon Doron posted 1 patch 6 years, 10 months ago
Test asan passed
Test checkpatch passed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test docker-clang@ubuntu passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190109082203.27142-1-arilou@gmail.com
dump.c                       | 5 +++--
scripts/dump-guest-memory.py | 1 +
2 files changed, 4 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH v3] dump: Set correct vaddr for ELF dump
Posted by Jon Doron 6 years, 10 months ago
vaddr needs to be equal to the paddr since the dump file represents the
physical memory image.

Without setting vaddr correctly, GDB would load all the different memory
regions on top of each other to vaddr 0, thus making GDB showing the wrong
memory data for a given address.

Signed-off-by: Jon Doron <arilou@gmail.com>
---
 dump.c                       | 5 +++--
 scripts/dump-guest-memory.py | 1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/dump.c b/dump.c
index 4ec94c5e25..de7f70f099 100644
--- a/dump.c
+++ b/dump.c
@@ -192,7 +192,7 @@ static void write_elf64_load(DumpState *s, MemoryMapping *memory_mapping,
     phdr.p_paddr = cpu_to_dump64(s, memory_mapping->phys_addr);
     phdr.p_filesz = cpu_to_dump64(s, filesz);
     phdr.p_memsz = cpu_to_dump64(s, memory_mapping->length);
-    phdr.p_vaddr = cpu_to_dump64(s, memory_mapping->virt_addr);
+    phdr.p_vaddr = cpu_to_dump64(s, memory_mapping->virt_addr) ? : phdr.p_paddr;
 
     assert(memory_mapping->length >= filesz);
 
@@ -216,7 +216,8 @@ static void write_elf32_load(DumpState *s, MemoryMapping *memory_mapping,
     phdr.p_paddr = cpu_to_dump32(s, memory_mapping->phys_addr);
     phdr.p_filesz = cpu_to_dump32(s, filesz);
     phdr.p_memsz = cpu_to_dump32(s, memory_mapping->length);
-    phdr.p_vaddr = cpu_to_dump32(s, memory_mapping->virt_addr);
+    phdr.p_vaddr =
+        cpu_to_dump32(s, memory_mapping->virt_addr) ? : phdr.p_paddr;
 
     assert(memory_mapping->length >= filesz);
 
diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
index 198cd0fe40..2c587cbefc 100644
--- a/scripts/dump-guest-memory.py
+++ b/scripts/dump-guest-memory.py
@@ -163,6 +163,7 @@ class ELF(object):
         phdr = get_arch_phdr(self.endianness, self.elfclass)
         phdr.p_type = p_type
         phdr.p_paddr = p_paddr
+        phdr.p_vaddr = p_paddr
         phdr.p_filesz = p_size
         phdr.p_memsz = p_size
         self.segments.append(phdr)
-- 
2.19.2


Re: [Qemu-devel] [PATCH v3] dump: Set correct vaddr for ELF dump
Posted by Marc-André Lureau 6 years, 10 months ago
On Wed, Jan 9, 2019 at 12:22 PM Jon Doron <arilou@gmail.com> wrote:
>
> vaddr needs to be equal to the paddr since the dump file represents the
> physical memory image.
>
> Without setting vaddr correctly, GDB would load all the different memory
> regions on top of each other to vaddr 0, thus making GDB showing the wrong
> memory data for a given address.
>
> Signed-off-by: Jon Doron <arilou@gmail.com>

Tested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  dump.c                       | 5 +++--
>  scripts/dump-guest-memory.py | 1 +
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/dump.c b/dump.c
> index 4ec94c5e25..de7f70f099 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -192,7 +192,7 @@ static void write_elf64_load(DumpState *s, MemoryMapping *memory_mapping,
>      phdr.p_paddr = cpu_to_dump64(s, memory_mapping->phys_addr);
>      phdr.p_filesz = cpu_to_dump64(s, filesz);
>      phdr.p_memsz = cpu_to_dump64(s, memory_mapping->length);
> -    phdr.p_vaddr = cpu_to_dump64(s, memory_mapping->virt_addr);
> +    phdr.p_vaddr = cpu_to_dump64(s, memory_mapping->virt_addr) ? : phdr.p_paddr;
>
>      assert(memory_mapping->length >= filesz);
>
> @@ -216,7 +216,8 @@ static void write_elf32_load(DumpState *s, MemoryMapping *memory_mapping,
>      phdr.p_paddr = cpu_to_dump32(s, memory_mapping->phys_addr);
>      phdr.p_filesz = cpu_to_dump32(s, filesz);
>      phdr.p_memsz = cpu_to_dump32(s, memory_mapping->length);
> -    phdr.p_vaddr = cpu_to_dump32(s, memory_mapping->virt_addr);
> +    phdr.p_vaddr =
> +        cpu_to_dump32(s, memory_mapping->virt_addr) ? : phdr.p_paddr;
>
>      assert(memory_mapping->length >= filesz);
>
> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
> index 198cd0fe40..2c587cbefc 100644
> --- a/scripts/dump-guest-memory.py
> +++ b/scripts/dump-guest-memory.py
> @@ -163,6 +163,7 @@ class ELF(object):
>          phdr = get_arch_phdr(self.endianness, self.elfclass)
>          phdr.p_type = p_type
>          phdr.p_paddr = p_paddr
> +        phdr.p_vaddr = p_paddr
>          phdr.p_filesz = p_size
>          phdr.p_memsz = p_size
>          self.segments.append(phdr)
> --
> 2.19.2
>
>


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v3] dump: Set correct vaddr for ELF dump
Posted by Laszlo Ersek 6 years, 10 months ago
On 01/09/19 09:25, Marc-André Lureau wrote:
> On Wed, Jan 9, 2019 at 12:22 PM Jon Doron <arilou@gmail.com> wrote:
>>
>> vaddr needs to be equal to the paddr since the dump file represents the
>> physical memory image.
>>
>> Without setting vaddr correctly, GDB would load all the different memory
>> regions on top of each other to vaddr 0, thus making GDB showing the wrong
>> memory data for a given address.
>>
>> Signed-off-by: Jon Doron <arilou@gmail.com>
> 
> Tested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Apparently the only change in v3, relative to v2, is cleaning up the
whitespace problem that was caught by Patchew.

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

>>  dump.c                       | 5 +++--
>>  scripts/dump-guest-memory.py | 1 +
>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/dump.c b/dump.c
>> index 4ec94c5e25..de7f70f099 100644
>> --- a/dump.c
>> +++ b/dump.c
>> @@ -192,7 +192,7 @@ static void write_elf64_load(DumpState *s, MemoryMapping *memory_mapping,
>>      phdr.p_paddr = cpu_to_dump64(s, memory_mapping->phys_addr);
>>      phdr.p_filesz = cpu_to_dump64(s, filesz);
>>      phdr.p_memsz = cpu_to_dump64(s, memory_mapping->length);
>> -    phdr.p_vaddr = cpu_to_dump64(s, memory_mapping->virt_addr);
>> +    phdr.p_vaddr = cpu_to_dump64(s, memory_mapping->virt_addr) ? : phdr.p_paddr;
>>
>>      assert(memory_mapping->length >= filesz);
>>
>> @@ -216,7 +216,8 @@ static void write_elf32_load(DumpState *s, MemoryMapping *memory_mapping,
>>      phdr.p_paddr = cpu_to_dump32(s, memory_mapping->phys_addr);
>>      phdr.p_filesz = cpu_to_dump32(s, filesz);
>>      phdr.p_memsz = cpu_to_dump32(s, memory_mapping->length);
>> -    phdr.p_vaddr = cpu_to_dump32(s, memory_mapping->virt_addr);
>> +    phdr.p_vaddr =
>> +        cpu_to_dump32(s, memory_mapping->virt_addr) ? : phdr.p_paddr;
>>
>>      assert(memory_mapping->length >= filesz);
>>
>> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
>> index 198cd0fe40..2c587cbefc 100644
>> --- a/scripts/dump-guest-memory.py
>> +++ b/scripts/dump-guest-memory.py
>> @@ -163,6 +163,7 @@ class ELF(object):
>>          phdr = get_arch_phdr(self.endianness, self.elfclass)
>>          phdr.p_type = p_type
>>          phdr.p_paddr = p_paddr
>> +        phdr.p_vaddr = p_paddr
>>          phdr.p_filesz = p_size
>>          phdr.p_memsz = p_size
>>          self.segments.append(phdr)
>> --
>> 2.19.2
>>
>>
> 
> 
I pre

Re: [Qemu-devel] [PATCH v3] dump: Set correct vaddr for ELF dump
Posted by Jon Doron 6 years, 9 months ago
ping, so this means that the patch will be merged in?

Thanks,
-- Jon.

On Wed, Jan 9, 2019 at 12:38 PM Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 01/09/19 09:25, Marc-André Lureau wrote:
> > On Wed, Jan 9, 2019 at 12:22 PM Jon Doron <arilou@gmail.com> wrote:
> >>
> >> vaddr needs to be equal to the paddr since the dump file represents the
> >> physical memory image.
> >>
> >> Without setting vaddr correctly, GDB would load all the different memory
> >> regions on top of each other to vaddr 0, thus making GDB showing the wrong
> >> memory data for a given address.
> >>
> >> Signed-off-by: Jon Doron <arilou@gmail.com>
> >
> > Tested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Apparently the only change in v3, relative to v2, is cleaning up the
> whitespace problem that was caught by Patchew.
>
> Acked-by: Laszlo Ersek <lersek@redhat.com>
>
> Thanks
> Laszlo
>
> >>  dump.c                       | 5 +++--
> >>  scripts/dump-guest-memory.py | 1 +
> >>  2 files changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/dump.c b/dump.c
> >> index 4ec94c5e25..de7f70f099 100644
> >> --- a/dump.c
> >> +++ b/dump.c
> >> @@ -192,7 +192,7 @@ static void write_elf64_load(DumpState *s, MemoryMapping *memory_mapping,
> >>      phdr.p_paddr = cpu_to_dump64(s, memory_mapping->phys_addr);
> >>      phdr.p_filesz = cpu_to_dump64(s, filesz);
> >>      phdr.p_memsz = cpu_to_dump64(s, memory_mapping->length);
> >> -    phdr.p_vaddr = cpu_to_dump64(s, memory_mapping->virt_addr);
> >> +    phdr.p_vaddr = cpu_to_dump64(s, memory_mapping->virt_addr) ? : phdr.p_paddr;
> >>
> >>      assert(memory_mapping->length >= filesz);
> >>
> >> @@ -216,7 +216,8 @@ static void write_elf32_load(DumpState *s, MemoryMapping *memory_mapping,
> >>      phdr.p_paddr = cpu_to_dump32(s, memory_mapping->phys_addr);
> >>      phdr.p_filesz = cpu_to_dump32(s, filesz);
> >>      phdr.p_memsz = cpu_to_dump32(s, memory_mapping->length);
> >> -    phdr.p_vaddr = cpu_to_dump32(s, memory_mapping->virt_addr);
> >> +    phdr.p_vaddr =
> >> +        cpu_to_dump32(s, memory_mapping->virt_addr) ? : phdr.p_paddr;
> >>
> >>      assert(memory_mapping->length >= filesz);
> >>
> >> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
> >> index 198cd0fe40..2c587cbefc 100644
> >> --- a/scripts/dump-guest-memory.py
> >> +++ b/scripts/dump-guest-memory.py
> >> @@ -163,6 +163,7 @@ class ELF(object):
> >>          phdr = get_arch_phdr(self.endianness, self.elfclass)
> >>          phdr.p_type = p_type
> >>          phdr.p_paddr = p_paddr
> >> +        phdr.p_vaddr = p_paddr
> >>          phdr.p_filesz = p_size
> >>          phdr.p_memsz = p_size
> >>          self.segments.append(phdr)
> >> --
> >> 2.19.2
> >>
> >>
> >
> >
> I pre

Re: [Qemu-devel] [PATCH v3] dump: Set correct vaddr for ELF dump
Posted by Laszlo Ersek 6 years, 9 months ago
On 01/22/19 06:36, Jon Doron wrote:
> ping, so this means that the patch will be merged in?
> 
> Thanks,
> -- Jon.

Marc-André, can you please confirm if you've queued this patch for a
pull? (According to scripts/get_maintainer.pl, you are "supporter:Dump",
for both files touched by this patch.)

Apologies if I missed a message elsewhere in this thread, and I should
know already.

Thanks!
Laszlo

> 
> On Wed, Jan 9, 2019 at 12:38 PM Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> On 01/09/19 09:25, Marc-André Lureau wrote:
>>> On Wed, Jan 9, 2019 at 12:22 PM Jon Doron <arilou@gmail.com> wrote:
>>>>
>>>> vaddr needs to be equal to the paddr since the dump file represents the
>>>> physical memory image.
>>>>
>>>> Without setting vaddr correctly, GDB would load all the different memory
>>>> regions on top of each other to vaddr 0, thus making GDB showing the wrong
>>>> memory data for a given address.
>>>>
>>>> Signed-off-by: Jon Doron <arilou@gmail.com>
>>>
>>> Tested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> Apparently the only change in v3, relative to v2, is cleaning up the
>> whitespace problem that was caught by Patchew.
>>
>> Acked-by: Laszlo Ersek <lersek@redhat.com>
>>
>> Thanks
>> Laszlo
>>
>>>>  dump.c                       | 5 +++--
>>>>  scripts/dump-guest-memory.py | 1 +
>>>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/dump.c b/dump.c
>>>> index 4ec94c5e25..de7f70f099 100644
>>>> --- a/dump.c
>>>> +++ b/dump.c
>>>> @@ -192,7 +192,7 @@ static void write_elf64_load(DumpState *s, MemoryMapping *memory_mapping,
>>>>      phdr.p_paddr = cpu_to_dump64(s, memory_mapping->phys_addr);
>>>>      phdr.p_filesz = cpu_to_dump64(s, filesz);
>>>>      phdr.p_memsz = cpu_to_dump64(s, memory_mapping->length);
>>>> -    phdr.p_vaddr = cpu_to_dump64(s, memory_mapping->virt_addr);
>>>> +    phdr.p_vaddr = cpu_to_dump64(s, memory_mapping->virt_addr) ? : phdr.p_paddr;
>>>>
>>>>      assert(memory_mapping->length >= filesz);
>>>>
>>>> @@ -216,7 +216,8 @@ static void write_elf32_load(DumpState *s, MemoryMapping *memory_mapping,
>>>>      phdr.p_paddr = cpu_to_dump32(s, memory_mapping->phys_addr);
>>>>      phdr.p_filesz = cpu_to_dump32(s, filesz);
>>>>      phdr.p_memsz = cpu_to_dump32(s, memory_mapping->length);
>>>> -    phdr.p_vaddr = cpu_to_dump32(s, memory_mapping->virt_addr);
>>>> +    phdr.p_vaddr =
>>>> +        cpu_to_dump32(s, memory_mapping->virt_addr) ? : phdr.p_paddr;
>>>>
>>>>      assert(memory_mapping->length >= filesz);
>>>>
>>>> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
>>>> index 198cd0fe40..2c587cbefc 100644
>>>> --- a/scripts/dump-guest-memory.py
>>>> +++ b/scripts/dump-guest-memory.py
>>>> @@ -163,6 +163,7 @@ class ELF(object):
>>>>          phdr = get_arch_phdr(self.endianness, self.elfclass)
>>>>          phdr.p_type = p_type
>>>>          phdr.p_paddr = p_paddr
>>>> +        phdr.p_vaddr = p_paddr
>>>>          phdr.p_filesz = p_size
>>>>          phdr.p_memsz = p_size
>>>>          self.segments.append(phdr)
>>>> --
>>>> 2.19.2
>>>>
>>>>
>>>
>>>
>> I pre