Re: [PATCH v5 00/12] GDB support for more powerpc registers on linux

Pedro Franco de Carvalho
2018-10-23T18:42:24Z

Pedro Alves <palves@redhat.com> writes:

> I.e., what is the conclusion wrt to the differences in one of the note
> sections generated by the kernel for a core file and the one generated
> by GDB? Was that resolved? Will it be?

Sorry, I really should have explained this better in the cover letter,
and I forgot to update the commit message explaining the solution we
picked for this issue.

I discussed this with Ulrich and we came to the conclusion that it would
be simpler to fill in the data for two of the registers that were
different in the core file using other registers that are available to
GDB, since they are not particularly useful as separate user-visible
registers. This still leaves out some additional registers in this
regset that GDB isn't aware of, which are most of the time 0. I will
explain this in more detail below, but in this patch this is implemented
with a different collect function used
(ppc_linux_collect_core_cpgrregset) used by
ppc_linux_iterate_over_regset_sections.

To give you some background, the checkpointed registers are the values
that are used to restore some of the regular registers to
pre-transactional values in case a hardware transaction fails. This
includes the GPRs, FPRs, vector registers and others.

The kernel uses a pt_regs struct to hold the regular GPRs and various
others registers. It also uses this struct to represent the equivalent
checkpointed registers, and to generate the note section, except that
only a subset of the registers in pt_regs is actually checkpointed by
the processor. All of the registers that are actually checkpointed are
visible to the GDB user and are correctly filled in the note section
generated by GDB.

This leaves out various other registers from pt_regs. One of these is
the NIP (the pc), which the kernel fills in with the value of another
register, TFHAR. Since TFHAR is in fact visible to GDB, GDB uses its
value to fill in the corresponding slot in the core file. The
"checkpointed" MSR (processor status register), is used by the kernel
for some internal state tracking. We use the regular MSR to fill in the
corresponding slot. These usually differ in some bits, and in certain
conditions the checkpointed MSR from the kernel can be zero.

The remaining registers in the checkpointed pt_regs are not filled in by
the kernel and don't represent anything. They will only have non-zero
values in a kernel-generated core file if someone explicitly sets them
with ptrace before the process crashes.

> As for the EBB/PMU patch, about gdbserver writing all registers in one go
> before resuming the inferior, and the error would not being detected at the
> time the user tries to write them, did you try making gdbserver flush
> the regcache after handling 'G' and 'P' packets? From the previous
> discussion, it sounded like that would work?

I haven't tried this yet. I did think about this more and although it
might work using 'P' packets, I'm not sure how to go about handling
unavailable registers in general with the 'G' packet. The problem is
that the server would need to somehow only flush these registers in
response to 'P' packets, and ignore them when they come from 'G'
packets.

This is becasue if the client always uses 'G' packets to store any
registers, including the ones that are always available, and the server
flushes the regcache to detect errors, most 'G' packets would return in
error because the inferior is usually not in a state in which the
unavailable registers can be written.

It's not clear that the 'xxxxx' encoding for unavailable registers can
be used in 'G' packets, like it is for 'g' packets. The gdbserver stub
doesn't handle this and assumes the 'G' packet contents will be properly
encoded register values, it only uses this encoding when replying to 'g'
packets.

Maybe a solution is to prevent writing to these registers on the client
side if the target is remote and print a warning (possibly through
gdbarch_cannot_store_register).

Thanks!!

--
Pedro Franco de Carvalho