From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8794 invoked by alias); 10 Jan 2014 20:41:09 -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 8784 invoked by uid 89); 10 Jan 2014 20:41:08 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.5 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 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, 10 Jan 2014 20:41:07 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s0AKf48q021237 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 10 Jan 2014 15:41:04 -0500 Received: from psique (ovpn-113-176.phx2.redhat.com [10.3.113.176]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s0AKf1cm010832 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Fri, 10 Jan 2014 15:41:02 -0500 From: Sergio Durigan Junior To: Pedro Alves Cc: Yao Qi , GDB Patches Subject: Re: [PATCH] Fix gdb.trace/mi-traceframe-changed.exp to check for target trace support References: <52CF4B40.3030500@codesourcery.com> <52CF57CF.9030503@codesourcery.com> <52CFD221.3050100@redhat.com> <52D04291.2000901@redhat.com> X-URL: http://www.redhat.com Date: Fri, 10 Jan 2014 20:41:00 -0000 In-Reply-To: <52D04291.2000901@redhat.com> (Pedro Alves's message of "Fri, 10 Jan 2014 18:57:21 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2014-01/txt/msg00296.txt.bz2 On Friday, January 10 2014, Pedro Alves wrote: > Hmm, OK, I think I see. I don't think the register block > size in the header needs to be correct for this test -- tfile.c > portability sounds like a red herring to me. (I don't think 500 > is correct for x86/x86_64 either?) There's no register block > in the trace file anyway. Only memory blocks. > > tfile knows to infer the PC from the tracepoint's address if > the PC wasn't collected (tfile_fetch_registers) but, > (guessing) the issue is that PC register in s390 is a > pseudo-register. tfile_fetch_registers guesses the PC from the > tracepoint's address and stores it in the regcache, but > when reading a (non-readonly) pseudo register from a regcache, > we never use the stored value in the cache -- we always > recompute it. In fact, writing the pseudo PC to the regcache > in tfile_fetch_registers with regcache_raw_supply seems reachable > when regnum==-1, and I'm surprised this isn't triggering the assertion > that makes sure the regnum being written to is a raw register > (as the regcache's buffer only holds the raw registers -- see > regcache_xmalloc_1 and regcache_raw_write). Thanks for the investigation. By debugging it a little more, I see that we don't write the pseudo PC to the regcache. You are probably talking about this loop: /* We get here if no register data has been found. Mark registers as unavailable. */ for (regn = 0; regn < gdbarch_num_regs (gdbarch); regn++) regcache_raw_supply (regcache, regn, NULL); The pseudo PC register number on s390x is 90, and "gdbarch_num_regs (gdbarch)" returns exactly 90. What happens is that when one requests the pseudo PC on s390x, it reads S390_PSWM_REGNUM, which is 1: static enum register_status s390_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache, int regnum, gdb_byte *buf) { struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); int regsize = register_size (gdbarch, regnum); ULONGEST val; if (regnum == tdep->pc_regnum) { enum register_status status; status = regcache_raw_read_unsigned (regcache, S390_PSWA_REGNUM, &val); Then, this check on tfile_fetch_registers evaluates to false: /* We can often usefully guess that the PC is going to be the same as the address of the tracepoint. */ pc_regno = gdbarch_pc_regnum (gdbarch); if (pc_regno >= 0 && (regno == -1 || regno == pc_regno)) Because regno == 1 and pc_regno == 90. Maybe you knew all that, but I thought it would be better to explicitly mention. > When debugging a live traceframe, i.e., with gdbserver, > it's gdbserver itself that does this same PC guessing. And > of course, that also only works when the PC isn't a pseudo > register, as the target backend only ever sees raw registers > accesses. > > Seems like this PC guessing should move out of > target_fetch_registers to somewhere higher level... > > I.e., this is not a bug specific to s390, but to the > tracing/unavailable machinery itself, for all targets > whose PC is a pseudo register. I see. I haven't had the chance to test on other targets where PC is a pseud register. > It's fine with me to skip the test on targets with pseudo > register PCs for now, though probably we should record this > info somewhere, probably in the code, like in the patch below. Nice, I like the comments, though your patch doesn't really change anything for s390x because of what I explained above (regno == 1 and pc_regno == 90), but I like to make things more explicit and your patch does that. I can push both our patches if you wish, btw. >> There is also another error before, but it was already failing for >> 7.6.2. > > Hard to say anything about it if you don't show it. :-) The same error: &"tfind 0\n"^M &"PC not available\n"^M ~"Found trace frame 0, tracepoint 1\n"^M ~"#-1 in ?? ()\n"^M ^done^M (gdb) ^M FAIL: gdb.trace/mi-traceframe-changed.exp: tfile: tfind 0 again So I think the same explanation is valid. > ------- > Subject: [PATCH] tfile: Don't infer the PC from the tracepoint if the PC is a > pseudo-register. > > This PC guessing can't work when the PC is a pseudo-register. > Pseudo-register values don't end up stored in the regcache, they're > always recomputed. And, it's actually wrong to try to write a > pseudo-register with regcache_raw_supply. Skip it and add a comment. > > gdb/ > 2014-01-10 Pedro Alves > > * tracepoint.c (tfile_fetch_registers): Don't infer the PC from > the tracepoint if the PC is a pseudo-register. > --- > gdb/tracepoint.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c > index 582c284..4b47282 100644 > --- a/gdb/tracepoint.c > +++ b/gdb/tracepoint.c > @@ -5060,7 +5060,17 @@ tfile_fetch_registers (struct target_ops *ops, > /* We can often usefully guess that the PC is going to be the same > as the address of the tracepoint. */ > pc_regno = gdbarch_pc_regnum (gdbarch); > - if (pc_regno >= 0 && (regno == -1 || regno == pc_regno)) > + > + /* XXX This guessing code below only works if the PC register isn't > + a pseudo-register. The value of a pseudo-register isn't stored > + in the (non-readonly) regcache -- instead it's recomputed > + (probably from some other cached raw register) whenever the > + register is read. This guesswork should probably move to some > + higher layer. */ > + if (pc_regno < 0 || pc_regno >= gdbarch_num_regs (gdbarch)) > + return; > + > + if (regno == -1 || regno == pc_regno) > { > struct tracepoint *tp = get_tracepoint (tracepoint_number); > > -- > 1.7.11.7 -- Sergio