unwind: implement automatic mmap cache invalidation
authorMasatake YAMATO <yamato@redhat.com>
Wed, 16 Apr 2014 06:33:07 +0000 (15:33 +0900)
committerDmitry V. Levin <ldv@altlinux.org>
Fri, 30 May 2014 22:56:14 +0000 (22:56 +0000)
A mmap cache belonging to a tcb was updated when a system call which
changed the memory mapping was called.  This implementation was assumed
the mapping was changed only by the tcb.  However, this assumption is
incorrect if the target application is multi-threaded; more than two
tcbs can shared the same memory mapping and a tcb can modify it without
being noticed by the others.

This change introduces a global integer variable mmap_cache_generation,
and mmap_cache_generation field to struct tcb.  The variable
is incremented each time a process enters a syscall that can modify its
memory mapping.  Each tcb records the value of this variable at the
moment if  building its mmap cache.  Every mmap cache associated with
the given tcb can be validated by comparing its mmap_cache_generation
field with the variable mmap_cache_generation.

This implementation is inefficient.  If strace attaches two processes
which don't share the memory mapping, rebuilding mmap cache of a tcb
triggered by another tcb's mmap system call is not necessary.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
defs.h
unwind.c

diff --git a/defs.h b/defs.h
index 70b42a88fccd5c56320f120c02156e8d6966ddf3..464a968aad3e1f85012b44447fca714662fd9d81 100644 (file)
--- a/defs.h
+++ b/defs.h
@@ -430,6 +430,7 @@ struct tcb {
        struct UPT_info* libunwind_ui;
        struct mmap_cache_t* mmap_cache;
        unsigned int mmap_cache_size;
+       unsigned int mmap_cache_generation;
        struct queue_t* queue;
 #endif
 };
index 5794801aa0a518f8760364a35cfd7eabf1258b23..ef5a404ceb0eb3cd6df0e460e089d4578fd7a36e 100644 (file)
--- a/unwind.c
+++ b/unwind.c
@@ -74,9 +74,12 @@ struct queue_t {
        struct call_t *tail;
        struct call_t *head;
 };
+
 static void queue_print(struct queue_t *queue);
+static void delete_mmap_cache(struct tcb *tcp, const char *caller);
 
 static unw_addr_space_t libunwind_as;
+static unsigned int mmap_cache_generation;
 
 void
 unwind_init(void)
@@ -107,7 +110,7 @@ unwind_tcb_fin(struct tcb *tcp)
        free(tcp->queue);
        tcp->queue = NULL;
 
-       unwind_cache_invalidate(tcp);
+       delete_mmap_cache(tcp, __FUNCTION__);
 
        _UPT_destroy(tcp->libunwind_ui);
        tcp->libunwind_ui = NULL;
@@ -119,7 +122,7 @@ unwind_tcb_fin(struct tcb *tcp)
  * The cache must be refreshed after some syscall: mmap, mprotect, munmap, execve
  */
 static void
-alloc_mmap_cache(struct tcb* tcp)
+build_mmap_cache(struct tcb* tcp)
 {
        unsigned long start_addr, end_addr, mmap_offset;
        char filename[sizeof ("/proc/0123456789/maps")];
@@ -192,13 +195,27 @@ alloc_mmap_cache(struct tcb* tcp)
        }
        fclose(fp);
        tcp->mmap_cache = cache_head;
+       tcp->mmap_cache_generation = mmap_cache_generation;
+
+       DPRINTF("tgen=%u, ggen=%u, tcp=%p, cache=%p",
+               "cache-build",
+               tcp->mmap_cache_generation,
+               mmap_cache_generation,
+               tcp, tcp->mmap_cache);
 }
 
 /* deleting the cache */
-void
-unwind_cache_invalidate(struct tcb* tcp)
+static void
+delete_mmap_cache(struct tcb *tcp, const char *caller)
 {
        unsigned int i;
+
+       DPRINTF("tgen=%u, ggen=%u, tcp=%p, cache=%p, caller=%s",
+               "cache-delete",
+               tcp->mmap_cache_generation,
+               mmap_cache_generation,
+               tcp, tcp->mmap_cache, caller);
+
        for (i = 0; i < tcp->mmap_cache_size; i++) {
                free(tcp->mmap_cache[i].binary_filename);
                tcp->mmap_cache[i].binary_filename = NULL;
@@ -208,6 +225,33 @@ unwind_cache_invalidate(struct tcb* tcp)
        tcp->mmap_cache_size = 0;
 }
 
+static bool
+rebuild_cache_if_invalid(struct tcb *tcp, const char *caller)
+{
+       if ((tcp->mmap_cache_generation != mmap_cache_generation)
+           && tcp->mmap_cache)
+               delete_mmap_cache(tcp, caller);
+
+       if (!tcp->mmap_cache)
+               build_mmap_cache(tcp);
+
+       if (!tcp->mmap_cache || !tcp->mmap_cache_size)
+               return false;
+       else
+               return true;
+}
+
+void
+unwind_cache_invalidate(struct tcb* tcp)
+{
+       mmap_cache_generation++;
+       DPRINTF("tgen=%u, ggen=%u, tcp=%p, cache=%p", "increment",
+               tcp->mmap_cache_generation,
+               mmap_cache_generation,
+               tcp,
+               tcp->mmap_cache);
+}
+
 /*
  * walking the stack
  */
@@ -229,9 +273,9 @@ stacktrace_walk(struct tcb *tcp,
        unsigned long true_offset;
 
        if (!tcp->mmap_cache)
-               alloc_mmap_cache(tcp);
-       if (!tcp->mmap_cache || !tcp->mmap_cache_size)
-               return;
+               error_msg_and_die("bug: mmap_cache is NULL");
+       if (tcp->mmap_cache_size == 0)
+               error_msg_and_die("bug: mmap_cache is empty");
 
        symbol_name = malloc(symbol_name_size);
        if (!symbol_name)
@@ -494,7 +538,7 @@ unwind_print_stacktrace(struct tcb* tcp)
               DPRINTF("tcp=%p, queue=%p", "queueprint", tcp, tcp->queue->head);
               queue_print(tcp->queue);
        }
-       else {
+       else if (rebuild_cache_if_invalid(tcp, __FUNCTION__)) {
                DPRINTF("tcp=%p, queue=%p", "stackprint", tcp, tcp->queue->head);
                stacktrace_walk(tcp, print_call_cb, print_error_cb, NULL);
        }
@@ -509,7 +553,9 @@ unwind_capture_stacktrace(struct tcb *tcp)
        if (tcp->queue->head)
                error_msg_and_die("bug: unprinted entries in queue");
 
-       stacktrace_walk(tcp, queue_put_call, queue_put_error,
-                       tcp->queue);
-       DPRINTF("tcp=%p, queue=%p", "captured", tcp, tcp->queue->head);
+       if (rebuild_cache_if_invalid(tcp, __FUNCTION__)) {
+               stacktrace_walk(tcp, queue_put_call, queue_put_error,
+                               tcp->queue);
+               DPRINTF("tcp=%p, queue=%p", "captured", tcp, tcp->queue->head);
+       }
 }