Hi - Patch 4 comments.
> From 4127acb18d0ce2e19bb4f3b6bf491b162c3c13f3 Mon Sep 17 00:00:00 2001 > From: Christopher Koch <[email protected]> > Date: Thu, 4 Aug 2016 14:18:28 -0700 > Subject: parlib/debug: Added ability to store memory and single-step. > > Removed some debugging print messages. > > Change-Id: I4263112b2e340ea02e56c273529de291a22ba9dc > Signed-off-by: Christopher Koch <[email protected]> > --- > tests/Makefile | 2 +- > user/parlib/debug.c | 68 > ++++++++++++++++++++++++++++++---- > user/parlib/include/parlib/debug.h | 13 ++++--- > user/parlib/include/parlib/x86/debug.h | 2 + > user/parlib/include/parlib/x86/trap.h | 1 + > user/parlib/thread0_sched.c | 1 + > user/parlib/x86/debug.c | 24 ++++++++++++ > 7 files changed, 97 insertions(+), 14 deletions(-) > > diff --git a/tests/Makefile b/tests/Makefile > index 3127f376e4f2..aabaaa8ebff0 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -5,7 +5,7 @@ all: > # TODO: when we clean this up, if we ditch OBJDIR, change the root makefile > TESTS_DIR = tests > > -CFLAGS_TESTS += $(CFLAGS_USER) -g > +CFLAGS_TESTS += $(CFLAGS_USER) -g -ggdb -O0 Is the -O0 still necessary? I'd rather not build our tests like that. If you want to set CFLAGS_TESTS for your own use (and not for the repo), you can do so in your Makelocal, which isn't under version control. > TESTS_CXXFLAGS += $(CXXFLAGS_USER) -g -std=gnu++11 > > TESTS_LDLIBS := -lpthread -lvmm -lbenchutil -lm -liplib -lndblib > diff --git a/user/parlib/debug.c b/user/parlib/debug.c > index 3395524e7e96..5117acb593f6 100644 > --- a/user/parlib/debug.c > +++ b/user/parlib/debug.c > @@ -92,7 +92,6 @@ static void debug_read_handler(struct event_queue *ev_q); > static int debug_send_packet(int fd, void *data, size_t size); > static int debug_send_error(uint32_t errnum); > static int debug_send_response(struct d9_header *packet); > -static void *d9_read_packet(struct d9_header *hdr); You can squash this and other fixups into older commits in this patchset. > static int d9s_treadmem(struct d9_header *hdr, void *data); > static int d9s_tstoremem(struct d9_header *hdr, void *data); > static int d9s_tfetchreg(struct d9_header *hdr, void *data); > @@ -303,8 +302,12 @@ int d9s_store_memory(const struct d9_tstoremem *req) > } > > /* d9s_resume is the d9_ops func called to fulfill a TRESUME request. */ > -void d9s_resume(void) > +void d9s_resume(bool singlestep) > { > + if (singlestep) > + uthread_apply_all(uthread_enable_single_step); > + else > + uthread_apply_all(uthread_disable_single_step); Does single-step need to be applied to every thread, or just the thread that triggered the breakpoint? From our discussions, I thought we'd single-step the thread that hit the breakpoint, and then the others would just stop and start subject to whatever stop-the-world policy we came up with. As you saw below, if we need to single-step arbitrary uthreads, we're going to run into problems if they are SW or VM ctxs. Barret > uthread_apply_all(uthread_runnable); > } > > @@ -366,6 +369,7 @@ int d9s_tresume(struct d9_header *hdr, void *data) > { > int ret; > struct d9_header *rpack; > + struct d9_tresume *dtr = (struct d9_tresume *) (hdr + 1); > > if (d9_ops == NULL || d9_ops->resume == NULL) > return debug_send_error(EBADF /* TODO: better error code */); > @@ -379,7 +383,7 @@ int d9s_tresume(struct d9_header *hdr, void *data) > free(rpack); > > /* Call user-supplied routine. */ > - d9_ops->resume(); > + d9_ops->resume(dtr->singlestep); > return ret; > } > > @@ -618,7 +622,7 @@ void *d9c_read_thread(void *arg) > * Investigate what the largest memory chunk gdb will ask for > is. */ > rlen = read(fd, buf+bufindex, 4096); > if (rlen <= 0) { > - perror("d9 read"); > + /* TODO(chrisko): error message? */ > return NULL; > } > > @@ -683,6 +687,52 @@ void d9c_init(struct d9c_ops *ops) > clt_msg_handlers[D9_HANDLER(D9_TADDTHREAD)] = &d9c_taddthread; > } > > +/* d9c_store_memory communicates with the 2LS to store from an address in > + * memory. */ > +int d9c_store_memory(int fd, uintptr_t address, const void *const data, > + uint32_t length) > +{ > + size_t return_packet_size; > + struct d9_header *hdr; > + struct d9_tstoremem *req; > + > + hdr = alloc_packet(sizeof(struct d9_tstoremem), D9_TSTOREMEM); > + if (hdr == NULL) > + return -1; > + > + req = (struct d9_tstoremem *) (hdr + 1); > + req->address = address; > + req->length = length; > + memcpy(&(req->data), data, length); > + > + int ret = debug_send_packet(fd, hdr, hdr->size); > + > + /* Provide space for the message to be stored in. */ > + return_packet_size = sizeof(struct d9_header) + > + MAX(sizeof(struct d9_rstoremem), sizeof(struct d9_rerror)); > + char pkt_buf[return_packet_size]; > + > + /* Set globals */ > + response_header = (struct d9_header *) pkt_buf; > + > + /* Wait for response message. */ > + uth_mutex_lock(sync_lock); > + while (new_message != 1) > + uth_cond_var_wait(sync_cv, sync_lock); > + > + /* Got response. */ > + if (check_error_packet(response_header, D9_RSTOREMEM)) { > + perror("d9 store memory"); > + ret = -1; > + } else { > + ret = 0; > + } > + > + new_message = 0; > + uth_mutex_unlock(sync_lock); > + return ret; > +} > + > /* d9c_read_memory communicates with the 2LS to read from an address in > memory. > */ > int d9c_read_memory(int fd, uintptr_t address, uint32_t length, uint8_t *buf) > @@ -818,18 +868,20 @@ int d9c_fetch_registers(int fd, uint64_t tid, struct > d9_regs *regs) > return ret; > } > > -/* d9c_resume tells the 2LS to resume all threads. > - * > - * TODO(chrisko): resume w/ hardware-step enabled. */ > -int d9c_resume(int fd) > +/* d9c_resume tells the 2LS to resume all threads. */ > +int d9c_resume(int fd, bool singlestep) > { > size_t return_packet_size; > struct d9_header *hdr; > + struct d9_tresume *req; > > hdr = alloc_packet(sizeof(struct d9_tresume), D9_TRESUME); > if (hdr == NULL) > return -1; > > + req = (struct d9_tresume *) (hdr + 1); > + req->singlestep = singlestep; > + > int ret = debug_send_packet(fd, hdr, hdr->size); > > /* Block on getting a response packet. */ > diff --git a/user/parlib/include/parlib/debug.h > b/user/parlib/include/parlib/debug.h > index c4e6a0eb8629..a068e1d8dbfe 100644 > --- a/user/parlib/include/parlib/debug.h > +++ b/user/parlib/include/parlib/debug.h > @@ -70,7 +70,9 @@ struct d9_tstorereg { > > struct d9_rstorereg {}; > > -struct d9_tresume {}; > +struct d9_tresume { > + bool singlestep:1; > +}; > > struct d9_rresume {}; > > @@ -96,7 +98,7 @@ struct d9_ops { > int (*store_memory)(const struct d9_tstoremem *req); > int (*fetch_registers)(struct uthread *t, struct d9_regs *resp); > int (*store_registers)(struct uthread *t, struct d9_regs *resp); > - void (*resume)(void); > + void (*resume)(bool singlestep); > }; > > /* gdbserver ops. > @@ -118,7 +120,7 @@ void d9s_init(struct d9_ops *debug_ops); > /* Implementations of d9_ops. */ > int d9s_read_memory(const struct d9_treadmem *req, struct d9_rreadmem *resp); > int d9s_store_memory(const struct d9_tstoremem *req); > -void d9s_resume(void); > +void d9s_resume(bool singlestep); > > /* Helpers to send messages from 2LS to gdbserver. */ > int d9s_notify_hit_breakpoint(uint64_t tid, uint64_t address); > @@ -130,7 +132,8 @@ void *d9c_read_thread(void *arg); > > /* Helpers to send messages from gdbserver to 2LS. */ > int d9c_read_memory(int fd, uintptr_t address, uint32_t length, uint8_t > *buf); > -int d9c_store_memory(int fd, uintptr_t address, void *data, uint32_t length); > +int d9c_store_memory(int fd, uintptr_t address, const void *const data, > + uint32_t length); > int d9c_fetch_registers(int fd, uint64_t tid, struct d9_regs *regs); > int d9c_store_registers(int fd, uint64_t tid, struct d9_regs *regs); > -int d9c_resume(int fd); > +int d9c_resume(int fd, bool singlestep); > diff --git a/user/parlib/include/parlib/x86/debug.h > b/user/parlib/include/parlib/x86/debug.h > index 2c75a4f2093c..46400779344c 100644 > --- a/user/parlib/include/parlib/x86/debug.h > +++ b/user/parlib/include/parlib/x86/debug.h > @@ -29,5 +29,7 @@ struct d9_regs { > uint64_t reg_gs; > }; > > +void uthread_disable_single_step(struct uthread *t); > +void uthread_enable_single_step(struct uthread *t); > int d9_fetch_registers(struct uthread *t, struct d9_regs *resp); > int d9_store_registers(struct uthread *t, struct d9_regs *resp); > diff --git a/user/parlib/include/parlib/x86/trap.h > b/user/parlib/include/parlib/x86/trap.h > index 3c9e907032db..957ec4e214be 100644 > --- a/user/parlib/include/parlib/x86/trap.h > +++ b/user/parlib/include/parlib/x86/trap.h > @@ -12,6 +12,7 @@ > __BEGIN_DECLS > > #define HW_TRAP_DIV_ZERO 0 > +#define HW_TRAP_DEBUG 1 > #define HW_TRAP_BRKPT 3 > #define HW_TRAP_GP_FAULT 13 > #define HW_TRAP_PAGE_FAULT 14 > diff --git a/user/parlib/thread0_sched.c b/user/parlib/thread0_sched.c > index ef993392c2b9..3d4d912472bf 100644 > --- a/user/parlib/thread0_sched.c > +++ b/user/parlib/thread0_sched.c > @@ -133,6 +133,7 @@ static void thread0_thread_refl_fault(struct uthread *uth, > refl_error(uth, trap_nr, err, aux); > break; > case HW_TRAP_BRKPT: > + case HW_TRAP_DEBUG: > /* We only have one thread, no need to stop other threads. */ > uthread_has_blocked(uth, 0); > d9s_notify_hit_breakpoint(uth->id, 0); > diff --git a/user/parlib/x86/debug.c b/user/parlib/x86/debug.c > index 7dd7b589b04f..8c005fbfff34 100644 > --- a/user/parlib/x86/debug.c > +++ b/user/parlib/x86/debug.c > @@ -1,4 +1,28 @@ > #include <parlib/arch/debug.h> > +#include <parlib/assert.h> > +#include <ros/arch/mmu.h> > + > +void uthread_enable_single_step(struct uthread *t) > +{ > + switch (t->u_ctx.type) { > + case ROS_HW_CTX: > + t->u_ctx.tf.hw_tf.tf_rflags |= FL_TF; > + break; > + default: > + panic("bad context type\n"); > + } > +} > + > +void uthread_disable_single_step(struct uthread *t) > +{ > + switch (t->u_ctx.type) { > + case ROS_HW_CTX: > + t->u_ctx.tf.hw_tf.tf_rflags &= ~FL_TF; > + break; > + default: > + panic("bad context type\n"); > + } > +} > > /* TODO(chrisko): add a way to signal that a register isn't supplied for sw > * contexts; because gdbserver has a notion of not knowing a register's > value. > -- > 2.8.0.rc3.226.g39d4020 > -- You received this message because you are subscribed to the Google Groups "Akaros" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. For more options, visit https://groups.google.com/d/optout.
