This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: record-btrace


> -----Original Message-----
> From: Jan Kratochvil [mailto:jan.kratochvil@redhat.com]
> Sent: Tuesday, January 29, 2013 4:36 PM

> > "target record-full"
> > "target record-btrace"
> > "target record" aliases "target record-full"
> > "record full"
> > "record btrace"
> > " record" aliases "record full"
> 
> Maybe "record" and "target record" could already print an error about the
> command being deprecated; but that is a subtle difference, I do not mind.

I made "record" default to "target record-full", without a deprecation notice.

The problem I have with "target record" is that the sub-commands are added in
add_target, one per added target. Would it be OK to export targetlist so I can add
an alias for target "record-full"? Or is it OK to just drop target "record"?

If we dropped target record, we would break backwards compatibility. There's a
single test in gdb.mi that would need to be adjusted, but I would also expect that
GUIs would be broken.


On a similar matter, I renamed the existing "set/show record" subcommands by
prepending "full-", i.e. "insn-number-max" becomes "full-insn-number-max".

This does not break any tests but would affect scripts and MI. If we wouldn't
do that, on the other hand, it would look quite odd when we started adding
btrace-related configuration options.


> > If we now want to add LBR-based branch tracing, we could either change
> > record-btrace to record-btrace-bts and add record-btrace-lbr or make "bts"
> > and "lbr" configuration options of record-btrace.
> 
> If we had LBR I believe it should be always enabled by default - when
> available - as it has AFAIK no measurable performance disadvantage.  So the
> way of enabling it explicitly should not matter much.

LBR does not work together with BTS. We can either have one or the other.


> [detail]
> Initialization of the common fields like to_record_list could be moved to some
> common initialization function like i386_use_watchpoints().

For record-btrace and record-full, to_record_list will be quite different. For all
other targets, it will be NULL.

We could share to_disconnect, to_detach, to_kill, and to_mourn_inferior. They
are just forwarding the request after unpushing the record target.

I made the "record stop" command generic. It searches for a record target
beneath the current and unpushes the first it finds. I could do something
similar for the above.

There's a record_changed notifier that is called in to_open and in the stop
command. Why is it not called in to_close instead of in the stop command?


> > > I would not be too strict with accessing the inferior memory.  I understand
> > > that the memory content may be different when GDB is in the btrace history but
> > > most of the memory including read/write variables a user may want to read will
> > > be the same.
> > >
> > > It could rather just print a CLI (the default command-line interface) warning
> > > if one accesses a read/write memory during command execution.
> > >
> > > Forbidding any memory access may be correct but it may be pain for the users.
> >
> > I would find this very confusing. Stack and heap might not be available any
> > more - or might contain different objects.
> 
> The problem is if you are in history and you want to print a variable - should
> you "continue" back to the current state, read the variable and move back?

As long as you know what you are doing, it's convenient to be able to read
variables that you know have not changed. You need to be very careful, though,
to consider also stack changes when you reverse-step into a different function.
Things get worse if the compiler generates location lists.


> > I can do the CLI warning as first step. Will this warning also be available
> > via MI?
> 
> It will be printed as general GDB output record but at least Eclipse CDT users
> won't notice it.  It gets displayed only after selecting Debug window -> "gdb"
> and then it is in the displayed window "Console" as a "warning: ..." text.

We should maybe make this configurable so GUIs will not show wrong data.

Thanks,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]