Merge branch 'misc-update'

Small refactoring including more comments added in the libmcount.

Signed-off-by: Namhyung Kim <[email protected]>

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);