|
From: Nicholas N. <nj...@ca...> - 2004-09-03 08:40:58
|
On Fri, 3 Sep 2004, Chris January wrote:
>> Now that 2.2.0 is out is there any chance of getting the
>> first of my debugging support patches into CVS?
>
> Should I treat the silence as an implicit 'no'?
No. More likely that no-one has any strong opinions, or are busy with
other things at the moment.
I looked at your patch, but haven't run it. I'm confused by a few
things.
- The single_step field you added to ThreadState -- it doesn't seem ever
be set.
- You've changed a few test conditions by one, and changed the 'jz' in the
dispatch loop to 'jle', but have not explained why this is necessary.
Some comments in the code would be helpful.
- You added 'stopped' to ThreadState, but I also can't see how it is set.
- 'stopped' is used like this:
+ if (VG_(threads)[tid_next].status == VgTs_Runnable &&
+ !VG_(threads)[tid_next].stopped)
should 'stopped' rather be folded into the 'status' type somehow? Eg.
have a VgTs_RunnableButStopped state, or similar?
- You said:
> These should be the only changes to existing parts of
> the core that are required to support live debugging. Obviously this
> patch doesn't add full debugging support in and of itself.
What more is required to get full debugging support? How much code will
it be?
- This code is unnecessarily repetitive:
+ if (trc == VG_TRC_EBP_JMP_BREAKPOINT) {
+ /* A software breakpoint. */
+ vg_tid_currently_in_baseBlock = vg_tid_last_in_baseBlock;
+ VG_(kkill) (0, VKI_SIGTRAP);
+ vg_tid_currently_in_baseBlock = 0;
+ break;
+ }
+
+ if (trc == VG_TRC_INNER_COUNTERZERO &&
+ VG_(threads)[tid].single_step) {
+ vg_tid_currently_in_baseBlock = vg_tid_last_in_baseBlock;
+ VG_(kkill) (0, VKI_SIGTRAP);
+ vg_tid_currently_in_baseBlock = 0;
+ break;
+ }
Finally, I still don't understand what the end result is intended to be
with these proposed changes -- do you start GDB, then load your program
under a Valgrind tool, then run it? Or something else?
N
|