This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] [PR gdb/13808] gdb.trace: Pass tdesc selected in gdbserver to IPA.
- From: Marcin KoÅcielnicki <koriakin at 0x04 dot net>
- To: Pedro Alves <palves at redhat dot com>, gdb-patches at sourceware dot org
- Cc: antoine dot tremblay at ericsson dot com
- Date: Thu, 25 Feb 2016 14:38:55 +0100
- Subject: Re: [PATCH] [PR gdb/13808] gdb.trace: Pass tdesc selected in gdbserver to IPA.
- Authentication-results: sourceware.org; auth=none
- References: <1456089436-1445-1-git-send-email-koriakin at 0x04 dot net> <56CEFCC5 dot 8070109 at redhat dot com>
On 25/02/16 14:08, Pedro Alves wrote:
On 02/21/2016 09:17 PM, Marcin KoÅcielnicki wrote:
If gdbserver and IPA are using different tdesc, they will disagree
about 'R' trace packet size. This results in mangled traces.
To make sure they pick the same tdesc, gdbserver pokes the tdesc
(specified as an index in a target-specific list) into a global
variable in IPA. In theory, IPA could find out the tdesc on its
own, but that may be complex (in particular, I don't know how to
tell whether we have LAST_BREAK on s390 without messing with ptrace),
and we'd have to duplicate the logic.
Tested on i386 and x86_64. On i386, it fixes two FAILs in ftrace.exp.
On x86_64, these failures have been KFAILed - one of them works now,
but the other now fails due to an unrelated reason (ugh).
Thanks.
I realized this grows each traceframe's size, probably several
times fold on x86, given the size/number of vector registers.
It'd be nice to avoid this.
- One way would be to actually respect the ax's register masks.
- Another way would be to trim recorded regblock sizes up until the
last recordable register in the tdesc. So if when reading back the
traceframe, the regblock is shorter than expected, just treat the
missing registers as unavailable.
Both of these require changing trace frame format - currently the
regblock size is shared for all tracepoints in file (ie. both trap and
fast tracepoints), but #1 sounds like the way to go.
BTW, do we print the not-actually-collected-in-fast-tracepoint registers
as <unavailable>, or as zero's?
Zeros - there's no way in the trace frame format to mark them unavailable.
It'd also be nice to be able to actually collect the now tdesc-accessible
registers, but that runs into the register size problems discussed earlier.
Plus, we probably wouldn't want to _always_ collect everything.
Well, they do work for trap tracepoints... but yeah.
I haven't analyzed the actual traceframe size impact, but I guess we can
consider those orthogonal optimization problems, as without this fix, things
just don't work.
One thing that wasn't super obvious in the patch is why do several different
target descriptions in gdbserver map to a single description in the IPA?
They're basically the same tdesc's for different ABIs (i386 vs x32 vs
x64). Since IPA is single-ABI, it's not necessary for the enum to
encode the ABI. I'll add a comment to linux-x86-tdesc.h.
I noticed several new functions with missing entry comment.
OK, will fix these.
+const struct target_desc *
+get_ipa_tdesc (int idx)
+{
+ switch (idx) {
+ case X86_TDESC_MMX: /* Should not happen. */
...
+const struct target_desc *
+get_ipa_tdesc (int idx)
+{
+ switch (idx) {
'{' goes on next line (and then reindent).
@@ -328,6 +332,14 @@ tracepoint_look_up_symbols (void)
}
}
+ /* Tell IPA about the correct tdesc. */
+ if (write_inferior_integer (ipa_sym_addrs.addr_ipa_tdesc_idx,
+ target_get_ipa_tdesc_idx ()))
+ {
+ internal_error (__FILE__, __LINE__,
+ "Error setting ipa_tdesc_idx variable in lib");
+ }
Failure to write to the inferior should never be an internal error.
The inferior might vanish, e.g., because it was SIGKILL'ed from
outside gdbserver.
Fair enough. The file is littered with internal errors in such cases
though (I just copied one of them), what should be done for these?
+
agent_look_up_symbols (NULL);
}
Thanks,
Pedro Alves