Date: Wed, 17 Sep 2008 12:51:02 -0700 From: "Navdeep Parhar" <nparhar@gmail.com> To: "John Baldwin" <jhb@freebsd.org> Cc: freebsd-hackers@freebsd.org Subject: Re: kgdb's add-kld broken on amd64 Message-ID: <d04e16b70809171251h474df74h89829048582f994a@mail.gmail.com> In-Reply-To: <200809171131.30819.jhb@freebsd.org> References: <d04e16b70809161307p11a44c53i4e0a33edc12257c8@mail.gmail.com> <200809171131.30819.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Hello John,
The patch did NOT fix the problem. Read on for more....
On Wed, Sep 17, 2008 at 8:31 AM, John Baldwin <jhb@freebsd.org> wrote:
> On Tuesday 16 September 2008 04:07:46 pm Navdeep Parhar wrote:
>> Hello everyone,
>>
>> The add-kld command in kgdb does not work as expected on amd64
>> (I'm using a recent HEAD, problem may affect others too). It uses
>> the same address for all sections:
>>
>> (kgdb) add-kld if_cxgb.ko
>> add symbol table from file "/boot/kernel/if_cxgb.ko" at
>> .text_addr = 0xffffffff81022000
>> .rodata_addr = 0xffffffff81022000
>> .rodata.str1.8_addr = 0xffffffff81022000
>> .rodata.str1.1_addr = 0xffffffff81022000
>> set_modmetadata_set_addr = 0xffffffff81022000
>> set_sysctl_set_addr = 0xffffffff81022000
>> set_sysinit_set_addr = 0xffffffff81022000
>> set_sysuninit_set_addr = 0xffffffff81022000
>> .data_addr = 0xffffffff81022000
>> .bss_addr = 0xffffffff81022000
>> (y or n)
>>
>> This is not correct. The .text section's address is OK but the
>> others are not.
>>
>> The problem seems to be that all amd64 kernel objects have VMA set
>> to 0 for all sections. add_section() in gnu/usr.bin/gdb/kgdb/kld.c
>> uses this VMA to adjust the address of the section:
>>
>> address = asi->base_addr + bfd_get_section_vma(bfd, sect);
>>
>> objdump -h shows that the userland objects on amd64 and all
>> objects (kernel + userland) on i386 set VMA. It is only the
>> kernel objects on amd64 that have VMA = 0. (sample output from
>> amd64 and i386 machines appended at the end)
>>
>> For the time being I've patched kgdb to consider the file offset
>> and not the VMA while calculating the section address. It seems
>> to work but is probably not the right way to fix the problem.
>>
>> Any thoughts?
>
> Hmm, I wonder if this is because on amd64 modules are .o's rather than .so's.
> It is. File offset isn't quite right. Instead, the way
> sys/kern/link_elf_obj.c works is that it just loads the PROGBITS (text, code,
> etc.) and NOBITS (bss) sections in the order they are in the file and
> concatenates them. So, the relocation logic in kgdb will need to be updated
> to recognize a .o vs .so and apply that algorithm for .o files.
>
> Actually, what I've done is to replace the home-rolled section relocation
> stuff with the gdb primitives that the solib code in gdb uses. It works here
> on i386, and hopefully this will fix this as this is how the sharedlibrary
> kld stuff is doing the relocations:
I had to modify the patch a bit as add-kld -> build_section_table() -> xfree()
was a bad free and led to bus errors or segv:
- struct section_table *sections, *sections_end, *s;
+ struct section_table *sections = NULL, *sections_end = NULL, *s;
After fixing that, add-kld still wouldn't pick up the correct
addresses:
(kgdb) add-kld if_cxgb.ko
add symbol table from file "/boot/kernel/if_cxgb.ko" at
.text_addr = 0xffffffff81022000
.rodata_addr = 0xffffffff81022000
.rodata.str1.8_addr = 0xffffffff81022000
.rodata.str1.1_addr = 0xffffffff81022000
set_modmetadata_set_addr = 0xffffffff81022000
set_sysctl_set_addr = 0xffffffff81022000
set_sysinit_set_addr = 0xffffffff81022000
set_sysuninit_set_addr = 0xffffffff81022000
.data_addr = 0xffffffff81022000
.bss_addr = 0xffffffff81022000
With the patch the section relocation is still taking place based
on the VMA (which is 0 for amd64 modules as I pointed out
earlier). So the behaviour is no different than before. If I
read the code right, each section's addr is calculated as:
load_kld -> build_section_table -> add_to_section_table
This sets it to bfd_section_vma(abfd, asect), which is no good
for amd64 kernel modules.
Regards,
Navdeep
>
> --- //depot/vendor/freebsd/src/gnu/usr.bin/gdb/kgdb/kld.c 2008/04/29 20:41:02
> +++ //depot/user/jhb/kgdb/gnu/usr.bin/gdb/kgdb/kld.c 2008/09/17 15:27:25
> @@ -37,6 +37,7 @@
> #include <command.h>
> #include <completer.h>
> #include <environ.h>
> +#include <exec.h>
> #include <frame-unwind.h>
> #include <inferior.h>
> #include <objfiles.h>
> @@ -196,39 +197,14 @@
> return (0);
> }
>
> -struct add_section_info {
> - struct section_addr_info *section_addrs;
> - int sect_index;
> - CORE_ADDR base_addr;
> -};
> -
> -static void
> -add_section (bfd *bfd, asection *sect, void *arg)
> -{
> - struct add_section_info *asi = arg;
> - CORE_ADDR address;
> - char *name;
> -
> - /* Ignore non-resident sections. */
> - if ((bfd_get_section_flags(bfd, sect) & (SEC_ALLOC | SEC_LOAD)) == 0)
> - return;
> -
> - name = xstrdup(bfd_get_section_name(bfd, sect));
> - make_cleanup(xfree, name);
> - address = asi->base_addr + bfd_get_section_vma(bfd, sect);
> - asi->section_addrs->other[asi->sect_index].name = name;
> - asi->section_addrs->other[asi->sect_index].addr = address;
> - asi->section_addrs->other[asi->sect_index].sectindex = sect->index;
> - printf_unfiltered("\t%s_addr = %s\n", name, local_hex_string(address));
> - asi->sect_index++;
> -}
> -
> static void
> load_kld (char *path, CORE_ADDR base_addr, int from_tty)
> {
> - struct add_section_info asi;
> + struct section_addr_info *sap;
> + struct section_table *sections, *sections_end, *s;
> struct cleanup *cleanup;
> bfd *bfd;
> + int i;
>
> /* Open the kld. */
> bfd = bfd_openr(path, gnutarget);
> @@ -244,19 +220,30 @@
> if (bfd_get_section_by_name (bfd, ".text") == NULL)
> error("\"%s\": can't find text section", path);
>
> + /* Build a section table from the bfd and relocate the sections. */
> + if (build_section_table (bfd, §ions, §ions_end))
> + error("\"%s\": can't find file sections", path);
> + cleanup = make_cleanup(xfree, sections);
> + for (s = sections; s < sections_end; s++) {
> + s->addr += base_addr;
> + s->endaddr += base_addr;
> + }
> +
> + /* Build a section addr info to pass to symbol_file_add(). */
> + sap = build_section_addr_info_from_section_table (sections,
> + sections_end);
> + cleanup = make_cleanup((make_cleanup_ftype *)free_section_addr_info,
> + sap);
> +
> printf_unfiltered("add symbol table from file \"%s\" at\n", path);
> -
> - /* Build a section table for symbol_file_add() from the bfd sections. */
> - asi.section_addrs = alloc_section_addr_info(bfd_count_sections(bfd));
> - cleanup = make_cleanup(xfree, asi.section_addrs);
> - asi.sect_index = 0;
> - asi.base_addr = base_addr;
> - bfd_map_over_sections(bfd, add_section, &asi);
> + for (i = 0; i < sap->num_sections; i++)
> + printf_unfiltered("\t%s_addr = %s\n", sap->other[i].name,
> + local_hex_string(sap->other[i].addr));
>
> if (from_tty && (!query("%s", "")))
> error("Not confirmed.");
>
> - symbol_file_add(path, from_tty, asi.section_addrs, 0, OBJF_USERLOADED);
> + symbol_file_add(path, from_tty, sap, 0, OBJF_USERLOADED);
>
> do_cleanups(cleanup);
> }
>
> --
> John Baldwin
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?d04e16b70809171251h474df74h89829048582f994a>