From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 6EAD13858C5E for ; Mon, 7 Nov 2022 13:32:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6EAD13858C5E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1667827953; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=wepHv/1GHNmTU9008YxNH6kgkvZzj7nIB5bcLE/sYsw=; b=bCwFI2BbYqKUulE3TqfM88yBl3SFvOBkXvQHhSI4uOUFhKo2Fa3O9Cie93F3AXNcj66Hu1 FCYeelOAwH/BoYVbkHRpL0Gy8WvM9c1lSe9aMJ61zxPrMkQz/5p4D7wIfMzl/1tLvHkXOF i2LaEu7uImVeXJwpXbETugZ7j0d8mJA= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-669-euOfbiF7PjGJGvLjhxXdxg-1; Mon, 07 Nov 2022 08:32:32 -0500 X-MC-Unique: euOfbiF7PjGJGvLjhxXdxg-1 Received: by mail-wm1-f70.google.com with SMTP id x10-20020a05600c420a00b003cfa33f2e7cso1924226wmh.2 for ; Mon, 07 Nov 2022 05:32:32 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=wepHv/1GHNmTU9008YxNH6kgkvZzj7nIB5bcLE/sYsw=; b=SgiZx2CZS6vJ5hNXtpJpwk2VfjANj3enTLw27SxUbhNvU0gdhKEbOhpQCt8cbjq7Ng M4dB6Ew4l91CEAekRw6x3nEG185wxlKGAjtwP0n7glHeCzBVrY76JF7eR4os7a0F5pqQ 1aEl0zPNsYKL5Ekiej0CKXeOygC/Yn7ZRp4UElRlLSlcgoYR0RRjv098dPOPYOkBaqQA hQN0Uv02QMabPAYyZwNlnvH1/ZkG8IJckfdRKkt0QTwQdrhEzFv4CYZQLl3+YaCbftwM WtFWNtGZEFDuyDBXoSea9KTcFk3z6l+JfKFWrtFw8ePsnKra6xnxqRiTx5+9ocGYaahw qrgQ== X-Gm-Message-State: ACrzQf11vaLR6tCMGLt//aEaqi9Pwjyf+vz0SWG7viwCQEemsTcCM7hi Xxc8qy5h+L07ZMSug2np4szem4l4YWvb3psFboV8JQEIwo/bACuFc6QIKNreVGP8+bNgXw4DzhX h8zA44oltWHG50tnemzv5OQ== X-Received: by 2002:a5d:5d89:0:b0:226:e5ca:4bc2 with SMTP id ci9-20020a5d5d89000000b00226e5ca4bc2mr31517651wrb.310.1667827951110; Mon, 07 Nov 2022 05:32:31 -0800 (PST) X-Google-Smtp-Source: AMsMyM7m/LoGBq+YKEjH73LnuxhicrlJlod21YXpOd8mi8BHgRM8f6WrYqDotJG1gZNSGvxNOZJ5eQ== X-Received: by 2002:a5d:5d89:0:b0:226:e5ca:4bc2 with SMTP id ci9-20020a5d5d89000000b00226e5ca4bc2mr31517640wrb.310.1667827950731; Mon, 07 Nov 2022 05:32:30 -0800 (PST) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id n14-20020adfe34e000000b002366ded5864sm7378926wrj.116.2022.11.07.05.32.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 07 Nov 2022 05:32:30 -0800 (PST) From: Andrew Burgess To: Tom de Vries , Ivan Tetyushkin , gdb-patches@sourceware.org, Andrew Burgess Subject: Re: [PATCH 2/7] [gdb/testsuite] fix test gdb.base/print-file-var.exp for remote execution In-Reply-To: References: <20221025162946.727169-1-ivan.tetyushkin@syntacore.com> <20221025162946.727169-3-ivan.tetyushkin@syntacore.com> Date: Mon, 07 Nov 2022 13:32:28 +0000 Message-ID: <87cz9yc2kz.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Tom de Vries via Gdb-patches writes: > On 10/25/22 18:29, Ivan Tetyushkin wrote: >> --- >> gdb/testsuite/gdb.base/print-file-var.exp | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp >> index 9abe87d7758..73137630fed 100644 >> --- a/gdb/testsuite/gdb.base/print-file-var.exp >> +++ b/gdb/testsuite/gdb.base/print-file-var.exp >> @@ -42,6 +42,8 @@ proc test {hidden dlopen version_id_main lang} { >> set libobj1 [standard_output_file ${lib1}$suffix.so] >> set libobj2 [standard_output_file ${lib2}$suffix.so] >> >> + set runtimelibobj2 [get_runtime_file $libobj2] >> + >> set lib_opts { debug $lang } >> lappend lib_opts "additional_flags=-DHIDDEN=$hidden" >> >> @@ -60,7 +62,7 @@ proc test {hidden dlopen version_id_main lang} { >> set link_opts [list debug shlib=${libobj1}] >> >> if {$dlopen} { >> - lappend main_opts "additional_flags=-DSHLIB_NAME=\"$libobj2\"" >> + lappend main_opts "additional_flags=-DSHLIB_NAME=\"$runtimelibobj2\"" >> lappend link_opts "shlib_load" >> } else { >> lappend link_opts "shlib=${libobj2}" > > I get this test-case passing by avoiding to use an absolute file name: > ... > diff --git a/gdb/testsuite/gdb.base/print-file-var.exp > b/gdb/testsuite/gdb.base/print-file- > var.exp > index 9abe87d7758..338840cb05e 100644 > --- a/gdb/testsuite/gdb.base/print-file-var.exp > +++ b/gdb/testsuite/gdb.base/print-file-var.exp > @@ -60,7 +60,7 @@ proc test {hidden dlopen version_id_main lang} { > set link_opts [list debug shlib=${libobj1}] > > if {$dlopen} { > - lappend main_opts "additional_flags=-DSHLIB_NAME=\"$libobj2\"" > + lappend main_opts "additional_flags=-DSHLIB_NAME=\"[file tail > $libobj2]\"" > lappend link_opts "shlib_load" > } else { > lappend link_opts "shlib=${libobj2}" > ... > which means that the file is found relative to $ORIGIN, as is the case > for $dlopen == 0. > > I'm not entirely happy with this fix, because what we'd rather want is > to use the name as returned by remote_download, but that's done in > gdb_load_shlib later on (which also handles shlib_target_file, so > actually, the name as returned by this proc is the one we really want), > so it's not available yet at the time we do the compilation. > > I've thought about allowing gdb_load_shlib to be called without gdb > instance, and for cases where there's no instance, registering the > solib-search-path setting to be done at gdb start. This will allow us > to move the gdb_load_shlib calls to before the compilation, such that we > can use the result to set -DSHLIB_NAME. But I think it's a solution > bound to cause confusion because things happen under the hood. > > Alternatively, we can try to not pass in the name into compilation > (working around only some of the problems related to remote host > testing, see https://sourceware.org/bugzilla/show_bug.cgi?id=16947), but > that also has its limitations: we need to be able to patch target memory > or the ability to use argv. > > Perhaps splitting up the functionality in gdb_load_shlib makes the most > sense. By default, it'll do the same as before, but by calling it > twice, once with -only-download, and once with -no-download can we get > the solib name before starting gdb. > > I've given this a try in patch attached below. I also prefer the approach that Tom has suggested here. I gave the patch a go and it fixed the problems in print_file-var.exp for me using a local/remote type setup. > > Ivan, could you check if the patch fixes the gdb.base/print-file-var.exp > test-case for you? > > Thanks, > - Tom > From b072645433f83d62a4b7cf857d3d50f0dd7e0c75 Mon Sep 17 00:00:00 2001 > From: Tom de Vries > Date: Sun, 6 Nov 2022 11:26:23 +0100 > Subject: [PATCH 3/3] [gdb/testsuite] Fix gdb.base/print-file-var.exp for > remote target > > --- > gdb/testsuite/gdb.base/print-file-var.exp | 6 ++- > gdb/testsuite/lib/gdb.exp | 48 +++++++++++++++++------ > 2 files changed, 39 insertions(+), 15 deletions(-) > > diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp > index 9abe87d7758..abdbbbfdce4 100644 > --- a/gdb/testsuite/gdb.base/print-file-var.exp > +++ b/gdb/testsuite/gdb.base/print-file-var.exp > @@ -59,8 +59,10 @@ proc test {hidden dlopen version_id_main lang} { > set main_opts [list debug $lang] > set link_opts [list debug shlib=${libobj1}] > > + set target_libobj2 [gdb_load_shlib $libobj2 -only-download] > + > if {$dlopen} { > - lappend main_opts "additional_flags=-DSHLIB_NAME=\"$libobj2\"" > + lappend main_opts "additional_flags=-DSHLIB_NAME=\"$target_libobj2\"" > lappend link_opts "shlib_load" > } else { > lappend link_opts "shlib=${libobj2}" > @@ -79,7 +81,7 @@ proc test {hidden dlopen version_id_main lang} { > > clean_restart $executable > gdb_load_shlib $libobj1 > - gdb_load_shlib $libobj2 > + gdb_load_shlib $libobj2 -no-download > > if ![runto_main] { > return -1 > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp > index e2cda30b95a..5328afb27e8 100644 > --- a/gdb/testsuite/lib/gdb.exp > +++ b/gdb/testsuite/lib/gdb.exp > @@ -5873,24 +5873,46 @@ proc gdb_remote_download {dest fromfile {tofile {}}} { > # > # Copy the listed library to the target. > > -proc gdb_load_shlib { file } { > +proc gdb_load_shlib { file args } { > global gdb_spawn_id The comments on this function will need updating for a final version of this patch. Otherwise, looks good. Thanks, Andrew > > - if ![info exists gdb_spawn_id] { > - perror "gdb_load_shlib: GDB is not running" > + set download 1 > + set solib-search-path 1 > + parse_args { > + {no-download} > + {only-download} > + } > + if { ${no-download} && ${only-download} } { > + perror \ > + "gdb_load_shlib: Cannot use both -no-download and -only-download" > + } > + if { ${no-download} } { > + set download 0 > + } > + if { ${only-download} } { > + set solib-search-path 0 > } > > - set dest [gdb_remote_download target [shlib_target_file $file]] > + set dest "" > + if { $download } { > + 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 > - # libraries. > - # > - # 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 $file]" "" \ > - "set solib-search-path for [file tail $file]" > + if { ${solib-search-path} } { > + if ![info exists gdb_spawn_id] { > + perror "gdb_load_shlib: GDB is not running" > + } > + > + if {[is_remote target]} { > + # If the target is remote, we need to tell gdb where to find the > + # libraries. > + # > + # 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 $file]" "" \ > + "set solib-search-path for [file tail $file]" > + } > } > > return $dest > -- > 2.35.3