From owner-freebsd-hackers@FreeBSD.ORG Wed Sep 17 19:51:03 2008 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id DECD2106564A for ; Wed, 17 Sep 2008 19:51:03 +0000 (UTC) (envelope-from nparhar@gmail.com) Received: from yx-out-2324.google.com (yx-out-2324.google.com [74.125.44.30]) by mx1.freebsd.org (Postfix) with ESMTP id 852858FC17 for ; Wed, 17 Sep 2008 19:51:03 +0000 (UTC) (envelope-from nparhar@gmail.com) Received: by yx-out-2324.google.com with SMTP id 8so1008576yxb.13 for ; Wed, 17 Sep 2008 12:51:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from:to :subject:cc:in-reply-to:mime-version:content-type :content-transfer-encoding:content-disposition:references; bh=rqeyPSilaTJ9m02HgKcDi6BykOPset8POY3z68tX4QM=; b=jsZ0ypGAMYRVmD29lnTkX6c47UI07mGQTH7uRkMZKbqp9QsD94PbSJgGsE7mUG91HR ur8U+1BqPdwqlw+Lxla19TX0Oan4XBVtfEkHVj+a2bLnhowgWY7GvOCtu2XKGTQtqxaF RfjqGVi3etPDpFrfZKDb5fLJwN8rqqj+shMBw= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:to:subject:cc:in-reply-to:mime-version :content-type:content-transfer-encoding:content-disposition :references; b=R6u3d5xbZFRValtdQP3IVKbMCLYhyMu7Czgof4RDzQtbBh88sUeKHWN5GEkayCGlld rSrFZXTDIwwBCO1eMwYTFME1CSorlYOB4S5G0yAEf2FyXpOilPosQi5KX34Euq6txsIi tdJL3AUsK2ayCmDrPvD9/UI2MPEIw+d3RMJpg= Received: by 10.151.143.14 with SMTP id v14mr4164418ybn.144.1221681062571; Wed, 17 Sep 2008 12:51:02 -0700 (PDT) Received: by 10.150.228.8 with HTTP; Wed, 17 Sep 2008 12:51:02 -0700 (PDT) Message-ID: Date: Wed, 17 Sep 2008 12:51:02 -0700 From: "Navdeep Parhar" To: "John Baldwin" In-Reply-To: <200809171131.30819.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <200809171131.30819.jhb@freebsd.org> Cc: freebsd-hackers@freebsd.org Subject: Re: kgdb's add-kld broken on amd64 X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Sep 2008 19:51:04 -0000 Hello John, The patch did NOT fix the problem. Read on for more.... On Wed, Sep 17, 2008 at 8:31 AM, John Baldwin 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 > #include > #include > +#include > #include > #include > #include > @@ -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 >