mmap_cache: do not activate unless requested

Do not call mmap_cache functions until mmap_cache_enable is invoked.
Change struct mmap_cache_t into a proxy structure, move all mmap_cache
data from struct tcb inside this new structure.

* Makefile.am (strace_SOURCES): Move mmap_cache.c and mmap_cache.h
to libstrace_a_SOURCES.
* defs.h (struct tcb): Remove mmap_cache_size and mmap_cache_generation
* fields.
* mmap_cache.h (struct mmap_cache_t): Rename
to struct mmap_cache_entry_t, create a new struct mmap_cache_t,
all users updated.
(mmap_cache_delete): Remove.
* mmap_cache.c (mmap_cache_delete): Rename to delete_mmap_cache,
add static qualifier.
(build_mmap_cache): Merge into mmap_cache_rebuild_if_invalid.
* strace.c (droptcb): Replace mmap_cache_delete invocation
with tcp->mmap_cache->free_fn.

Author: ldv-alt

Makefile.am

@@ -75,6 +75,8 @@ libstrace_a_SOURCES =	\
 	fstatfs64.c \
 	getpagesize.c \
 	ipc.c \
+	mmap_cache.c	\
+	mmap_cache.h	\
 	sigreturn.c	\
 	socketcall.c \
 	statfs.c \
@@ -190,8 +192,6 @@ strace_SOURCES =	\
 	membarrier.c	\
 	memfd_create.c	\
 	mknod.c		\
-	mmap_cache.c	\
-	mmap_cache.h	\
 	mmap_notify.c	\
 	mmap_notify.h	\
 	mmsghdr.c	\

defs.h

@@ -219,8 +219,6 @@ struct tcb {
 	struct timespec delay_expiration_time; /* When does the delay end */
 
 	struct mmap_cache_t *mmap_cache;
-	unsigned int mmap_cache_size;
-	unsigned int mmap_cache_generation;
 
 #ifdef ENABLE_STACKTRACE
 	void *unwind_ctx;

mmap_cache.c

@@ -46,9 +46,9 @@ mmap_cache_invalidate(struct tcb *tcp, void *unused)
 #endif
 	mmap_cache_generation++;
 	debug_func_msg("tgen=%u, ggen=%u, tcp=%p, cache=%p",
-		       tcp->mmap_cache_generation,
-		       mmap_cache_generation,
-		       tcp, tcp->mmap_cache);
+		       tcp->mmap_cache ? tcp->mmap_cache->generation : 0,
+		       mmap_cache_generation, tcp,
+		       tcp->mmap_cache ? tcp->mmap_cache->entry : 0);
 }
 
 void
@@ -62,33 +62,66 @@ mmap_cache_enable(void)
 	}
 }
 
+/* deleting the cache */
+static void
+delete_mmap_cache(struct tcb *tcp, const char *caller)
+{
+	debug_func_msg("tgen=%u, ggen=%u, tcp=%p, cache=%p, caller=%s",
+		       tcp->mmap_cache ? tcp->mmap_cache->generation : 0,
+		       mmap_cache_generation, tcp,
+		       tcp->mmap_cache ? tcp->mmap_cache->entry : 0, caller);
+
+	if (!tcp->mmap_cache)
+		return;
+
+	while (tcp->mmap_cache->size) {
+		unsigned int i = --tcp->mmap_cache->size;
+		free(tcp->mmap_cache->entry[i].binary_filename);
+		tcp->mmap_cache->entry[i].binary_filename = NULL;
+	}
+
+	free(tcp->mmap_cache->entry);
+	tcp->mmap_cache->entry = NULL;
+
+	free(tcp->mmap_cache);
+	tcp->mmap_cache = NULL;
+}
+
 /*
  * caching of /proc/ID/maps for each process to speed up stack tracing
  *
  * The cache must be refreshed after syscalls that affect memory mappings,
  * e.g. mmap, mprotect, munmap, execve.
  */
-static void
-build_mmap_cache(struct tcb *tcp)
+extern enum mmap_cache_rebuild_result
+mmap_cache_rebuild_if_invalid(struct tcb *tcp, const char *caller)
 {
-	FILE *fp;
-	struct mmap_cache_t *cache_head = tcp->mmap_cache;
-	/* start with a small dynamically-allocated array and then expand it */
-	size_t cur_array_size = 0;
-	char filename[sizeof("/proc/4294967296/maps")];
-	char buffer[PATH_MAX + 80];
+	if (tcp->mmap_cache
+	    && tcp->mmap_cache->generation != mmap_cache_generation)
+		delete_mmap_cache(tcp, caller);
 
+	if (tcp->mmap_cache)
+		return MMAP_CACHE_REBUILD_READY;
+
+	char filename[sizeof("/proc/4294967296/maps")];
 	xsprintf(filename, "/proc/%u/maps", tcp->pid);
-	fp = fopen_stream(filename, "r");
+
+	FILE *fp = fopen_stream(filename, "r");
 	if (!fp) {
 		perror_msg("fopen: %s", filename);
-		return;
+		return MMAP_CACHE_REBUILD_NOCACHE;
 	}
 
-	tcp->mmap_cache_size = 0;
+	struct mmap_cache_t cache = {
+		.free_fn = delete_mmap_cache,
+		.generation = mmap_cache_generation
+	};
+
+	/* start with a small dynamically-allocated array and then expand it */
+	size_t allocated = 0;
+	char buffer[PATH_MAX + 80];
 
 	while (fgets(buffer, sizeof(buffer), fp) != NULL) {
-		struct mmap_cache_t *entry;
 		unsigned long start_addr, end_addr, mmap_offset;
 		char read_bit;
 		char write_bit;
@@ -120,12 +153,13 @@ build_mmap_cache(struct tcb *tcp)
 			break;
 		}
 
+		struct mmap_cache_entry_t *entry;
 		/*
 		 * sanity check to make sure that we're storing
 		 * non-overlapping regions in ascending order
 		 */
-		if (tcp->mmap_cache_size > 0) {
-			entry = &cache_head[tcp->mmap_cache_size - 1];
+		if (cache.size > 0) {
+			entry = &cache.entry[cache.size - 1];
 			if (entry->start_addr == start_addr &&
 			    entry->end_addr == end_addr) {
 				/* duplicate entry, e.g. [vsyscall] */
@@ -143,11 +177,11 @@ build_mmap_cache(struct tcb *tcp)
 			}
 		}
 
-		if (tcp->mmap_cache_size >= cur_array_size)
-			cache_head = xgrowarray(cache_head, &cur_array_size,
-						sizeof(*cache_head));
+		if (cache.size >= allocated)
+			cache.entry = xgrowarray(cache.entry, &allocated,
+						 sizeof(*cache.entry));
 
-		entry = &cache_head[tcp->mmap_cache_size];
+		entry = &cache.entry[cache.size];
 		entry->start_addr = start_addr;
 		entry->end_addr = end_addr;
 		entry->mmap_offset = mmap_offset;
@@ -161,73 +195,40 @@ build_mmap_cache(struct tcb *tcp)
 		entry->major = major;
 		entry->minor = minor;
 		entry->binary_filename = xstrdup(binary_path);
-		tcp->mmap_cache_size++;
+		cache.size++;
 	}
 	fclose(fp);
-	tcp->mmap_cache = cache_head;
-	tcp->mmap_cache_generation = mmap_cache_generation;
 
-	debug_func_msg("tgen=%u, ggen=%u, tcp=%p, cache=%p",
-		       tcp->mmap_cache_generation,
-		       mmap_cache_generation,
-		       tcp, tcp->mmap_cache);
-}
+	if (!cache.size)
+		return MMAP_CACHE_REBUILD_NOCACHE;
 
-/* deleting the cache */
-extern void
-mmap_cache_delete(struct tcb *tcp, const char *caller)
-{
-	unsigned int i;
+	tcp->mmap_cache = xmalloc(sizeof(*tcp->mmap_cache));
+	memcpy(tcp->mmap_cache, &cache, sizeof(cache));
 
 	debug_func_msg("tgen=%u, ggen=%u, tcp=%p, cache=%p, caller=%s",
-		       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;
-	}
-	free(tcp->mmap_cache);
-	tcp->mmap_cache = NULL;
-	tcp->mmap_cache_size = 0;
-}
+		       tcp->mmap_cache->generation, mmap_cache_generation,
+		       tcp, tcp->mmap_cache->entry, caller);
 
-extern enum mmap_cache_rebuild_result
-mmap_cache_rebuild_if_invalid(struct tcb *tcp, const char *caller)
-{
-	enum mmap_cache_rebuild_result r = MMAP_CACHE_REBUILD_READY;
-	if ((tcp->mmap_cache_generation != mmap_cache_generation)
-	    && tcp->mmap_cache)
-		mmap_cache_delete(tcp, caller);
-
-	if (!tcp->mmap_cache) {
-		r = MMAP_CACHE_REBUILD_RENEWED;
-		build_mmap_cache(tcp);
-	}
-
-	if (!(tcp->mmap_cache && tcp->mmap_cache_size))
-		r = MMAP_CACHE_REBUILD_NOCACHE;
-
-	return r;
+	return MMAP_CACHE_REBUILD_RENEWED;
 }
 
-struct mmap_cache_t *
+struct mmap_cache_entry_t *
 mmap_cache_search(struct tcb *tcp, unsigned long ip)
 {
+	if (!tcp->mmap_cache)
+		return NULL;
+
 	int lower = 0;
-	int upper = (int) tcp->mmap_cache_size - 1;
+	int upper = (int) tcp->mmap_cache->size - 1;
 
 	while (lower <= upper) {
-		struct mmap_cache_t *cur_mmap_cache;
 		int mid = (upper + lower) / 2;
+		struct mmap_cache_entry_t *entry = &tcp->mmap_cache->entry[mid];
 
-		cur_mmap_cache = &tcp->mmap_cache[mid];
-
-		if (ip >= cur_mmap_cache->start_addr &&
-		    ip < cur_mmap_cache->end_addr)
-			return cur_mmap_cache;
-		else if (ip < cur_mmap_cache->start_addr)
+		if (ip >= entry->start_addr &&
+		    ip < entry->end_addr)
+			return entry;
+		else if (ip < entry->start_addr)
 			upper = mid - 1;
 		else
 			lower = mid + 1;

mmap_cache.h

@@ -31,7 +31,15 @@
  * Keep a sorted array of cache entries,
  * so that we can binary search through it.
  */
+
 struct mmap_cache_t {
+	struct mmap_cache_entry_t *entry;
+	void (*free_fn)(struct tcb *, const char *caller);
+	unsigned int size;
+	unsigned int generation;
+};
+
+struct mmap_cache_entry_t {
 	/**
 	 * example entry:
 	 * 7fabbb09b000-7fabbb09f000 r-xp 00179000 fc:00 1180246 /lib/libc-2.11.1.so
@@ -68,13 +76,10 @@ enum mmap_cache_rebuild_result {
 extern void
 mmap_cache_enable(void);
 
-extern void
-mmap_cache_delete(struct tcb *, const char *caller);
-
 extern enum mmap_cache_rebuild_result
 mmap_cache_rebuild_if_invalid(struct tcb *, const char *caller);
 
-extern struct mmap_cache_t *
+extern struct mmap_cache_entry_t *
 mmap_cache_search(struct tcb *, unsigned long ip);
 
 #endif /* !STRACE_MMAP_CACHE_H */

strace.c

@@ -819,7 +819,8 @@ droptcb(struct tcb *tcp)
 		unwind_tcb_fin(tcp);
 #endif
 
-	mmap_cache_delete(tcp, __func__);
+	if (tcp->mmap_cache)
+		tcp->mmap_cache->free_fn(tcp, __func__);
 
 	nprocs--;
 	debug_msg("dropped tcb for pid %d, %d remain", tcp->pid, nprocs);

unwind-libunwind.c

@@ -89,26 +89,25 @@ print_stack_frame(struct tcb *tcp,
 		  size_t *symbol_name_size)
 {
 	unw_word_t ip;
-	struct mmap_cache_t *cur_mmap_cache;
 
 	if (unw_get_reg(cursor, UNW_REG_IP, &ip) < 0) {
 		perror_msg("cannot walk the stack of process %d", tcp->pid);
 		return -1;
 	}
 
-	cur_mmap_cache = mmap_cache_search(tcp, ip);
-	if (cur_mmap_cache
+	struct mmap_cache_entry_t *entry = mmap_cache_search(tcp, ip);
+
+	if (entry
 	    /* ignore mappings that have no PROT_EXEC bit set */
-	    && (cur_mmap_cache->protections & MMAP_CACHE_PROT_EXECUTABLE)) {
-		unsigned long true_offset;
+	    && (entry->protections & MMAP_CACHE_PROT_EXECUTABLE)) {
 		unw_word_t function_offset;
 
 		get_symbol_name(cursor, symbol_name, symbol_name_size,
 				&function_offset);
-		true_offset = ip - cur_mmap_cache->start_addr +
-			cur_mmap_cache->mmap_offset;
+		unsigned long true_offset =
+			ip - entry->start_addr + entry->mmap_offset;
 		call_action(data,
-			    cur_mmap_cache->binary_filename,
+			    entry->binary_filename,
 			    *symbol_name,
 			    function_offset,
 			    true_offset);
@@ -139,8 +138,6 @@ walk(struct tcb *tcp,
 
 	if (!tcp->mmap_cache)
 		error_func_msg_and_die("mmap_cache is NULL");
-	if (tcp->mmap_cache_size == 0)
-		error_func_msg_and_die("mmap_cache is empty");
 
 	symbol_name = xmalloc(symbol_name_size);