From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 121718 invoked by alias); 19 Feb 2016 15:05:18 -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 121695 invoked by uid 89); 19 Feb 2016 15:05:17 -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=*arm*, H*r:0500 X-HELO: usplmg20.ericsson.net Received: from usplmg20.ericsson.net (HELO usplmg20.ericsson.net) (198.24.6.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Fri, 19 Feb 2016 15:05:12 +0000 Received: from EUSAAHC001.ericsson.se (Unknown_Domain [147.117.188.75]) by usplmg20.ericsson.net (Symantec Mail Security) with SMTP id F6.1E.12433.83B27C65; Fri, 19 Feb 2016 15:48:24 +0100 (CET) Received: from elxa4wqvvz1 (147.117.188.8) by smtps-am.internal.ericsson.com (147.117.188.75) with Microsoft SMTP Server (TLS) id 14.3.248.2; Fri, 19 Feb 2016 10:05:09 -0500 References: <1455884629-23354-1-git-send-email-koriakin@0x04.net> User-agent: mu4e 0.9.17; emacs 24.4.1 From: Antoine Tremblay To: Marcin =?utf-8?Q?Ko=C5=9Bcielnicki?= CC: , Subject: Re: [PATCH] gdb.trace: Move more target dependencies to trace-support.exp In-Reply-To: <1455884629-23354-1-git-send-email-koriakin@0x04.net> Date: Fri, 19 Feb 2016 15:05:00 -0000 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2016-02/txt/msg00602.txt.bz2 Marcin Kościelnicki writes: > While groveling through the old PPC64 tracepoint support patch, I've > noticed a few target dependencies in the testsuite that both me and > Antoine missed for s390 and ARM tracepoints, respectively. This patch > moves them all to one place, so that anyone working on a new target > will hopefully see the whole set of needed changes. Thanks for this, it seems I indeed had missed a few, it also fixes another issue in the arm tracepoints were I was using is_aarch32_target rather then istarget[*arm*]. It's much more clear like that. > > I've also removed a check for fast tracepoint support in ftrace.exp - > it used hardcoded targets and wasn't doing anything useful anyway, > since unsupported architectures blow up on link due to missing > the IPA library before they ever get to that check. > Maybe this should be another patch ? , Obvious ? > For some strange reason, the call_insn setting code already knew about > arm, powerpc, s390, and mips - I went ahead and added the remaining > information about those. I'm not particularly sure if I got mips right, > but that won't matter anyway until someone actually writes tracepoint > support for that. > > In addition, the PPC64 tracepoint patch added \y at the end of the call_insn > pattern - without that, it embarassed itself and matched the 'bl' in > "Dump of assem*bl*er code for function" as the powerpc call opcode. Since > that sounds like a generally good idea, I've added \y before and after > call_insn for every target. As a result, I had to change x86_64's mnemonic > to 'callq'. > Also this could be another patch ? > Tested on x86_64, i386, ppc, ppc64, ppc64le, s390, s390x. Would be good > to test it on aarch64. > > diff --git a/gdb/testsuite/lib/trace-support.exp b/gdb/testsuite/lib/trace-support.exp > index f593c43..829786b 100644 > --- a/gdb/testsuite/lib/trace-support.exp > +++ b/gdb/testsuite/lib/trace-support.exp > @@ -20,26 +20,99 @@ > > > # > -# Program counter / stack pointer / frame pointer for supported targets. > -# Used in many tests, kept here to avoid duplication. > +# Target-specific information. Used in many tests, kept here > +# to avoid duplication and make it easier to add a new target. > # > > if [is_amd64_regs_target] { > + # Frame pointer. > set fpreg "rbp" > + # Stack pointer. > set spreg "rsp" > + # Program counter. > set pcreg "rip" > + # How to collect the first argument to a function. Used to test > + # register usage in tracepoint conditions. > + set arg0exp "\$rdi" > + # The mnemonic of the usual, unconditional call instruction. > + set call_insn "callq" > + # Number of the PC register. > + set pcnum 16 > + # Number of any GPR (it's supposed not to be some register that's not > + # collected by default). > + set gpr0num 0 > } elseif [is_x86_like_target] { > set fpreg "ebp" > set spreg "esp" > set pcreg "eip" > + set arg0exp "*(int *) (\$ebp + 8)" > + set call_insn "call" > + set pcnum 8 > + set gpr0num 0 > } elseif [is_aarch64_target] { > set fpreg "x29" > set spreg "sp" > set pcreg "pc" > + set arg0exp "\$x0" > + set call_insn "bl" > + set pcnum 32 > + set gpr0num 0 > +} elseif [istarget "arm*-*-*"] { > + set fpreg "r11" There is no fp as far as I can tell looking at arm aapcs maybe we can use sp here ? It's just to collect a register that has a value in the tests. > + set spreg "r13" Need to use sp here, as r13 is not defined in arm-core.xml, using r13 causes a 'r13' is a user-register; GDB cannot yet trace user-register contents. error when collecting. > + set pcreg "r15" Use pc here for the same reasons as above. > + set arg0exp "\$r0" > + set call_insn "bl" > + set pcnum 15 > + set gpr0num 0 Also in general since arm tracepoints are not there yet, this should be part of my tracepoint patch I think, so you can ommit it here and I will add it. > +} elseif [istarget "powerpc*-*-*"] { > + set fpreg "r31" > + set spreg "r1" > + set pcreg "pc" > + set arg0exp "\$r3" > + set call_insn "bl" > + set pcnum 64 > + set gpr0num 0 > +} elseif [istarget "s390*-*-*"] { > + set fpreg "r11" > + set spreg "r15" > + set pcreg "pc" > + set arg0exp "\$r2" > + set call_insn "brasl" > + # Strictly speaking, this is PSWA, not PC. > + set pcnum 1 > + set gpr0num 2 > +} elseif { [istarget "mips*-*-*"] } { > + set fpreg "s8" > + set spreg "sp" > + set pcreg "pc" > + set arg0exp "\$a0" > + # Skip the delay slot after the instruction used to make a call > + # (which can be a jump or a branch) if it has one. > + # > + # JUMP (or BRANCH) foo > + # insn1 > + # insn2 > + # > + # Most MIPS instructions used to make calls have a delay slot. > + # These include JAL, JALS, JALX, JALR, JALRS, BAL and BALS. > + # In this case the program continues from `insn2' when `foo' > + # returns. The only exception is JALRC, in which case execution > + # resumes from `insn1' instead. > + set call_insn {jalrc|[jb]al[sxr]*[ \t][^\r\n]+\r\n} > + set pcnum 37 > + set gpr0num 0 > } else { > + # Defaults. Probably won't work, but we don't want to error out > + # here on unsupported platforms, since this file is imported to check > + # for supported platforms in the first place. > set fpreg "fp" > set spreg "sp" > set pcreg "pc" > + set arg0exp "\$a0" > + set call_insn "call" > + set pcnum -1 > + set gpr0num -1 > } > > # Otherwise I've tested this on arm with the sp/fp/pc changes and the tracepoint patch , and it passes tests except for collecting some registers locals but I think this is unrelated and will investigate. Thanks, Antoine