From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17230 invoked by alias); 8 Dec 2017 23:43:41 -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 17166 invoked by uid 89); 8 Dec 2017 23:43:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,KAM_SHORT,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 08 Dec 2017 23:43:38 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7BB6180F6C; Fri, 8 Dec 2017 23:43:37 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 828DF60C4A; Fri, 8 Dec 2017 23:43:31 +0000 (UTC) Subject: Re: [PATCH v3.1 1/5] cc-with-tweaks.sh: Use gdb-add-index.sh To: Jan Kratochvil , Simon Marchi References: <149790572259.20186.14601775821404892582.stgit@host1.jankratochvil.net> <149790573738.20186.9976823318110510912.stgit@host1.jankratochvil.net> <20170701152310.GA5857@host1.jankratochvil.net> Cc: gdb-patches@sourceware.org, Victor Leschuk From: Pedro Alves Message-ID: Date: Fri, 08 Dec 2017 23:43:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20170701152310.GA5857@host1.jankratochvil.net> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-12/txt/msg00202.txt.bz2 On 07/01/2017 04:23 PM, Jan Kratochvil wrote: > On Thu, 29 Jun 2017 21:40:32 +0200, Simon Marchi wrote: > >>> + if [ -f ./contrib/gdb-add-index.sh ] >>> + then >>> + GDB_ADD_INDEX="./contrib/gdb-add-index.sh" >>> + elif [ -f ../contrib/gdb-add-index.sh ] >>> + then >>> + GDB_ADD_INDEX="../contrib/gdb-add-index.sh" >>> + elif [ -f ../../contrib/gdb-add-index.sh ] >>> + then >>> + GDB_ADD_INDEX="../../contrib/gdb-add-index.sh" >>> + else >>> + echo "$myname: unable to find usable contrib/gdb-add-index.sh" >&2 >>> + exit 1 >>> + fi >>> + fi >>> +fi >> >> This strategy doesn't work for out of tree builds. > > I use it always with explicit $GDB_ADD_INDEX so this auto-detection does not > really matter to me: > https://git.jankratochvil.net/?p=nethome.git;a=commitdiff;h=44099fb634eca7837d30ab9b6afde0ec2838f705 > > Still, it's not OK to break testing with out of tree builds, which is what most GDB developers use. >> Perhaps cooking up >> something based on $0 would be better? I see that the GDB variable is >> auto-detected the same way, but the gdb binary is found in the build >> directory, so it works for the tests, whereas gdb-add-index.sh is in the >> source directory. >> >> I think it would still be nice to improve how GDB is auto-detected (in >> another patch), > > OK, I believe that is unrelated to this patchset. I am fine to always specify > all the tools explicitly. We must not break out of tree builds testing (with e.g --target_board=dwarf4-gdb-index) at least. I couldn't figure out what this hunk in the patch was for: > --- a/gdb/contrib/cc-with-tweaks.sh > +++ b/gdb/contrib/cc-with-tweaks.sh > @@ -74,6 +74,8 @@ DWP=${DWP:-dwp} > have_link=unknown > next_is_output_file=no > output_file=a.out > +t=/tmp/cc-with-tweaks.$$ > +rm -f $t > so I removed it. > The problem is when no index is produced whether contrib/cc-with-tweaks.sh > should fail or not. As originally contrib/cc-with-tweaks.sh was more quiet > (=successful) than contrib/gdb-add-index.sh and so after this patch the > testsuite runs with an index would "regress". I have tried to keep the > behavior unchanged. Some cases still error with: > Ada is not currently supported by the index > But some cases (such as some trivial gdb.dwarf2/ testcases with no DWARF data > to index) produce no index while the testcases still PASS now instead of: > -PASS: gdb.arch/i386-bp_permanent.exp: stack pointer value matches > +gdb compile failed, gdb-add-index.sh: No index was created for /quadgdb/testsuite.unix.-m64/outputs/gdb.arch/i386-bp_permanent/i386-bp_permanent > +gdb-add-index.sh: [Was there no debuginfo? Was there already an index?] > +UNTESTED: gdb.arch/i386-bp_permanent.exp: failed to compile I think we may want to revisit this -- may be better to skip the tests when testing with an index, using some skip_foo_marker (like the existing skip_python_tests, etc.), so that testing doesn't spend time needlessly on those. But I sympathize with not wanting to change the current behavior. Thus, here's what I pushed. >From 6432ec65a8822246db5bcede0c6bd3ed0e464a0b Mon Sep 17 00:00:00 2001 From: Jan Kratochvil Date: Fri, 8 Dec 2017 22:44:10 +0000 Subject: [PATCH 1/9] cc-with-tweaks.sh: Use gdb-add-index.sh With DWARF-5 .debug_names, the commands to add the index to the symbol file are more complicated, as now also .debug_str needs to be modified. Currently, contrib/cc-with-tweaks.sh calls objcopy to handle the '-i' option instead of using contrib/gdb-add-index.sh which basically does the same. To help with .debug_names, this commit makes contrib/cc-with-tweaks.sh reuse contrib/gdb-add-index.sh instead. A problem this ran into is whether contrib/cc-with-tweaks.sh should fail or not when no index is produced. Currently, contrib/cc-with-tweaks.sh is more quiet (=successful) than contrib/gdb-add-index.sh and so with no further changes testsuite runs with an index would "regress". This commit tries to keep the behavior unchanged. Some cases still error with: Ada is not currently supported by the index But some cases (such as some trivial gdb.dwarf2/ testcases with no DWARF data to index) produce no index while the testcases still PASS now instead of: -PASS: gdb.arch/i386-bp_permanent.exp: stack pointer value matches +gdb compile failed, gdb-add-index.sh: No index was created for gdb/testsuite.unix.-m64/outputs/gdb.arch/i386-bp_permanent/i386-bp_permanent +gdb-add-index.sh: [Was there no debuginfo? Was there already an index?] +UNTESTED: gdb.arch/i386-bp_permanent.exp: failed to compile gdb/ChangeLog 2017-12-08 Jan Kratochvil Pedro Alves * contrib/cc-with-tweaks.sh: Change interpreter to bash, incl. initial comment. (GDB_ADD_INDEX): New variable. <$want_index>: Call $GDB_ADD_INDEX. --- gdb/ChangeLog | 8 ++++++++ gdb/contrib/cc-with-tweaks.sh | 40 +++++++++++++++++++++++----------------- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 26cf18e..e90a3f0 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,11 @@ +2017-12-08 Jan Kratochvil + Pedro Alves + + * contrib/cc-with-tweaks.sh: Change interpreter to bash, incl. initial + comment. + (GDB_ADD_INDEX): New variable. + <$want_index>: Call $GDB_ADD_INDEX. + 2017-12-08 Sergio Durigan Junior * dtrace-probe.c (dtrace_process_dof_probe): Do not declare a new diff --git a/gdb/contrib/cc-with-tweaks.sh b/gdb/contrib/cc-with-tweaks.sh index 7d39c00..a65fef8 100755 --- a/gdb/contrib/cc-with-tweaks.sh +++ b/gdb/contrib/cc-with-tweaks.sh @@ -1,4 +1,4 @@ -#! /bin/sh +#!/usr/bin/env bash # Wrapper around gcc to tweak the output in various ways when running # the testsuite. @@ -27,8 +27,8 @@ # # bash$ cd $objdir/gdb/testsuite # bash$ runtest \ -# CC_FOR_TARGET="/bin/sh $srcdir/gdb/contrib/cc-with-tweaks.sh ARGS gcc" \ -# CXX_FOR_TARGET="/bin/sh $srcdir/gdb/contrib/cc-with-tweaks.sh ARGS g++" +# CC_FOR_TARGET="/bin/bash $srcdir/gdb/contrib/cc-with-tweaks.sh ARGS gcc" \ +# CXX_FOR_TARGET="/bin/bash $srcdir/gdb/contrib/cc-with-tweaks.sh ARGS g++" # # For documentation on Fission and dwp files: # http://gcc.gnu.org/wiki/DebugFission @@ -47,6 +47,7 @@ # If nothing is given, no changes are made myname=cc-with-tweaks.sh +mydir=`dirname "$0"` if [ -z "$GDB" ] then @@ -93,6 +94,20 @@ while [ $# -gt 0 ]; do shift done +if [ "$want_index" = true ] +then + if [ -z "$GDB_ADD_INDEX" ] + then + if [ -f $mydir/gdb-add-index.sh ] + then + GDB_ADD_INDEX="$mydir/gdb-add-index.sh" + else + echo "$myname: unable to find usable contrib/gdb-add-index.sh" >&2 + exit 1 + fi + fi +fi + for arg in "$@" do if [ "$next_is_output_file" = "yes" ] @@ -152,20 +167,11 @@ if [ "$want_objcopy_compress" = true ]; then fi if [ "$want_index" = true ]; then - $GDB --batch-silent -nx -ex "set auto-load no" -ex "file $output_file" -ex "save gdb-index $output_dir" - rc=$? - [ $rc != 0 ] && exit $rc - - # GDB might not always create an index. Cope. - if [ -f "$index_file" ] - then - $OBJCOPY --add-section .gdb_index="$index_file" \ - --set-section-flags .gdb_index=readonly \ - "$output_file" "$output_file" - rc=$? - else - rc=0 - fi + # Filter out these messages which would stop dejagnu testcase run: + # echo "$myname: No index was created for $file" 1>&2 + # echo "$myname: [Was there no debuginfo? Was there already an index?]" 1>&2 + GDB=$GDB $GDB_ADD_INDEX "$output_file" 2>&1|grep -v "^${GDB_ADD_INDEX##*/}: " >&2 + rc=${PIPESTATUS[0]} [ $rc != 0 ] && exit $rc fi -- 2.5.5