Merge branch 'misc-update'

Small refactoring including more comments added in the libmcount.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>

libmcount/internal.h

@@ -196,7 +196,7 @@ extern uint64_t mcount_threshold; /* nsec */
 extern unsigned mcount_minsize;
 extern pthread_key_t mtd_key;
 extern int shmem_bufsize;
-extern int pfd;
+extern int mcount_pfd;
 extern int mcount_depth;
 extern char *mcount_exename;
 extern int page_size_in_kb;
@@ -317,11 +317,11 @@ extern void uftrace_send_message(int type, void *data, size_t len);
 extern void build_debug_domain(char *dbg_domain_str);
 
 extern void mcount_rstack_restore(struct mcount_thread_data *mtdp);
-extern void mcount_rstack_reset(struct mcount_thread_data *mtdp);
-extern void mcount_rstack_reset_exception(struct mcount_thread_data *mtdp,
-					  unsigned long frame_addr);
+extern void mcount_rstack_rehook(struct mcount_thread_data *mtdp);
+extern void mcount_rstack_rehook_exception(struct mcount_thread_data *mtdp,
+					   unsigned long frame_addr);
 extern void mcount_auto_restore(struct mcount_thread_data *mtdp);
-extern void mcount_auto_reset(struct mcount_thread_data *mtdp);
+extern void mcount_auto_rehook(struct mcount_thread_data *mtdp);
 extern bool mcount_rstack_has_plthook(struct mcount_thread_data *mtdp);
 
 extern void prepare_shmem_buffer(struct mcount_thread_data *mtdp);

libmcount/mcount.c

@@ -32,6 +32,21 @@
 #include "utils/utils.h"
 #include "version.h"
 
+/*
+ * mcount global variables.
+ *
+ * These are to control various features in the libmcount.
+ * They are set during initialization (mcount_startup) which I believe, runs in
+ * a single thread.  After that multiple threads (mostly) read the value so it's
+ * not protected by lock or something.  So special care needs to be taken if you
+ * want to change it at runtime (like in the agent).
+ *
+ * Some are marked as 'maybe unused' because they are only used when filter
+ * functions are implemented.  Note that libmcount is built with different
+ * settings like -fast and -single to be more efficient in some situation like
+ * when no filter is specified in the command line and/or single-thread only.
+ */
+
 /* time filter in nsec */
 uint64_t mcount_threshold;
 
@@ -55,11 +70,17 @@ unsigned long mcount_global_flags = MCOUNT_GFL_SETUP;
 /* TSD key to save mtd below */
 pthread_key_t mtd_key = (pthread_key_t)-1;
 
-/* thread local data to trace function execution */
+/*
+ * A thread local data to trace function execution.
+ * While this is itself TLS so ok to by accessed safely by each thread,
+ * mcount routines use TSD APIs to access it for performance reason.
+ * Also TSD provides destructor so it can release the resources when the thread
+ * exits.
+ */
 TLS struct mcount_thread_data mtd;
 
 /* pipe file descriptor to communite to uftrace */
-int pfd = -1;
+int mcount_pfd = -1;
 
 /* maximum depth of mcount rstack */
 static int mcount_rstack_max = MCOUNT_RSTACK_MAX;
@@ -106,6 +127,10 @@ __weak void dynamic_return(void)
 static LIST_HEAD(mcount_watch_list);
 
 #ifdef DISABLE_MCOUNT_FILTER
+/*
+ * These functions are only the FAST version of libmcount libraries which don't
+ * implement filters (other than time and size filters).
+ */
 
 static void mcount_filter_init(struct uftrace_filter_setting *filter_setting, bool force)
 {
@@ -129,6 +154,9 @@ static void mcount_watch_finish(void)
 }
 
 #else
+/*
+ * Here goes the regular libmcount's filter and trigger functions.
+ */
 
 /* be careful: this can be called from signal handler */
 static void mcount_finish_trigger(void)
@@ -581,6 +609,10 @@ static void mcount_watch_release(struct mcount_thread_data *mtdp)
 
 #endif /* DISABLE_MCOUNT_FILTER */
 
+/*
+ * These are common routines used in every libmcount libraries.
+ */
+
 static void send_session_msg(struct mcount_thread_data *mtdp, const char *sess_id)
 {
 	struct uftrace_msg_sess sess = {
@@ -612,14 +644,14 @@ static void send_session_msg(struct mcount_thread_data *mtdp, const char *sess_i
 	};
 	int len = sizeof(msg) + msg.len;
 
-	if (pfd < 0)
+	if (mcount_pfd < 0)
 		return;
 
 	mcount_memcpy4(sess.sid, sess_id, sizeof(sess.sid));
 
-	if (writev(pfd, iov, 3) != len) {
+	if (writev(mcount_pfd, iov, 3) != len) {
 		if (!mcount_should_stop())
-			pr_err("write tid info failed");
+			pr_err("send session msg failed");
 	}
 }
 
@@ -640,9 +672,9 @@ static void mcount_trace_finish(bool send_msg)
 	if (send_msg)
 		uftrace_send_message(UFTRACE_MSG_FINISH, NULL, 0);
 
-	if (pfd != -1) {
-		close(pfd);
-		pfd = -1;
+	if (mcount_pfd != -1) {
+		close(mcount_pfd);
+		mcount_pfd = -1;
 	}
 
 	trace_finished = true;
@@ -919,6 +951,10 @@ static bool mcount_check_rstack(struct mcount_thread_data *mtdp)
 }
 
 #ifndef DISABLE_MCOUNT_FILTER
+/*
+ * Again, this implements filter functionality used in !fast versions.
+ */
+
 extern void *get_argbuf(struct mcount_thread_data *, struct mcount_ret_stack *);
 
 /**
@@ -1246,7 +1282,7 @@ void mcount_exit_filter_record(struct mcount_thread_data *mtdp, struct mcount_re
 			mtdp->filter.out_count--;
 
 		if (rstack->flags & MCOUNT_FL_RECOVER)
-			mcount_rstack_reset(mtdp);
+			mcount_rstack_rehook(mtdp);
 	}
 
 #undef FLAGS_TO_CHECK
@@ -1309,6 +1345,10 @@ void mcount_exit_filter_record(struct mcount_thread_data *mtdp, struct mcount_re
 }
 
 #else /* DISABLE_MCOUNT_FILTER */
+/*
+ * Here fast versions don't implement filters.
+ */
+
 enum filter_result mcount_entry_filter_check(struct mcount_thread_data *mtdp, unsigned long child,
 					     struct uftrace_trigger *tr)
 {
@@ -1448,7 +1488,7 @@ static int __mcount_entry(unsigned long *parent_loc, unsigned long child, struct
 		if (frame_addr < (unsigned long)parent_loc)
 			frame_addr = (unsigned long)(parent_loc - 1);
 
-		mcount_rstack_reset_exception(mtdp, frame_addr);
+		mcount_rstack_rehook_exception(mtdp, frame_addr);
 		mtdp->in_exception = false;
 	}
 
@@ -1522,7 +1562,7 @@ static unsigned long __mcount_exit(long *retval)
 
 	/* re-hijack return address of parent */
 	if (mcount_auto_recover)
-		mcount_auto_reset(mtdp);
+		mcount_auto_rehook(mtdp);
 
 	__mcount_unguard_recursion(mtdp);
 
@@ -1586,7 +1626,7 @@ static int __cygprof_entry(unsigned long parent, unsigned long child)
 		if (frame_addr < (unsigned long)frame_ptr)
 			frame_addr = (unsigned long)frame_ptr;
 
-		mcount_rstack_reset_exception(mtdp, frame_addr);
+		mcount_rstack_rehook_exception(mtdp, frame_addr);
 		mtdp->in_exception = false;
 	}
 
@@ -1733,7 +1773,7 @@ static void _xray_entry(unsigned long parent, unsigned long child, struct mcount
 		if (frame_addr < (unsigned long)frame_ptr)
 			frame_addr = (unsigned long)frame_ptr;
 
-		mcount_rstack_reset_exception(mtdp, frame_addr);
+		mcount_rstack_rehook_exception(mtdp, frame_addr);
 		mtdp->in_exception = false;
 	}
 
@@ -1979,7 +2019,7 @@ static __used void mcount_startup(void)
 		dirname = UFTRACE_DIR_NAME;
 
 	xasprintf(&channel, "%s/%s", dirname, ".channel");
-	pfd = open(channel, O_WRONLY);
+	mcount_pfd = open(channel, O_WRONLY);
 	free(channel);
 
 	if (getenv("UFTRACE_LIST_EVENT")) {
@@ -2093,14 +2133,20 @@ static void mcount_cleanup(void)
  */
 #define UFTRACE_ALIAS(_func) void uftrace_##_func(void *, void *) __alias(_func)
 
+/* This is the historic startup routine for mcount but not used here. */
 void __visible_default __monstartup(unsigned long low, unsigned long high)
 {
 }
 
+/* This is the historic cleanup routine for mcount but not used here. */
 void __visible_default _mcleanup(void)
 {
 }
 
+/*
+ * This is a non-standard external function to work around some stack
+ * corruption problems in the past.  I hope we don't need it anymore.
+ */
 void __visible_default mcount_restore(void)
 {
 	struct mcount_thread_data *mtdp;
@@ -2112,6 +2158,10 @@ void __visible_default mcount_restore(void)
 	mcount_rstack_restore(mtdp);
 }
 
+/*
+ * This is a non-standard external function to work around some stack
+ * corruption problems in the past.  I hope we don't need it anymore.
+ */
 void __visible_default mcount_reset(void)
 {
 	struct mcount_thread_data *mtdp;
@@ -2120,9 +2170,13 @@ void __visible_default mcount_reset(void)
 	if (unlikely(check_thread_data(mtdp)))
 		return;
 
-	mcount_rstack_reset(mtdp);
+	mcount_rstack_rehook(mtdp);
 }
 
+/*
+ * External entry points for -finstrument-functions.  The alias was added to
+ * avoid calling them through PLT.
+ */
 void __visible_default __cyg_profile_func_enter(void *child, void *parent)
 {
 	cygprof_entry((unsigned long)parent, (unsigned long)child);

libmcount/misc.c

@@ -76,13 +76,13 @@ void uftrace_send_message(int type, void *data, size_t len)
 		},
 	};
 
-	if (pfd < 0)
+	if (mcount_pfd < 0)
 		return;
 
 	len += sizeof(msg);
-	if (writev(pfd, iov, 2) != (ssize_t)len) {
+	if (writev(mcount_pfd, iov, 2) != (ssize_t)len) {
 		if (!mcount_should_stop())
-			pr_err("writing shmem name to pipe");
+			pr_err("send msg (type %d) failed", type);
 	}
 }
 
@@ -190,7 +190,7 @@ void mcount_rstack_restore(struct mcount_thread_data *mtdp)
 }
 
 /* hook return address again (used after mcount_rstack_restore) */
-void mcount_rstack_reset(struct mcount_thread_data *mtdp)
+void mcount_rstack_rehook(struct mcount_thread_data *mtdp)
 {
 	int idx;
 	struct mcount_ret_stack *rstack;
@@ -244,7 +244,7 @@ void mcount_auto_restore(struct mcount_thread_data *mtdp)
 	}
 }
 
-void mcount_auto_reset(struct mcount_thread_data *mtdp)
+void mcount_auto_rehook(struct mcount_thread_data *mtdp)
 {
 	struct mcount_ret_stack *curr_rstack;
 	struct mcount_ret_stack *prev_rstack;

libmcount/plthook.c

@@ -948,7 +948,7 @@ static unsigned long __plthook_entry(unsigned long *ret_addr, unsigned long chil
 			    strcmp(pd->mod_name, mcount_exename)) {
 				*ret_addr = rstack->parent_ip;
 				if (mcount_auto_recover)
-					mcount_auto_reset(mtdp);
+					mcount_auto_rehook(mtdp);
 
 				/*
 				 * as its return address was recovered,
@@ -1078,7 +1078,7 @@ static unsigned long __plthook_exit(long *retval)
 
 	/* re-hijack return address of parent */
 	if (mcount_auto_recover)
-		mcount_auto_reset(mtdp);
+		mcount_auto_rehook(mtdp);
 
 	__mcount_unguard_recursion(mtdp);
 

libmcount/record.c

@@ -240,6 +240,10 @@ static struct mcount_event *get_event_pointer(void *base, unsigned idx)
 }
 
 #ifndef DISABLE_MCOUNT_FILTER
+/*
+ * These functions are for regular libmcount with filters.
+ */
+
 void *get_argbuf(struct mcount_thread_data *mtdp, struct mcount_ret_stack *rstack)
 {
 	ptrdiff_t idx = rstack - mtdp->rstack;
@@ -818,6 +822,10 @@ void save_watchpoint(struct mcount_thread_data *mtdp, struct mcount_ret_stack *r
 }
 
 #else
+/*
+ * These are for fast libmcount libraries without filters.
+ */
+
 void *get_argbuf(struct mcount_thread_data *mtdp, struct mcount_ret_stack *rstack)
 {
 	return NULL;
@@ -893,7 +901,17 @@ static int record_event(struct mcount_thread_data *mtdp, struct mcount_event *ev
 
 	/*
 	 * instead of set bit fields, do the bit operations manually.
-	 * this would be good for both performance and portability.
+	 * this would be good for both performance and portability,
+	 * and should be equivalent to the following:
+	 *
+	 *	struct uftrace_record *data = curr_buf->data + curr_buf->size;
+	 *
+	 *	data->time   = event->time;
+	 *	data->type   = UFTRACE_EVENT;
+	 *	data->magic  = RECORD_MAGIC;
+	 *	data->more   = 0;
+	 *	data->depth  = 0;
+	 *	data->addr   = event->id;
 	 */
 	rec->data = UFTRACE_EVENT | RECORD_MAGIC << 3;
 	rec->data += (uint64_t)event->id << 16;
@@ -971,19 +989,19 @@ static int record_ret_stack(struct mcount_thread_data *mtdp, enum uftrace_record
 	if (curr_buf == NULL)
 		return mtdp->shmem.done ? 0 : -1;
 
-#if 0
-	frstack = (void *)(curr_buf->data + curr_buf->size);
-
-	frstack->time   = timestamp;
-	frstack->type   = type;
-	frstack->magic  = RECORD_MAGIC;
-	frstack->more   = !!argbuf;
-	frstack->depth  = mrstack->depth;
-	frstack->addr   = mrstack->child_ip;
-#else
 	/*
 	 * instead of set bit fields, do the bit operations manually.
-	 * this would be good for both performance and portability.
+	 * this would be good for both performance and portability,
+	 * and should be equivalent to the following:
+	 *
+	 *	frstack = (void *)(curr_buf->data + curr_buf->size);
+	 *
+	 *	frstack->time   = timestamp;
+	 *	frstack->type   = type;
+	 *	frstack->magic  = RECORD_MAGIC;
+	 *	frstack->more   = !!argbuf;
+	 *	frstack->depth  = mrstack->depth;
+	 *	frstack->addr   = mrstack->child_ip;
 	 */
 	rec = type | RECORD_MAGIC << 3;
 	rec += argbuf ? 4 : 0;
@@ -993,7 +1011,6 @@ static int record_ret_stack(struct mcount_thread_data *mtdp, enum uftrace_record
 	buf = (void *)(curr_buf->data + curr_buf->size);
 	buf[0] = timestamp;
 	buf[1] = rec;
-#endif
 
 	curr_buf->size += sizeof(*frstack);
 	mrstack->flags |= MCOUNT_FL_WRITTEN;
@@ -1033,6 +1050,30 @@ static int record_ret_stack(struct mcount_thread_data *mtdp, enum uftrace_record
 	return 0;
 }
 
+/*
+ * For performance reasons and time filter, it doesn't record trace data one at
+ * a time.  Instead it usually writes the data when an EXIT record is ready so
+ * it needs to record ENTRY data in the current and may in the parent functions.
+ *
+ * For example, if it has a time filter for 1 usec.
+ *
+ * foo() {
+ *   bar() {
+ *     leaf1();   // takes 0.5 usec
+ *     leaf2();   // takes 1.2 usec
+ *
+ * Then it can start to record when leaf2 function returns (at this moment,
+ * mcount_ret_stack for leaf1 is gone) then it'd save the following records
+ * (unless ENTRY foo or bar is saved by an earlier child before leaf[12]).
+ *
+ *   ENTRY (foo)
+ *   ENTRY (bar)
+ *   ENTRY (leaf2)
+ *   EXIT  (leaf2)
+ *
+ * Then it adds MCOUNT_FL_WRITTEN flag to parent (foo and bar) so that they
+ * never be written anymore by other child function.
+ */
 int record_trace_data(struct mcount_thread_data *mtdp, struct mcount_ret_stack *mrstack,
 		      long *retval)
 {

libmcount/wrap.c

@@ -56,14 +56,14 @@ static void send_dlopen_msg(struct mcount_thread_data *mtdp, const char *sess_id
 	};
 	int len = sizeof(msg) + msg.len;
 
-	if (pfd < 0)
+	if (mcount_pfd < 0)
 		return;
 
 	mcount_memcpy4(dlop.sid, sess_id, sizeof(dlop.sid));
 
-	if (writev(pfd, iov, 3) != len) {
+	if (writev(mcount_pfd, iov, 3) != len) {
 		if (!mcount_should_stop())
-			pr_err("write tid info failed");
+			pr_err("send dlopen msg failed");
 	}
 }
 
@@ -93,7 +93,7 @@ static int dlopen_base_callback(struct dl_phdr_info *info, size_t size, void *ar
 	return 0;
 }
 
-void mcount_rstack_reset_exception(struct mcount_thread_data *mtdp, unsigned long frame_addr)
+void mcount_rstack_rehook_exception(struct mcount_thread_data *mtdp, unsigned long frame_addr)
 {
 	int idx;
 	struct mcount_ret_stack *rstack;
@@ -148,7 +148,7 @@ void mcount_rstack_reset_exception(struct mcount_thread_data *mtdp, unsigned lon
 	mtdp->idx = idx + 1;
 	pr_dbg3("%s: exception returned to [%d]\n", __func__, mtdp->idx);
 
-	mcount_rstack_reset(mtdp);
+	mcount_rstack_rehook(mtdp);
 }
 
 static char **collect_uftrace_envp(void)
@@ -305,7 +305,7 @@ __visible_default int backtrace(void **buffer, int sz)
 	ret = real_backtrace(buffer, sz);
 
 	if (!check_thread_data(mtdp))
-		mcount_rstack_reset(mtdp);
+		mcount_rstack_rehook(mtdp);
 
 	return ret;
 }
@@ -326,7 +326,7 @@ __visible_default void __cxa_throw(void *exception, void *type, void *dest)
 		/*
 		 * restore return addresses so that it can unwind stack
 		 * frames safely during the exception handling.
-		 * It pairs to mcount_rstack_reset_exception().
+		 * It pairs to mcount_rstack_rehook_exception().
 		 */
 		mcount_rstack_restore(mtdp);
 	}
@@ -350,7 +350,7 @@ __visible_default void __cxa_rethrow(void)
 		/*
 		 * restore return addresses so that it can unwind stack
 		 * frames safely during the exception handling.
-		 * It pairs to mcount_rstack_reset_exception()
+		 * It pairs to mcount_rstack_rehook_exception()
 		 */
 		mcount_rstack_restore(mtdp);
 	}
@@ -374,7 +374,7 @@ __visible_default void _Unwind_Resume(void *exception)
 		/*
 		 * restore return addresses so that it can unwind stack
 		 * frames safely during the exception handling.
-		 * It pairs to mcount_rstack_reset_exception().
+		 * It pairs to mcount_rstack_rehook_exception().
 		 */
 		mcount_rstack_restore(mtdp);
 	}
@@ -404,7 +404,7 @@ __visible_default void *__cxa_begin_catch(void *exception)
 		if (frame_addr < (unsigned long)frame_ptr)
 			frame_addr = (unsigned long)frame_ptr;
 
-		mcount_rstack_reset_exception(mtdp, frame_addr);
+		mcount_rstack_rehook_exception(mtdp, frame_addr);
 		mtdp->in_exception = false;
 		pr_dbg2("%s: exception caught begin on [%d]\n", __func__, mtdp->idx);
 	}
@@ -443,7 +443,7 @@ __visible_default void __cxa_guard_abort(void *guard_obj)
 		if (frame_addr < (unsigned long)frame_ptr)
 			frame_addr = (unsigned long)frame_ptr;
 
-		mcount_rstack_reset_exception(mtdp, frame_addr);
+		mcount_rstack_rehook_exception(mtdp, frame_addr);
 		mtdp->in_exception = false;
 
 		/*
@@ -615,7 +615,7 @@ __visible_default int close(int fd)
 	if (unlikely(real_close == NULL))
 		mcount_hook_functions();
 
-	if (unlikely(fd == pfd)) {
+	if (unlikely(fd == mcount_pfd)) {
 		return 0;
 	}