From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19985 invoked by alias); 13 Apr 2016 21:05:36 -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 19961 invoked by uid 89); 13 Apr 2016 21:05:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY autolearn=no version=3.3.2 spammy=favorite, remotely X-HELO: usplmg21.ericsson.net Received: from usplmg21.ericsson.net (HELO usplmg21.ericsson.net) (198.24.6.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Wed, 13 Apr 2016 21:05:24 +0000 Received: from EUSAAHC007.ericsson.se (Unknown_Domain [147.117.188.93]) by usplmg21.ericsson.net (Symantec Mail Security) with SMTP id 4A.F3.03614.A64BE075; Wed, 13 Apr 2016 23:04:42 +0200 (CEST) Received: from [142.133.110.144] (147.117.188.8) by smtp-am.internal.ericsson.com (147.117.188.95) with Microsoft SMTP Server id 14.3.248.2; Wed, 13 Apr 2016 17:05:21 -0400 Subject: Re: [PATCH 1/2] Change gdb_load_shlibs to gdb_load_shlib To: Pedro Alves , References: <1460502865-10999-1-git-send-email-simon.marchi@ericsson.com> <570E8D61.3010100@redhat.com> CC: , From: Simon Marchi Message-ID: <570EB491.7090001@ericsson.com> Date: Wed, 13 Apr 2016 21:05:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <570E8D61.3010100@redhat.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-04/txt/msg00309.txt.bz2 On 16-04-13 02:18 PM, Pedro Alves wrote: > On 04/13/2016 12:14 AM, Simon Marchi wrote: >> Note: this series should be applied on top of >> https://sourceware.org/ml/gdb-patches/2016-04/msg00252.html >> >> ------- >> >> This patch renames gdb_load_shlibs to gdb_load_shlib, and changes its >> arguments so that it can only take a single library at the time. >> >> The following patch requires to be able to get the destination path of a >> loaded shared library, so it's more straightforward if the procedure can >> only accept one argument. >> >> On top of the mass renaming, there are a handful of locations where I >> had to split up a gdb_load_shlibs in two gdb_load_shlib. >> > > I'd have been nicer and more focused if you split this in two: > > #1 - make gdb_load_shlibs accept only one shlib. > #2 - do the trivial mass rename. Right, I'll have it split in v2. >> -proc gdb_load_shlibs { args } { >> - foreach file $args { >> - gdb_remote_download target [shlib_target_file $file] >> - } >> +proc gdb_load_shlib { file } { >> + set dest [gdb_remote_download target [shlib_target_file $file]] >> >> if {[is_remote target]} { >> # If the target is remote, we need to tell gdb where to find the >> @@ -4237,8 +4235,10 @@ proc gdb_load_shlibs { args } { >> # We could set this even when not testing remotely, but a user >> # generally won't set it unless necessary. In order to make the tests >> # more like the real-life scenarios, we don't set it for local testing. >> - gdb_test "set solib-search-path [file dirname [lindex $args 0]]" "" "" >> + gdb_test "set solib-search-path [file dirname [lindex $file 0]]" "" "" > > $file is no longer a list, so [lindex $file 0] can just be $file. Right, fixed. > With: > > gdb_load_shlib $libobj1 > gdb_load_shlib $libobj2 > > We'll now only end up with $libobj2's dirname in the solib-search-path. > This usually won't be a problem since the dirnames will be the same, > though I guess some test might be doing something with subdirs. > Grepping a bit I found solib-search.exp, though it's currently disabled > on remote testing. Ah that's true. For some reason I thought it appended to the list in GDB. In the original implementation, it only used the dirname of the first passed shared library, didn't it? So the behavior will change (we will keep the directory of the last one, where we used to keep the directory of the first one), but is the situation worse than it was? > Do we need to maintain a list of dirnames, and clear it somewhere? If a test case needs to have multiple different values in solib-search-path, we have multiple options. Option #1 Maintain a list somewhere in the TCL code. It's not my favorite, because we already have a ton of global stuff hard to keep track of. Option #2 Get the current value using "show solib-search-path" and append the new value to it. Option #3 Initially I thought of a lazy way to achieve what I want. I thought to make gdb_load_shlibs return a list of the destination paths (one for each passed solib). This way I wouldn't have had to modify all the callers. If we used this approach, we could build the list of all the directories and pass that to set solib-search-path. Options #4 Maybe it's not a big deal, tests that do some special solib path stuff can just override solib-search-path as they see fit. The setting of solib-search-path in gdb_load_shlib[s] is only for convenience in the general case. Simon