Re: Concerns regarding e17bebd049 ("dump: Set correct vaddr for ELF dump")

[Stephen Brennan <stephen.s.brennan@oracle.com>]

Hi Jon,

Jon Doron <arilou@gmail.com> writes:
> Hi Stephen,
> Like you have said the reason is as I wrote in the commit message, 
> without "fixing" the vaddr GDB is messing up mapping and working with 
> the generated core file.

For the record I totally love this workaround :)

It's clever and gets the job done and I would have done it in a
heartbeat. It's just that it does end up making vmcores that have
incorrect data, which is a pain for debuggers that are actually designed
to look at kernel core dumps.

> This patch is almost 4 years old, perhaps some changes to GDB has been 
> introduced to resolve this, I have not checked since then.

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  NOTE           0x0000000000000168 0x0000000000000000 0x0000000000000000
                 0x0000000000001980 0x0000000000001980         0x0
  LOAD           0x0000000000001ae8 0x0000000000000000 0x0000000000000000
                 0x0000000080000000 0x0000000080000000         0x0
  LOAD           0x0000000080001ae8 0x0000000000000000 0x00000000fffc0000
                 0x0000000000040000 0x0000000000040000         0x0

(gdb) info files
Local core dump file:
        `/home/stepbren/repos/test_code/elf/dumpfile', file type elf64-x86-64.
        0x0000000000000000 - 0x0000000080000000 is load1
        0x0000000000000000 - 0x0000000000040000 is load2

$ gdb --version 
GNU gdb (GDB) Red Hat Enterprise Linux 10.2-10.0.2.el9
Copyright (C) 2021 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.


It doesn't *look like* anything has changed in this version of GDB. But
I'm not really certain that GDB is expected to use the physical
addresses in the load segments: it's not a kernel debugger.

I think hacking the p_vaddr field _is_ the way to get GDB to behave in
the way you want: allow you to read physical memory addresses.

> As I'm no longer using this feature and have not worked and tested it 
> in a long while, so I have no obligations to this change, but perhaps
> someone else might be using it...

I definitely think it's valuable for people to continue being able to
use QEMU vmcores generated with paging=off in GDB, even if GDB isn't
desgined for it. It seems like a useful hack that appeals to the lowest
common denominator: most people have GDB and not a purpose-built kernel
debugger. But maybe we could point to a program like the below that will
tweak the p_paddr field after the fact, in order to appeal to GDB's
sensibilities?

Thanks,
Stephen

---
#include <stdbool.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <byteswap.h>

#include <elf.h>

static void fail(const char *msg)
{
	fprintf(stderr, "%s\n", msg);
	exit(EXIT_FAILURE);
}

static void perror_fail(const char *pfx)
{
	perror(pfx);
	exit(EXIT_FAILURE);
}

static void usage(void)
{
	puts("usage: phys2virt COREFILE");
	puts("Modifies the ELF COREFILE so that load segments have their virtual");
	puts("address value copied from the physical address field.");
	exit(EXIT_SUCCESS);
}

static int endian(void)
{
	union {
		uint32_t ival;
		char     cval[4];
	} data;
	data.ival = 1;
	if (data.cval[0])
		return ELFDATA2LSB;
	else
		return ELFDATA2MSB;
}

int main(int argc, char **argv)
{
	char *filename;
	FILE *f;
	Elf64_Ehdr hdr;
	Elf64_Phdr *phdrs;
	off_t phoff;
	int phnum, phentsize;
	bool bswap;

	if (argc != 2 || strcmp(argv[1], "-h") == 0)
		usage();

	filename = argv[1];
	f = fopen(filename,  "r+");
	if (!f)
		perror_fail("open");

	if (fread(&hdr, sizeof(hdr), 1, f) != 1)
		perror_fail("read elf header");

	if (memcmp(hdr.e_ident, ELFMAG, 4) != 0)
		fail("not an ELF file");

	if (hdr.e_ident[EI_CLASS] != ELFCLASS64)
		fail("file is not 64-bits: unsupported");

	if (bswap) {
		phoff = bswap_64(hdr.e_phoff);
		phnum = bswap_16(hdr.e_phnum);
		phentsize = bswap_16(hdr.e_phentsize);
	} else {
		phoff = hdr.e_phoff;
		phnum = hdr.e_phnum;
		phentsize = hdr.e_phentsize;
	}
	if (phentsize != sizeof(Elf64_Phdr))
		fail("error: mismatch between phentsize and sizeof(Elf64_Phdr)");

	if (fseek(f, phoff, SEEK_SET) < 0)
		perror_fail("fseek");

	phdrs = calloc(phnum, phentsize);
	if (!phdrs)
		fail("error: allocation error");

	if (fread(phdrs, phentsize, phnum, f) != phnum)
		perror_fail("fread phdrs");

	for (int i = 0; i < phnum; i++)
		phdrs[i].p_vaddr = 0; //phdrs[i].p_paddr;

	if (fseek(f, phoff, SEEK_SET) < 0)
		perror_fail("fseek");
	if (fwrite(phdrs, phentsize, phnum, f) != phnum)
		perror_fail("fwrite phdrs");

	fclose(f);
	return EXIT_SUCCESS;
}