From 840bb0acb548125ff574e40754fcb88620fec3f9 Mon Sep 17 00:00:00 2001 From: Masatake YAMATO Date: Mon, 30 Apr 2018 06:45:40 +0900 Subject: [PATCH] unwind-libdw: use the mmap_notify subsystem The unwind subsystem uses the mmap_cache subsystem even it uses unwind-libdw as backend. unwind-libdw doesn't need the full set of the mmap_cache subsystem; libdw has a feature for caching a memory mapping. This commit does three things. (1) Make the unwind subsystem not use the mmap_cache subsystem. The unwind subsystem never concern the memory mapping of the target. It becomes a thin layer. (2) Make unwind-libunwind use the mmap_cache subsystem directly. (3) Make unwind-libdw use the mmap_notify subsystem to know when it should call dwfl_linux_proc_report/dwfl_report_end for updating the cache. Here is a subsystem structure that this patch introduces: +-------------------------------------+ | unwind subsys | +------------------+------------------+ | unwind-libunwind | unwind-libdw | +------------------+------------------+ | mmap_cache | | +------------------+ | | mmap_notify | +-------------------------------------+ | syscall | +-------------------------------------+ mmap/munmap/mprotect/brk... * unwind.c: Don't include "mmap_cache.h". (unwind_init): Don't call mmap_cache_enable. (unwind_tcb_print, unwind_tcb_capture): Don't call mmap_cache related functions, just invoke unwinder.tcb_walk. * unwind.h (struct unwind_unwinder_t): Remove tcb_flush_cache field. * unwind-libdw.c: Include "mmap_notify.h" instead of "mmap_cache.h". (struct ctx): Add last_proc_updating field to record the generation of memory mapping that is cached by dwfl_linux_proc_report and dwfl_report_end. (mapping_generation): A variable counting how many times the memory mapping of targets has been changed. (updating_mapping_generation): New utility function for updating mapping_generation. (init): New function for registering updating_mapping_generation in the mmap_notify subsystem as a callback function. (tcb_init): Initialize ctx::last_proc_updating. (tcb_flush_cache): Rename to flush_cache_maybe. Rebuild the cache data only if the data is stale. (tcb_walk): Call flush_cache_maybe for avoiding referring staled cache data. (unwinder): Set init function, remove tcb_flush_cache field. * unwind-libunwind.c (init): Enable the mmap_cache subsystem. (tcb_walk): Call mmap_cache_rebuild_if_invalid and unw_flush_cache for updating the cache of the memory mapping before walking the stack. (tcb_walk): Rename to walk. (unwinder): Remove tcb_flush_cache field. Signed-off-by: Masatake YAMATO --- unwind-libdw.c | 69 ++++++++++++++++++++++++++++++---------------- unwind-libunwind.c | 33 +++++++++++++++++----- unwind.c | 34 ++++------------------- unwind.h | 6 ---- 4 files changed, 78 insertions(+), 64 deletions(-) diff --git a/unwind-libdw.c b/unwind-libdw.c index b20ba204..60391465 100644 --- a/unwind-libdw.c +++ b/unwind-libdw.c @@ -36,13 +36,28 @@ #include "defs.h" #include "unwind.h" -#include "mmap_cache.h" +#include "mmap_notify.h" #include struct ctx { Dwfl *dwfl; + unsigned int last_proc_updating; }; +static unsigned int mapping_generation; + +static void +update_mapping_generation(struct tcb *tcp, void *unused) +{ + mapping_generation++; +} + +static void +init(void) +{ + mmap_notify_register_client(update_mapping_generation, NULL); +} + static void * tcb_init(struct tcb *tcp) { @@ -74,6 +89,7 @@ tcb_init(struct tcb *tcp) struct ctx *ctx = xmalloc(sizeof(*ctx)); ctx->dwfl = dwfl; + ctx->last_proc_updating = 0; return ctx; } @@ -87,6 +103,31 @@ tcb_fin(struct tcb *tcp) } } +static void +flush_cache_maybe(struct tcb *tcp) +{ + struct ctx *ctx = tcp->unwind_ctx; + if (!ctx) + return; + + if (ctx->last_proc_updating == mapping_generation) + return; + + int r = dwfl_linux_proc_report(ctx->dwfl, tcp->pid); + + if (r < 0) + error_msg("dwfl_linux_proc_report returned an error" + " for pid %d: %s", tcp->pid, dwfl_errmsg(-1)); + else if (r > 0) + error_msg("dwfl_linux_proc_report returned an error" + " for pid %d", tcp->pid); + else if (dwfl_report_end(ctx->dwfl, NULL, NULL) != 0) + error_msg("dwfl_report_end returned an error" + " for pid %d: %s", tcp->pid, dwfl_errmsg(-1)); + + ctx->last_proc_updating = mapping_generation; +} + struct frame_user_data { unwind_call_action_fn call_action; unwind_error_action_fn error_action; @@ -150,6 +191,9 @@ tcb_walk(struct tcb *tcp, .data = data, .stack_depth = 256, }; + + flush_cache_maybe(tcp); + int r = dwfl_getthread_frames(ctx->dwfl, tcp->pid, frame_callback, &user_data); if (r) @@ -158,31 +202,10 @@ tcb_walk(struct tcb *tcp, 0); } -static void -tcb_flush_cache(struct tcb *tcp) -{ - struct ctx *ctx = tcp->unwind_ctx; - if (!ctx) - return; - - int r = dwfl_linux_proc_report(ctx->dwfl, tcp->pid); - - if (r < 0) - error_msg("dwfl_linux_proc_report returned an error" - " for pid %d: %s", tcp->pid, dwfl_errmsg(-1)); - else if (r > 0) - error_msg("dwfl_linux_proc_report returned an error" - " for pid %d", tcp->pid); - else if (dwfl_report_end(ctx->dwfl, NULL, NULL) != 0) - error_msg("dwfl_report_end returned an error" - " for pid %d: %s", tcp->pid, dwfl_errmsg(-1)); -} - const struct unwind_unwinder_t unwinder = { .name = "libdw", - .init = NULL, + .init = init, .tcb_init = tcb_init, .tcb_fin = tcb_fin, .tcb_walk = tcb_walk, - .tcb_flush_cache = tcb_flush_cache, }; diff --git a/unwind-libunwind.c b/unwind-libunwind.c index 38b3e2c5..c64e92cb 100644 --- a/unwind-libunwind.c +++ b/unwind-libunwind.c @@ -36,6 +36,8 @@ static unw_addr_space_t libunwind_as; static void init(void) { + mmap_cache_enable(); + libunwind_as = unw_create_addr_space(&_UPT_accessors, 0); if (!libunwind_as) error_msg_and_die("failed to create address space" @@ -125,10 +127,10 @@ print_stack_frame(struct tcb *tcp, } static void -tcb_walk(struct tcb *tcp, - unwind_call_action_fn call_action, - unwind_error_action_fn error_action, - void *data) +walk(struct tcb *tcp, + unwind_call_action_fn call_action, + unwind_error_action_fn error_action, + void *data) { char *symbol_name; size_t symbol_name_size = 40; @@ -159,9 +161,27 @@ tcb_walk(struct tcb *tcp, } static void -tcb_flush_cache(struct tcb *tcp) +tcb_walk(struct tcb *tcp, + unwind_call_action_fn call_action, + unwind_error_action_fn error_action, + void *data) { - unw_flush_cache(libunwind_as, 0, 0); + switch (mmap_cache_rebuild_if_invalid(tcp, __func__)) { + case MMAP_CACHE_REBUILD_RENEWED: + /* + * Rebuild the unwinder internal cache. + * Called when mmap cache subsystem detects a + * change of tracee memory mapping. + */ + unw_flush_cache(libunwind_as, 0, 0); + ATTRIBUTE_FALLTHROUGH; + case MMAP_CACHE_REBUILD_READY: + walk(tcp, call_action, error_action, data); + break; + default: + /* Do nothing */ + ; + } } const struct unwind_unwinder_t unwinder = { @@ -170,5 +190,4 @@ const struct unwind_unwinder_t unwinder = { .tcb_init = tcb_init, .tcb_fin = tcb_fin, .tcb_walk = tcb_walk, - .tcb_flush_cache = tcb_flush_cache, }; diff --git a/unwind.c b/unwind.c index 586f7450..e4c6623c 100644 --- a/unwind.c +++ b/unwind.c @@ -26,7 +26,6 @@ */ #include "defs.h" -#include "mmap_cache.h" #include "unwind.h" #ifdef USE_DEMANGLE @@ -59,7 +58,6 @@ unwind_init(void) { if (unwinder.init) unwinder.init(); - mmap_cache_enable(); } void @@ -296,19 +294,8 @@ unwind_tcb_print(struct tcb *tcp) debug_func_msg("head: tcp=%p, queue=%p", tcp, tcp->unwind_queue->head); queue_print(tcp->unwind_queue); - } else switch (mmap_cache_rebuild_if_invalid(tcp, __func__)) { - case MMAP_CACHE_REBUILD_RENEWED: - unwinder.tcb_flush_cache(tcp); - ATTRIBUTE_FALLTHROUGH; - case MMAP_CACHE_REBUILD_READY: - debug_func_msg("walk: tcp=%p, queue=%p", - tcp, tcp->unwind_queue->head); - unwinder.tcb_walk(tcp, print_call_cb, print_error_cb, NULL); - break; - default: - /* Do nothing */ - ; - } + } else + unwinder.tcb_walk(tcp, print_call_cb, print_error_cb, NULL); } /* @@ -325,19 +312,10 @@ unwind_tcb_capture(struct tcb *tcp) #endif if (tcp->unwind_queue->head) error_msg_and_die("bug: unprinted entries in queue"); - - switch (mmap_cache_rebuild_if_invalid(tcp, __func__)) { - case MMAP_CACHE_REBUILD_RENEWED: - unwinder.tcb_flush_cache(tcp); - ATTRIBUTE_FALLTHROUGH; - case MMAP_CACHE_REBUILD_READY: - unwinder.tcb_walk(tcp, queue_put_call, queue_put_error, - tcp->unwind_queue); - debug_func_msg("tcp=%p, queue=%p", + else { + debug_func_msg("walk: tcp=%p, queue=%p", tcp, tcp->unwind_queue->head); - break; - default: - /* Do nothing */ - ; + unwinder.tcb_walk(tcp, queue_put_call, queue_put_error, + tcp->unwind_queue); } } diff --git a/unwind.h b/unwind.h index 3a6065b4..678c561f 100644 --- a/unwind.h +++ b/unwind.h @@ -63,12 +63,6 @@ struct unwind_unwinder_t { unwind_call_action_fn, unwind_error_action_fn, void *); - - /* - * Rebuild the unwinder internal cache. Called when mmap cache - * subsystem detects a change of tracee memory mapping. - */ - void (*tcb_flush_cache)(struct tcb *); }; extern const struct unwind_unwinder_t unwinder; -- 2.40.0