From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f42.google.com (mail-wr1-f42.google.com [209.85.221.42]) by sourceware.org (Postfix) with ESMTPS id 0679E3888C6B for ; Fri, 18 Mar 2022 17:38:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0679E3888C6B Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wr1-f42.google.com with SMTP id r10so12675084wrp.3 for ; Fri, 18 Mar 2022 10:38:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:from:to:cc:references:in-reply-to :content-transfer-encoding; bh=+qumnv/UMzVfwXSNrTQFc6Vb6VzE+5hJxgBrNmiTiTY=; b=WFBJdczAVYz1E3E4F731q5mHWfYXY7w36rYs9inGduNOX30LvwX5vQ/zUwL/gfyRDe JhSs5J3HrBoyuZ9oHIlIhFpUZlYYjNVaJLfm6Ra+bi5ie5b2jDrcG4vmP4BBESI+D33d wpidiuXtGfLsXOFvqWLzvgBedyo8G0qpDw7cBpunhPKrHIZi/HoYxHSbf4Obt0vNovwf zDHHjf62O/xIWn0jFervvcst3Xot3ek0+Ab23G1RQkRm4LiDw1MMhK4MYWe16TYFLKiE 5MnfbbTsw+D7lBTU5lLu8LUYixrWPKORE/12Uv8IQxWRyZoIMvBg3sRdoCk60G8Foa13 q4Iw== X-Gm-Message-State: AOAM530N11uQYB1W9zgIsFwbV0faEqZKQbiZ7ypELR+P7b5JjQQSLi2s OhvydNMXbrGYOrpRi8ymU6X9jdZK1Go= X-Google-Smtp-Source: ABdhPJzkVzQTvu8Y2vttmDJ/S3U61MPXHBxmBQQERfsyH0w2jxSa16IEp/lxggD+6p8cj1AMrFSuWA== X-Received: by 2002:a05:6000:1849:b0:203:d6ae:b4b8 with SMTP id c9-20020a056000184900b00203d6aeb4b8mr9396176wri.542.1647625080820; Fri, 18 Mar 2022 10:38:00 -0700 (PDT) Received: from ?IPV6:2001:8a0:f924:2600:209d:85e2:409e:8726? ([2001:8a0:f924:2600:209d:85e2:409e:8726]) by smtp.gmail.com with ESMTPSA id l15-20020a05600c1d0f00b0038c8ff8e708sm973041wms.13.2022.03.18.10.37.59 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 18 Mar 2022 10:37:59 -0700 (PDT) Message-ID: Date: Fri, 18 Mar 2022 17:37:58 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Subject: [PATCH v2] gdb/testsuite: Installed-GDB testing & data-directory Content-Language: en-US From: Pedro Alves To: Andrew Burgess Cc: gdb-patches@sourceware.org References: <20220318161036.3290639-1-pedro@palves.net> <20220318162318.GR1212730@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-9.8 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 Mar 2022 17:38:04 -0000 On 2022-03-18 16:43, Pedro Alves wrote: > Do you disagree that what we suggest in testsuite/README: > > make check RUNTESTFLAGS=GDB=/usr/bin/gdb > > behaves incorrectly (in the sense that it does unsupportable things), > and we should instead suggest something that works correctly by default? > > My first approach was to just let users override GDB_DATA_DIRECTORY, and > thus tweak the README to read: > > make check RUNTESTFLAGS="GDB=/usr/bin/gdb GDB_DATA_DIRECTORY=" > > but then extending the README text to explain this felt pretty awkward and I > realized that we should probably default GDB_DATA_DIRECTORY to "" if > GDB was overridden, and then things would work for 99% of the users. > > I admit I didn't think people were using this the way you are using it. Below's what such an approach looks like. This works for us as well, we just have to pass the GDB_DATA_DIRECTORY= explicitly, which isn't as nice, but isn't a big deal. Let me know what you think. >From e701a7ea7812cf60a0a146c705f4ba27dd31ad11 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Fri, 18 Mar 2022 12:39:55 +0000 Subject: [[PATCH v2]] gdb/testsuite: Installed-GDB testing & data-directory In testsuite/README, we suggest that you can run the testsuite against some other GDB binary by using: make check RUNTESTFLAGS=GDB=/usr/bin/gdb However, that example isn't fully correct, because with that command line, the testsuite will still pass -data-directory=[pwd]/../data-directory to /usr/bin/gdb, like e.g.: ... builtin_spawn /usr/bin/gdb -nw -nx -data-directory /home/pedro/gdb/build/gdb/testsuite/../data-directory -iex set height 0 -iex set width 0 ... while if you're testing an installed GDB (the system GDB being the most usual scenario), then you should normally let it use its own configured directory, not the just-built GDB's data directory. This commit improves the status quo by letting the user override the data directory, via a new GDB_DATA_DIRECTORY global. This replaces the existing BUILD_DATA_DIRECTORY variable in testsuite/lib/gdb.exp, which wasn't overridable, and was a bit misnamed for the new purpose. Here are some use case examples: # Test the non-installed GDB in some build dir: make check \ RUNTESTFLAGS="GDB=/path/to/other/build/gdb \ GDB_DATA_DIRECTORY=/path/to/other/build/gdb/data-directory" # Test the GDB installed in some prefix: make check \ RUNTESTFLAGS="GDB=/opt/gdb/bin/gdb GDB_DATA_DIRECTORY=" # Test the built GDB with some alternative data directory, e.g., the system GDB's data directory: make check \ RUNTESTFLAGS="GDB_DATA_DIRECTORY=/usr/share/gdb" Change-Id: Icdc21c85219155d9564a9900961997e6624b78fb --- gdb/testsuite/README | 29 +++++++++++++++++++++++++---- gdb/testsuite/lib/gdb.exp | 37 +++++++++++++++++++++++++++++++------ 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/gdb/testsuite/README b/gdb/testsuite/README index c2f659a7188..cae78fe7946 100644 --- a/gdb/testsuite/README +++ b/gdb/testsuite/README @@ -13,14 +13,17 @@ There are two ways to run the testsuite and pass additional parameters to DejaGnu. The first is to do `make check' in the main build directory and specifying the makefile variable `RUNTESTFLAGS': - make check RUNTESTFLAGS='GDB=/usr/bin/gdb gdb.base/a2-run.exp' + make check \ + RUNTESTFLAGS='GDB=/usr/bin/gdb \ + GDB_DATA_DIRECTORY= \ + gdb.base/a2-run.exp' The second is to cd to the testsuite directory and invoke the DejaGnu `runtest' command directly. cd testsuite make site.exp - runtest GDB=/usr/bin/gdb + runtest GDB=/usr/bin/gdb GDB_DATA_DIRECTORY= (The `site.exp' file contains a handful of useful variables like host and target triplets, and pathnames.) @@ -151,9 +154,10 @@ By default, the testsuite exercises the GDB in the build directory, but you can set GDB to be a pathname to a different version. For instance, - make check RUNTESTFLAGS=GDB=/usr/bin/gdb + make check RUNTESTFLAGS="GDB=/usr/bin/gdb GDB_DATA_DIRECTORY=" -runs the testsuite on the GDB in /usr/bin. +runs the testsuite on the GDB installed in /usr/bin. See below for +the description of GDB_DATA_DIRECTORY. GDBSERVER @@ -164,6 +168,23 @@ instance checks both the installed GDB and GDBserver. +GDB_DATA_DIRECTORY + +By default the tested GDB uses the data directory found under the GDB +build directory. You can override this by setting GDB_DATA_DIRECTORY. +For instance: + + make check \ + RUNTESTFLAGS="GDB=/path/to/other/build/gdb \ + GDB_DATA_DIRECTORY=/path/to/other/build/gdb/data-directory" + +If GDB_DATA_DIRECTORY is set to the empty string, then the testsuite +lets GDB use its configured data directory. This is useful to test an +installed GDB. For example: + + make check \ + RUNTESTFLAGS="GDB=/usr/bin/gdb GDB_DATA_DIRECTORY=" + INTERNAL_GDBFLAGS Command line options passed to all GDB invocations. diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index 08726f78563..89011336b6d 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -92,8 +92,13 @@ load_lib gdb-utils.exp load_lib memory.exp load_lib check-test-names.exp +# The path to the GDB binary to test. global GDB +# The data directory to use for testing. If this is the empty string, +# then we let GDB use its own configured data directory. +global GDB_DATA_DIRECTORY + # The spawn ID used for I/O interaction with the inferior. For native # targets, or remote targets that can do I/O through GDB # (semi-hosting) this will be the same as the host/GDB's spawn ID. @@ -114,6 +119,14 @@ if ![info exists GDB] { } verbose "using GDB = $GDB" 2 +# The data directory the testing GDB will use. By default, assume +# we're testing a non-installed GDB in the build directory. Users may +# also explictly override the -data-directory from the command line. +if ![info exists GDB_DATA_DIRECTORY] { + set GDB_DATA_DIRECTORY "[pwd]/../data-directory" +} +verbose "using GDB_DATA_DIRECTORY = $GDB_DATA_DIRECTORY" 2 + # GDBFLAGS is available for the user to set on the command line. # E.g. make check RUNTESTFLAGS=GDBFLAGS=mumble # Testcases may use it to add additional flags, but they must: @@ -125,23 +138,35 @@ if ![info exists GDBFLAGS] { } verbose "using GDBFLAGS = $GDBFLAGS" 2 -# Make the build data directory available to tests. -set BUILD_DATA_DIRECTORY "[pwd]/../data-directory" +# Append the -data-directory option to pass to GDB to CMDLINE and +# return the resulting string. If GDB_DATA_DIRECTORY is empty, +# nothing is appended. +proc append_gdb_data_directory_option {cmdline} { + global GDB_DATA_DIRECTORY + + if { $GDB_DATA_DIRECTORY != "" } { + return "$cmdline -data-directory $GDB_DATA_DIRECTORY" + } else { + return $cmdline + } +} # INTERNAL_GDBFLAGS contains flags that the testsuite requires. # `-nw' disables any of the windowed interfaces. # `-nx' disables ~/.gdbinit, so that it doesn't interfere with the tests. -# `-data-directory' points to the data directory in the build directory. # `-iex "set {height,width} 0"' disables pagination. +# `-data-directory' points to the data directory, usually in the build +# directory. global INTERNAL_GDBFLAGS if ![info exists INTERNAL_GDBFLAGS] { set INTERNAL_GDBFLAGS \ [join [list \ "-nw" \ "-nx" \ - "-data-directory $BUILD_DATA_DIRECTORY" \ {-iex "set height 0"} \ {-iex "set width 0"}]] + + set INTERNAL_GDBFLAGS [append_gdb_data_directory_option $INTERNAL_GDBFLAGS] } # The variable gdb_prompt is a regexp which matches the gdb prompt. @@ -8021,9 +8046,9 @@ proc verify_psymtab_expanded { filename readin } { # string (the default) means .gdb_index; "-dwarf-5" means .debug_names. proc add_gdb_index { program {style ""} } { - global srcdir GDB env BUILD_DATA_DIRECTORY + global srcdir GDB env set contrib_dir "$srcdir/../contrib" - set env(GDB) "$GDB --data-directory=$BUILD_DATA_DIRECTORY" + set env(GDB) [append_gdb_data_directory_option $GDB] set result [catch "exec $contrib_dir/gdb-add-index.sh $style $program" output] if { $result != 0 } { verbose -log "result is $result" base-commit: 0a30596cfad9cd221a81eea984b6fe3fabb20b95 -- 2.26.2