| Message ID | 20190109082203.27142-1-arilou@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] dump: Set correct vaddr for ELF dump | expand |
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 > >
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
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
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
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)
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(-)