From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31087 invoked by alias); 20 Jun 2007 13:15:19 -0000 Received: (qmail 31079 invoked by uid 22791); 20 Jun 2007 13:15:17 -0000 X-Spam-Check-By: sourceware.org Received: from NaN.false.org (HELO nan.false.org) (208.75.86.248) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 20 Jun 2007 13:15:13 +0000 Received: from nan.false.org (localhost [127.0.0.1]) by nan.false.org (Postfix) with ESMTP id 4B274982E6; Wed, 20 Jun 2007 13:15:10 +0000 (GMT) Received: from caradoc.them.org (22.svnf5.xdsl.nauticom.net [209.195.183.55]) by nan.false.org (Postfix) with ESMTP id B2037982BA; Wed, 20 Jun 2007 13:15:09 +0000 (GMT) Received: from drow by caradoc.them.org with local (Exim 4.67) (envelope-from ) id 1I101s-0005bO-PE; Wed, 20 Jun 2007 09:15:24 -0400 Date: Wed, 20 Jun 2007 13:15:00 -0000 From: Daniel Jacobowitz To: "Joseph S. Myers" Cc: Thiago Jung Bauermann , gdb-patches ml Subject: Re: [PATCH] Fixes problem setting breakpoint in dynamic loader Message-ID: <20070620131524.GA21190@caradoc.them.org> Mail-Followup-To: "Joseph S. Myers" , Thiago Jung Bauermann , gdb-patches ml References: <1177701733.10993.27.camel@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.15 (2007-04-09) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2007-06/txt/msg00389.txt.bz2 On Thu, Jun 14, 2007 at 11:38:33PM +0000, Joseph S. Myers wrote: > I have a patch version partly based on the earlier revision > which does > set the breakpoint correctly, but I still have other GDB problems I'm > investigating with that patch applied. Here's an extended version of Joseph's revision of Thiago's revision of Paul's patch (begat by... who begat...). I tested it with gdbserver on powerpc-linux -m64; it fixes the function calling test in nodebug.exp and all the threading tests. The branch I was testing on is a little bit older than HEAD, and I had an unstripped libc; but I believe that on HEAD and with a stripped libc, it will also fix a number of shared library load/unload tests and all of the calls to malloc to create strings. I'm going to test it natively now. The threading tests are a remote-specific problem. gdbserver asks "where's __nptl_create_event?" and gdb was responding with the .opd location. We need to give the function instead, since gdbserver is going to put a breakpoint there. This requires handling memory reads (from the .opd entry) during qSymbol conversations. Any comments on this patch? -- Daniel Jacobowitz CodeSourcery 2007-06-20 Paul Gilliam Thiago Bauermann Joseph S. Myers Daniel Jacobowitz gdb/ * remote.c (remote_check_symbols): Use gdbarch_convert_from_func_ptr_addr. * infcall.c (find_function_addr): Handle function descriptors without debugging information. * ppc-linux-tdep.c (ppc_linux_convert_from_func_ptr_addr): Renamed from ppc64_linux_convert_from_func_ptr_addr. Handle -msecure-plt. (ppc_linux_init_abi): Always set convert_from_func_ptr_addr. * solib-svr4.c (solib_break_names): Remove "._dl_debug_state". (bfd_lookup_symbol): Do not take a SECT_FLAGS argument. Always allow SEC_CODE and SEC_DATA. (enable_break): Update calls. Pass current_target to solib_add. Use gdbarch_convert_from_func_ptr_addr. gdb/gdbserver/ * remote-utils.c (look_up_one_symbol): Handle 'm' packets. Index: remote.c =================================================================== --- remote.c (revision 174465) +++ remote.c (working copy) @@ -2187,9 +2187,19 @@ remote_check_symbols (struct objfile *ob if (sym == NULL) xsnprintf (msg, get_remote_packet_size (), "qSymbol::%s", &reply[8]); else - xsnprintf (msg, get_remote_packet_size (), "qSymbol:%s:%s", - paddr_nz (SYMBOL_VALUE_ADDRESS (sym)), - &reply[8]); + { + CORE_ADDR sym_addr = SYMBOL_VALUE_ADDRESS (sym); + + /* If this is a function address, return the start of code + instead of any data function descriptor. */ + sym_addr = gdbarch_convert_from_func_ptr_addr (current_gdbarch, + sym_addr, + ¤t_target); + + xsnprintf (msg, get_remote_packet_size (), "qSymbol:%s:%s", + paddr_nz (sym_addr), &reply[8]); + } + putpkt (msg); getpkt (&rs->buf, &rs->buf_size, 0); reply = rs->buf; Index: infcall.c =================================================================== --- infcall.c (revision 174465) +++ infcall.c (working copy) @@ -222,8 +222,24 @@ find_function_addr (struct value *functi if (TYPE_LENGTH (ftype) == 1) funaddr = value_as_address (value_addr (function)); else - /* Handle integer used as address of a function. */ - funaddr = (CORE_ADDR) value_as_long (function); + { + /* Handle function descriptors lacking debug info. */ + int found_descriptor = 0; + if (VALUE_LVAL (function) == lval_memory) + { + CORE_ADDR nfunaddr; + funaddr = value_as_address (value_addr (function)); + nfunaddr = funaddr; + funaddr = gdbarch_convert_from_func_ptr_addr (current_gdbarch, + funaddr, + ¤t_target); + if (funaddr != nfunaddr) + found_descriptor = 1; + } + if (!found_descriptor) + /* Handle integer used as address of a function. */ + funaddr = (CORE_ADDR) value_as_long (function); + } value_type = builtin_type_int; } Index: ppc-linux-tdep.c =================================================================== --- ppc-linux-tdep.c (revision 174465) +++ ppc-linux-tdep.c (working copy) @@ -725,39 +725,73 @@ ppc64_skip_trampoline_code (CORE_ADDR pc } -/* Support for CONVERT_FROM_FUNC_PTR_ADDR (ARCH, ADDR, TARG) on PPC64 +/* Support for convert_from_func_ptr_addr (ARCH, ADDR, TARG) on PPC GNU/Linux. Usually a function pointer's representation is simply the address - of the function. On GNU/Linux on the 64-bit PowerPC however, a - function pointer is represented by a pointer to a TOC entry. This - TOC entry contains three words, the first word is the address of - the function, the second word is the TOC pointer (r2), and the - third word is the static chain value. Throughout GDB it is - currently assumed that a function pointer contains the address of - the function, which is not easy to fix. In addition, the + of the function. On GNU/Linux on the PowerPC however, a function + pointer may be a pointer to a function descriptor. + + For PPC64, a function descriptor is a TOC entry, in a data section, + which contains three words: the first word is the address of the + function, the second word is the TOC pointer (r2), and the third word + is the static chain value. + + For PPC32, there are two kinds of function pointers: non-secure and + secure. Non-secure function pointers point directly to the + function in a code section and thus need no translation. Secure + ones (from GCC's -msecure-plt option) are in a data section and + contain one word: the address of the function. + + Throughout GDB it is currently assumed that a function pointer contains + the address of the function, which is not easy to fix. In addition, the conversion of a function address to a function pointer would require allocation of a TOC entry in the inferior's memory space, with all its drawbacks. To be able to call C++ virtual methods in the inferior (which are called via function pointers), find_function_addr uses this function to get the function address - from a function pointer. */ + from a function pointer. -/* If ADDR points at what is clearly a function descriptor, transform - it into the address of the corresponding function. Be - conservative, otherwize GDB will do the transformation on any - random addresses such as occures when there is no symbol table. */ + If ADDR points at what is clearly a function descriptor, transform + it into the address of the corresponding function, if needed. Be + conservative, otherwise GDB will do the transformation on any + random addresses such as occur when there is no symbol table. */ static CORE_ADDR -ppc64_linux_convert_from_func_ptr_addr (struct gdbarch *gdbarch, - CORE_ADDR addr, - struct target_ops *targ) +ppc_linux_convert_from_func_ptr_addr (struct gdbarch *gdbarch, + CORE_ADDR addr, + struct target_ops *targ) { + struct gdbarch_tdep *tdep; struct section_table *s = target_section_by_addr (targ, addr); + char *sect_name = NULL; + + if (!s) + return addr; + + tdep = gdbarch_tdep (gdbarch); + + switch (tdep->wordsize) + { + case 4: + sect_name = ".plt"; + break; + case 8: + sect_name = ".opd"; + break; + default: + internal_error (__FILE__, __LINE__, + _("failed internal consistency check")); + } /* Check if ADDR points to a function descriptor. */ - if (s && strcmp (s->the_bfd_section->name, ".opd") == 0) - return get_target_memory_unsigned (targ, addr, 8); + + /* NOTE: this depends on the coincidence that the address of a functions + entry point is contained in the first word of its function descriptor + for both PPC-64 and for PPC-32 with secure PLTs. */ + if ((strcmp (s->the_bfd_section->name, sect_name) == 0) + && s->the_bfd_section->flags & SEC_DATA) + return get_target_memory_unsigned (targ, addr, tdep->wordsize); return addr; } @@ -1038,6 +1072,11 @@ ppc_linux_init_abi (struct gdbarch_info /* NOTE: cagney/2005-01-25: True for both 32- and 64-bit. */ set_gdbarch_long_double_bit (gdbarch, 8 * TARGET_CHAR_BIT); + /* Handle PPC GNU/Linux 64-bit function pointers (which are really + function descriptors) and 32-bit secure PLT entries. */ + set_gdbarch_convert_from_func_ptr_addr + (gdbarch, ppc_linux_convert_from_func_ptr_addr); + if (tdep->wordsize == 4) { /* Until November 2001, gcc did not comply with the 32 bit SysV @@ -1065,13 +1104,8 @@ ppc_linux_init_abi (struct gdbarch_info if (tdep->wordsize == 8) { - /* Handle PPC64 GNU/Linux function pointers (which are really - function descriptors). */ - set_gdbarch_convert_from_func_ptr_addr - (gdbarch, ppc64_linux_convert_from_func_ptr_addr); - set_gdbarch_skip_trampoline_code (gdbarch, ppc64_skip_trampoline_code); - /* Shared library handling. */ + set_gdbarch_skip_trampoline_code (gdbarch, ppc64_skip_trampoline_code); set_solib_svr4_fetch_link_map_offsets (gdbarch, svr4_lp64_fetch_link_map_offsets); Index: gdbserver/remote-utils.c =================================================================== --- gdbserver/remote-utils.c (revision 174465) +++ gdbserver/remote-utils.c (working copy) @@ -1063,15 +1063,33 @@ look_up_one_symbol (const char *name, CO if (len < 0) return -1; - if (strncmp (own_buf, "qSymbol:", strlen ("qSymbol:")) != 0) + /* We ought to handle pretty much any packet at this point while we + wait for the qSymbol "response". That requires re-entering the + main loop. For now, this is an adequate approximation; allow + GDB to read from memory while it figures out the address of the + packet. */ + while (own_buf[0] == 'm') { - /* Malformed response. */ - if (remote_debug) - { - fprintf (stderr, "Malformed response to qSymbol, ignoring.\n"); - fflush (stderr); - } + CORE_ADDR mem_addr; + unsigned char *mem_buf; + decode_m_packet (&own_buf[1], &mem_addr, &len); + mem_buf = malloc (len); + if (read_inferior_memory (mem_addr, mem_buf, len) == 0) + convert_int_to_ascii (mem_buf, own_buf, len); + else + write_enn (own_buf); + free (mem_buf); + if (putpkt (own_buf) < 0) + return -1; + len = getpkt (own_buf); + if (len < 0) + return -1; + } + + if (strncmp (own_buf, "qSymbol:", strlen ("qSymbol:")) != 0) + { + warning ("Malformed response to qSymbol, ignoring: %s\n", own_buf); return -1; } Index: solib-svr4.c =================================================================== --- solib-svr4.c (revision 174465) +++ solib-svr4.c (working copy) @@ -85,16 +85,6 @@ static char *solib_break_names[] = "rtld_db_dlactivity", "_rtld_debug_state", - /* On the 64-bit PowerPC, the linker symbol with the same name as - the C function points to a function descriptor, not to the entry - point. The linker symbol whose name is the C function name - prefixed with a '.' points to the function's entry point. So - when we look through this table, we ignore symbols that point - into the data section (thus skipping the descriptor's symbol), - and eventually try this one, giving us the real entry point - address. */ - "._dl_debug_state", - NULL }; @@ -317,7 +307,7 @@ static char *debug_loader_name; static int match_main (char *); -static CORE_ADDR bfd_lookup_symbol (bfd *, char *, flagword); +static CORE_ADDR bfd_lookup_symbol (bfd *, char *); /* @@ -327,24 +317,25 @@ static CORE_ADDR bfd_lookup_symbol (bfd SYNOPSIS - CORE_ADDR bfd_lookup_symbol (bfd *abfd, char *symname, flagword sect_flags) + CORE_ADDR bfd_lookup_symbol (bfd *abfd, char *symname) DESCRIPTION An expensive way to lookup the value of a single symbol for bfd's that are only temporary anyway. This is used by the shared library support to find the address of the debugger - interface structures in the shared library. + notification routine in the shared library. - If SECT_FLAGS is non-zero, only match symbols in sections whose - flags include all those in SECT_FLAGS. + The returned symbol may be in a code or data section; functions + will normally be in a code section, but may be in a data section + if this architecture uses function descriptors. Note that 0 is specifically allowed as an error return (no such symbol). */ static CORE_ADDR -bfd_lookup_symbol (bfd *abfd, char *symname, flagword sect_flags) +bfd_lookup_symbol (bfd *abfd, char *symname) { long storage_needed; asymbol *sym; @@ -366,9 +357,9 @@ bfd_lookup_symbol (bfd *abfd, char *symn { sym = *symbol_table++; if (strcmp (sym->name, symname) == 0 - && (sym->section->flags & sect_flags) == sect_flags) + && (sym->section->flags & (SEC_CODE | SEC_DATA)) != 0) { - /* Bfd symbols are section relative. */ + /* BFD symbols are section relative. */ symaddr = sym->value + sym->section->vma; break; } @@ -395,9 +386,9 @@ bfd_lookup_symbol (bfd *abfd, char *symn sym = *symbol_table++; if (strcmp (sym->name, symname) == 0 - && (sym->section->flags & sect_flags) == sect_flags) + && (sym->section->flags & (SEC_CODE | SEC_DATA)) != 0) { - /* Bfd symbols are section relative. */ + /* BFD symbols are section relative. */ symaddr = sym->value + sym->section->vma; break; } @@ -1105,7 +1096,7 @@ enable_break (void) /* On a running target, we can get the dynamic linker's base address from the shared library table. */ - solib_add (NULL, 0, NULL, auto_solib_add); + solib_add (NULL, 0, ¤t_target, auto_solib_add); so = master_so_list (); while (so) { @@ -1128,7 +1119,7 @@ enable_break (void) debug_loader_name = xstrdup (buf); debug_loader_offset_p = 1; debug_loader_offset = load_addr; - solib_add (NULL, 0, NULL, auto_solib_add); + solib_add (NULL, 0, ¤t_target, auto_solib_add); } /* Record the relocated start and end address of the dynamic linker @@ -1153,20 +1144,19 @@ enable_break (void) /* Now try to set a breakpoint in the dynamic linker. */ for (bkpt_namep = solib_break_names; *bkpt_namep != NULL; bkpt_namep++) { - /* On ABI's that use function descriptors, there are usually - two linker symbols associated with each C function: one - pointing at the actual entry point of the machine code, - and one pointing at the function's descriptor. The - latter symbol has the same name as the C function. - - What we're looking for here is the machine code entry - point, so we are only interested in symbols in code - sections. */ - sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_CODE); + sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep); if (sym_addr != 0) break; } + if (sym_addr != 0) + /* Convert 'sym_addr' from a function pointer to an address. + Because we pass tmp_bfd_target instead of the current + target, this will always produce an unrelocated value. */ + sym_addr = gdbarch_convert_from_func_ptr_addr (current_gdbarch, + sym_addr, + tmp_bfd_target); + /* We're done with both the temporary bfd and target. Remember, closing the target closes the underlying bfd. */ target_close (tmp_bfd_target, 0);