From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17738 invoked by alias); 20 Jul 2011 20:53:35 -0000 Received: (qmail 17729 invoked by uid 22791); 20 Jul 2011 20:53:34 -0000 X-SWARE-Spam-Status: No, hits=-7.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_BJ X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 20 Jul 2011 20:53:18 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p6KKrHq4005082 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 20 Jul 2011 16:53:17 -0400 Received: from host1.jankratochvil.net (ovpn-116-20.ams2.redhat.com [10.36.116.20]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p6KKrFYT029807 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 20 Jul 2011 16:53:17 -0400 Received: from host1.jankratochvil.net (localhost [127.0.0.1]) by host1.jankratochvil.net (8.14.4/8.14.4) with ESMTP id p6KKrEfZ029829; Wed, 20 Jul 2011 22:53:14 +0200 Received: (from jkratoch@localhost) by host1.jankratochvil.net (8.14.4/8.14.4/Submit) id p6KKrEb4029823; Wed, 20 Jul 2011 22:53:14 +0200 Date: Wed, 20 Jul 2011 22:01:00 -0000 From: Jan Kratochvil To: Sergio Durigan Junior Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] On-demand loading of shlib's debuginfo Message-ID: <20110720205313.GA2611@host1.jankratochvil.net> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) 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: 2011-07/txt/msg00566.txt.bz2 Hi Sergio, it crashes for me on fedora-rawhide-x86_64: $ echo 'main(){pause();}'|gcc -g -x c -;./gdb -nx ./a.out -ex 'set solib-add lazy' -ex start -ex 'list pause' [...] #3 in extract_typed_address (buf=0x10
, type=0x2000d60) at findvar.c:180 #4 in lm_dynamic_from_link_map (so=0x2009c00) at solib-svr4.c:169 [...] On Tue, 19 Jul 2011 00:23:49 +0200, Sergio Durigan Junior wrote: > With that in mind, we decided to tackle this problem progressively, and the > first part of the solution is ready for submission. That is currently it still touches the solib files on disk for the purpose of solib_map_sections so that will be a different patch, looking forward. > 1) Change the name of the `auto-solib-add' command to just `solib-add' > (deprecatin `auto-solib-add'). Also, make the command be a tri-state, > with `on', `off', and `lazy' options. The default option is still `on'. I do not understand why you haven't kept the original name, just extending it to be tri-state. Otherwise the auto_solib_add -> solib_add_opt rename could be in a separate mechanical patch. > 3) This patch only affects the `backtrace' command for now, Do you have some benchmark of the benefits? But without the solib_map_sections part it may not yet been so significant I guess. > --- a/gdb/breakpoint.c > +++ b/gdb/breakpoint.c > @@ -4593,9 +4593,9 @@ bpstat_what (bpstat bs_head) > target_terminal_ours_for_output (); > > #ifdef SOLIB_ADD > - SOLIB_ADD (NULL, 0, ¤t_target, auto_solib_add); > + SOLIB_ADD (NULL, 0, ¤t_target, solib_add_opt, /*lazy_read=*/0); nitpick: I find - except one such case - in GDB the style: , 0 /* lazy_read */); +everywhere. > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -14948,14 +14948,18 @@ will be loaded automatically when the inferior begins execution, you > attach to an independently started inferior, or when the dynamic linker > informs @value{GDBN} that a new library has been loaded. If @var{mode} > is @code{off}, symbols must be loaded manually, using the > -@code{sharedlibrary} command. The default value is @code{on}. > +@code{sharedlibrary} command. If @var{mode} is @code{lazy}, then > +@value{GDBN} will lazily load symbols from shared libraries, i.e., it > +will load symbols on-demand. The default value is @code{on}. The crash reproducer at the top should have shown the "lazy" mode has limited applicability now - which is also probably why it is not a default. The lazy hook is now there only for backtrace so the documentation should describe it works only for backtraces. It could be applied to /usr/bin/gstack but that is a Fedora only command/patch so far. > +This command is now deprecated. Use @code{set solib-add} instead. This `auto-solib-add' gets deprecated but I do not see the new `solib-add' setting described in the doc. > --- a/gdb/solib-frv.c > +++ b/gdb/solib-frv.c > @@ -1302,7 +1302,7 @@ frv_fetch_objfile_link_map (struct objfile *objfile) > > /* Cause frv_current_sos() to be run if it hasn't been already. */ > if (main_lm_addr == 0) > - solib_add (0, 0, 0, 1); > + solib_add (0, 0, 0, 1, /*lazy_read=*/0); 1 should be SOLIB_ADD_ON instead. > --- a/gdb/solib-svr4.c > +++ b/gdb/solib-svr4.c > @@ -2449,6 +2451,29 @@ elf_lookup_lib_symbol (const struct objfile *objfile, > return lookup_global_symbol_from_objfile (objfile, name, domain); > } > > +/* Given PC, try to match a so_list which contains it. > + See match_pc_solist in solist.h. */ > + > +static struct so_list * > +svr4_match_pc_solist (CORE_ADDR pc, struct so_list *so) > +{ > + struct so_list *iter, *res = NULL; > + CORE_ADDR cur = CORE_ADDR_MAX; > + > + for (iter = so; iter; iter = iter->next) > + { > + CORE_ADDR addr = lm_dynamic_from_link_map (iter); > + > + if (addr >= pc && addr < cur) Maybe a note "cur" is in the DYNAMIC segment while PC is in the LOAD segment and normally LOAD segment is under DYNAMIC segment; which is why the conditional looks like reversed. > + { > + cur = addr; > + res = iter; > + } > + } > + > + return res; > +} > + > --- a/gdb/solib.c > +++ b/gdb/solib.c > @@ -888,6 +900,19 @@ libpthread_solib_p (struct so_list *so) > return libpthread_name_p (so->so_name); > } > > +void > +solib_on_demand_load (CORE_ADDR pc) Missing function comment. > +{ > + struct so_list *solib; > + > + /* On-demand loading of shared libraries' debuginfo. */ > + solib = solib_match_pc_solist (pc); > + > + if (solib && !solib->symbols_loaded) > + solib_add (solib->so_name, 0, ¤t_target, 1, > + /*lazy_read=*/1); 1 should be SOLIB_ADD_ON instead. > +} > + > /* GLOBAL FUNCTION > > solib_add -- read in symbol info for newly added shared libraries > @@ -967,7 +1006,7 @@ solib_add (char *pattern, int from_tty, > printf_unfiltered > ("No loaded shared libraries match the pattern `%s'.\n", pattern); > > - if (loaded_any_symbols) > + if (loaded_any_symbols && !lazy_read) I think it can break sunos_special_symbol_handling, that should still be called there or to disable lazy_read functionality on sunos or so. > { > struct target_so_ops *ops = solib_ops (target_gdbarch); > > @@ -1265,6 +1304,28 @@ in_solib_dynsym_resolve_code (CORE_ADDR pc) > return ops->in_dynsym_resolve_code (pc); > } > > +/* GLOBAL FUNCTION > + > + solib_match_pc_solist -- check to see to which so_list PC belongs. > + > + SYNOPSIS > + > + struct so_list *solib_match_pc_solist (CORE_ADDR pc) > + > + DESCRIPTION > + > + Determine to which so_list the given PC belongs. Returns the > + so_list if found, NULL otherwise. > +*/ > + > +struct so_list * > +solib_match_pc_solist (CORE_ADDR pc) > +{ > + struct target_so_ops *ops = solib_ops (target_gdbarch); > + > + return ops->match_pc_solist (pc, so_list_head); On solib platforms where match_pc_solist is not defined it will crash. > +} > + > /* > > LOCAL FUNCTION > @@ -1283,7 +1344,7 @@ static void > sharedlibrary_command (char *args, int from_tty) > { > dont_repeat (); > - solib_add (args, from_tty, (struct target_ops *) 0, 1); > + solib_add (args, from_tty, (struct target_ops *) 0, 1, /*lazy_read=*/0); 1 should be SOLIB_ADD_ON instead. Thanks, Jan