cool - glad to hear it. i'll think a little bit about that breakpoint issue (overloading the meaning of breakpoint) and get back to you too.
On 2016-08-10 at 19:21 "'Christopher Koch' via Akaros" <[email protected]> wrote: > Thanks for all the comments! It'll be a day or so before I fix > everything up -- I'd actually started changing gdbserver from async > back to sync again like you're also suggesting. Async *was* a hard > requirement when I started, but I've since changed the protocol such > that it's not actually a requirement anymore. > > On Wed, Aug 10, 2016, 12:17 Barret Rhoden <[email protected]> > wrote: > > > 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. > > -- 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.
