[PATCH v4 09/37] unwind: consider multi-threading when rebuilding mmap cache

[Masatake YAMATO]

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.

In this patch an integer variable, `mmap_cache_generation' and
`mmap_cache_generation' field of tcb are introduced.  The variable
`mmap_cache_generation' is incremented each time when any tcb calls a
system call which modifies its memory mapping. Each tcb sets the value
to its `mmap_cache_generation' field when building its mmap cache. A
tcb can know its mmap cache is fresh enough or not by comparing its
`mmap_cache_generation' field with the variable
`mmap_cache_generation'.

restriction and todo:

1. As strace targets the user must specify not only an interesting
   thread but also threads belonging to the same process with using
   -f and/or -p option. In the future strace should detect the
   threads and attach to them stealthy.

2. This implementation is insufficient. If strace attach two processes,
   which don't share the memory mapping, rebuilding  mmap cache of a
   tcb triggered by another tcb's mmap system call is waste of cpu
   time. In the future strace should manage `mmap_cache_generation'
   in each process.

Signed-off-by: Masatake YAMATO <yamato at redhat.com>
---
 defs.h   |  1 +
 unwind.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 58 insertions(+), 11 deletions(-)

diff --git a/defs.h b/defs.h
index b5f30c5..cfd767d 100644
--- 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
 };
diff --git a/unwind.c b/unwind.c
index f9af146..42c885e 100644
--- 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);
+	}
 }
-- 
1.9.0