diff --git a/kernel-src/include/kernel/kcov.h b/kernel-src/include/kernel/kcov.h index 377d13c3..8b1cea28 100644 --- a/kernel-src/include/kernel/kcov.h +++ b/kernel-src/include/kernel/kcov.h @@ -22,7 +22,7 @@ #define KCOV_CMP_FLAGS_64BIT 6 typedef struct { - unsigned int mode; + unsigned int mode; // transitions protected by mutex mutex_t mapping_mutex; unsigned int entry_count; // protected by mutex uint64_t *buffer; // pointer protected by mutex @@ -34,8 +34,8 @@ typedef struct { void kcov_publish_pc(uint64_t pc); void kcov_publish_cmp(uint64_t pc, uint64_t v1, uint64_t v2, uint64_t flags); -int kcov_set_buffer_size(unsigned int entry_count); -int kcov_set_mode(unsigned int mode); +int kcov_set_buffer_size(kcov_t *kcov, unsigned int entry_count); +int kcov_set_mode(kcov_t *kcov, unsigned int mode); void kcov_hold(kcov_t *kcov); void kcov_release(kcov_t *kcov); void kcov_free_buffer(kcov_t *kcov); diff --git a/kernel-src/sys/kcov.c b/kernel-src/sys/kcov.c index ebb7e62b..c214fa9d 100644 --- a/kernel-src/sys/kcov.c +++ b/kernel-src/sys/kcov.c @@ -69,8 +69,7 @@ KCOV_DISABLED static int resize_buffer(kcov_t *kcov, unsigned int entry_count) { return 0; } -KCOV_DISABLED int kcov_set_mode(unsigned int mode) { - kcov_t *kcov = current_thread()->kcov; +KCOV_DISABLED int kcov_set_mode(kcov_t *kcov, unsigned int mode) { __assert(kcov); if (mode != KCOV_MODE_DISABLED && @@ -78,26 +77,44 @@ KCOV_DISABLED int kcov_set_mode(unsigned int mode) { mode != KCOV_MODE_CMP) return EINVAL; - if (mode == KCOV_MODE_DISABLED && kcov->mode == KCOV_MODE_DISABLED) - return EINVAL; + MUTEX_ACQUIRE(&kcov->mapping_mutex); + int error = 0; + if (mode == KCOV_MODE_DISABLED) { + if (current_thread()->kcov != kcov) { + error = EINVAL; + goto cleanup; + } - if (mode != KCOV_MODE_DISABLED && kcov->mode != KCOV_MODE_DISABLED) - return EINVAL; + current_thread()->kcov = NULL; + } else { + if (current_thread()->kcov || kcov->mode != KCOV_MODE_DISABLED) { + error = EBUSY; + goto cleanup; + } - if (kcov->buffer == NULL) - return EINVAL; + if (kcov->buffer == NULL) { + error = EINVAL; + goto cleanup; + } + + kcov_hold(kcov); + } kcov->mode = mode; - return 0; + if (mode != KCOV_MODE_DISABLED) + current_thread()->kcov = kcov; + + cleanup: + MUTEX_RELEASE(&kcov->mapping_mutex); + if (error == 0 && mode == KCOV_MODE_DISABLED) + kcov_release(kcov); + return error; } -KCOV_DISABLED int kcov_set_buffer_size(unsigned int entry_count) { +KCOV_DISABLED int kcov_set_buffer_size(kcov_t *kcov, unsigned int entry_count) { if (entry_count < 0 || entry_count > MAX_ENTRIES) return EINVAL; - kcov_t *kcov = current_thread()->kcov; - __assert(kcov); - MUTEX_ACQUIRE(&kcov->mapping_mutex); int error = 0; if (kcov->mappings || kcov->mode != KCOV_MODE_DISABLED) { diff --git a/kernel-src/sys/kcov_dev.c b/kernel-src/sys/kcov_dev.c index bf9e4005..df1e3423 100644 --- a/kernel-src/sys/kcov_dev.c +++ b/kernel-src/sys/kcov_dev.c @@ -89,10 +89,6 @@ static int create_node_for_kcov(kcov_t *kcov, int minor, vnode_t **vnodep) { kcov->vnode_handle = vnode; devfs_remove_anonymous(DEV_MAJOR_KCOV, minor); - // a kcov will have two references initially: - // - one for the thread holding it - // - one for the vnode - kcov_hold(kcov); *vnodep = vnode; return 0; } @@ -103,18 +99,6 @@ static int open(int oldminor, vnode_t **vnodep, int flags) { if (oldminor != KCOV_MAX) return ENODEV; - // if the thread already has a kcov attached, return it - if (current_thread()->kcov) { - kcov_t *kcov = current_thread()->kcov; - - int error = create_node_for_kcov(kcov, kcov->minor, vnodep); - if (error) - return error; - - VOP_RELEASE(old); - return 0; - } - int minor; int error = allocate_kcov(&minor); if (error) @@ -127,7 +111,6 @@ static int open(int oldminor, vnode_t **vnodep, int flags) { return error; } - current_thread()->kcov = kcov; VOP_RELEASE(old); return 0; } @@ -137,8 +120,8 @@ static int open(int oldminor, vnode_t **vnodep, int flags) { #define IOCTL_REQUEST_SET_SIZE 0xF1177200 static int ioctl(int minor, unsigned long request, void *arg, int *result, cred_t *cred) { - kcov_t *kcov = current_thread()->kcov; - if (kcov == NULL || kcov->minor != minor) + kcov_t *kcov = get_kcov(minor); + if (kcov == NULL) return EINVAL; switch (request) { @@ -148,24 +131,21 @@ static int ioctl(int minor, unsigned long request, void *arg, int *result, cred_ if (error) return error; - return kcov_set_buffer_size(size); + return kcov_set_buffer_size(kcov, size); } case IOCTL_REQUEST_START: { - if (kcov->mode != KCOV_MODE_DISABLED) - return EBUSY; - - if (kcov->buffer == NULL) - return EINVAL; - int mode; int error = USERCOPY_POSSIBLY_FROM_USER(&mode, arg, sizeof(mode)); if (error) return error; - return kcov_set_mode(mode); + if (mode != KCOV_MODE_PC && mode != KCOV_MODE_CMP) + return EINVAL; + + return kcov_set_mode(kcov, mode); } case IOCTL_REQUEST_STOP: - return kcov_set_mode(KCOV_MODE_DISABLED); + return kcov_set_mode(kcov, KCOV_MODE_DISABLED); default: return ENOTTY; } diff --git a/kernel-src/sys/thread.c b/kernel-src/sys/thread.c index e044a046..587c40ee 100644 --- a/kernel-src/sys/thread.c +++ b/kernel-src/sys/thread.c @@ -57,10 +57,6 @@ thread_t *sched_newthread(void *ip, size_t kstacksize, int nice, proc_t *proc, v } void sched_destroythread(thread_t *thread) { -#ifdef KCOV_ENABLED - if (thread->kcov) - kcov_release(thread->kcov); -#endif mm_unmap(thread->kernelstack, thread->kernelstacksize, 0); arch_extracontext_free(&thread->extracontext); slab_free(thread_cache, thread); @@ -88,6 +84,11 @@ __attribute__((noreturn)) void sched_threadexit() { thread_t *thread = current_thread(); proc_t *proc = thread->proc; +#ifdef KCOV_ENABLED + if (thread->kcov) + kcov_set_mode(thread->kcov, KCOV_MODE_DISABLED); +#endif + mm_context_t *oldctx = thread->mmctx; mm_switch_context(&mm_kernel_ctx); @@ -105,11 +106,6 @@ __attribute__((noreturn)) void sched_threadexit() { PROC_RELEASE(proc); } -#ifdef KCOV_ENABLED - if (current_thread()->kcov) - kcov_set_mode(KCOV_MODE_DISABLED); -#endif - interrupt_set(false); arch_context_saveandcall(threadexit_internal, current_cpu()->schedulerstack, NULL); __builtin_unreachable();