Re: [PATCH] Fix gdb.trace/mi-traceframe-changed.exp to check for target trace support

[Pedro Alves <palves@redhat.com>]

On 01/10/2014 05:30 PM, Sergio Durigan Junior wrote:
> On Friday, January 10 2014, Pedro Alves wrote:
> 
>>>> The root cause is that tfile.c isn't portable and unable to produce
>>>> trace file properly for s390x.  Search FIXME in it.
>>>
>>> Indeed, thanks for pointing that.
>>
>> Is it the register block size?  What's the actual error that
>> causes / you're seeing?
> 
> The error is:
> 
> -trace-find frame-number 0^M
> &"PC not available\n"^M
> ^done,found="1",tracepoint="1",traceframe="0",frame={level="-1",addr="<unavailable>",func="??",args=[]}^M
> (gdb) ^M
> FAIL: gdb.trace/mi-traceframe-changed.exp: tfile: -trace-find frame-number 0

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).

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.

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.

> 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.  :-)

-------
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  <palves@redhat.com>

	* 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