From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20370 invoked by alias); 30 May 2013 17:18:14 -0000 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 Received: (qmail 20358 invoked by uid 89); 30 May 2013 17:18:12 -0000 X-Spam-SWARE-Status: No, score=-8.1 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS,TW_XS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Thu, 30 May 2013 17:18:11 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r4UHIAwH022713 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 30 May 2013 13:18:10 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r4UHI8sL012427; Thu, 30 May 2013 13:18:08 -0400 Message-ID: <51A789CF.2000302@redhat.com> Date: Thu, 30 May 2013 17:18:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 MIME-Version: 1.0 To: Jan Kratochvil , gdb-patches@sourceware.org Subject: Re: [RFA 5/7 take 3] Improved linker-debugger interface References: <20130516144340.GA2105@blade.nx> <20130516144838.GF2105@blade.nx> <20130517185002.GA10447@host2.jankratochvil.net> <20130524083044.GC4602@blade.nx> <51A64E24.2020901@redhat.com> <20130530104346.GA6217@blade.nx> In-Reply-To: <20130530104346.GA6217@blade.nx> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-05/txt/msg01088.txt.bz2 On 05/30/2013 11:43 AM, Gary Benson wrote: > Pedro Alves wrote: >> On 05/24/2013 09:30 AM, Gary Benson wrote: >>> +static int >>> +svr4_update_solib_event_breakpoint (struct breakpoint *b, void *arg) >>> +{ >>> + struct svr4_info *info = get_svr4_info (); >>> + struct bp_location *loc; >>> + >> ... >> >>> + for (loc = b->loc; loc; loc = loc->next) >>> + { >>> + struct probe_and_action *pa = solib_event_probe_at (info, loc->address); >> >> There's no relation between INFO and the locations' inferior/program >> space. IOW, I believe this is doing the wrong thing for multi-process. > > It took me a while to figure out what you meant here, but I think I > see what you mean. Sorry. :-/ For those following along at home, the issue is that get_svr4_info () returns the svr4 state object corresponding to the current program space, while here we're walking over all locations of all breakpoints, including breakpoints/locations of all inferiors. So in the 'solib_event_probe_at (info, loc->address)' call, we need to pass it the 'info' of the program space of 'loc'. > I've attached an updated patch with this function > reworked. Could you let me know if it looks correct? ... +/* Helper function for svr4_update_solib_event_breakpoints. */ + +static int +svr4_update_solib_event_breakpoint (struct breakpoint *b, void *arg) +{ + struct bp_location *loc; + + if (b->type != bp_shlib_event) + { + /* Continue iterating. */ + return 0; + } + + for (loc = b->loc; loc != NULL; loc = loc->next) + { + struct svr4_info *info; + struct probe_and_action *pa; + + info = program_space_data (loc->pspace, solib_svr4_pspace_data); + if (info == NULL || info->probes_table == NULL) + continue; Exactly. Excellent, looks great now. On 05/30/2013 11:43 AM, Gary Benson wrote:> + { > + char name[32] = { '\0' }; > + > + /* Fedora 17 and Red Hat Enterprise Linux 6.2-6.4 > + shipped with an early version of the probes code in > + which the probes' names were prefixed with "rtld_" > + and the "map_failed" probe did not exist. The > + locations of the probes are otherwise the same, so > + we check for probes with prefixed names if probes > + with unprefixed names are not present. */ > + > + if (with_prefix) > + strncat (name, "rtld_", sizeof (name) - strlen (name) - 1); > + > + strncat (name, probe_info[i].name, > + sizeof (name) - strlen (name) - 1); > + BTW, I'd rather this was written as something like: char name[32]; if (with_prefix) xsnprintf (name, sizeof (name), "rtld_%s", probe_info[i].name); else xsnprintf (name, sizeof (name), "%s", probe_info[i].name); strncat like that is really not future proof, as we'll just get get a silently truncated string if a new probe appears with a name that is too big. xsnprintf OTOH gdb_asserts the string fits in the buffer. Granted, whoever added such a probe would probably notice this, but it's a style best avoided. (Witness how so few strncat calls there are in gdb. There are more strcat calls, but that only shows that whenever we _do_ care about string overruns, we tend to use something else, not strncat.) Plus, it looks more readable to me that way. :-) Alternatively, we could add a xstrncat that asserts truncation never happens, though xsnprintf in this case still looks a bit more readable to me :-) -- Pedro Alves