From 8e5a699e0541afdfc4ca5d91623cdb3abbf91b4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Fri, 10 Mar 2023 11:22:00 -0500 Subject: [PATCH 1/3] dynamic: Don't patch all functions by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Originally, when the user asks to only unpatch functions, uftrace would patch all other functions by default. This is counter-intuitive, especially when using the agent to unpatch at runtime. The user doesn't expect functions to be patched when they only unpatch funtions. This commit removes this behavior, and requires the user to explicitly define functions to patch. Co-authored-by: Gabriel-Andrew Pollo-Guilbert Signed-off-by: Clément Guidi --- libmcount/dynamic.c | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/libmcount/dynamic.c b/libmcount/dynamic.c index c9c058eb5..c180cc4f5 100644 --- a/libmcount/dynamic.c +++ b/libmcount/dynamic.c @@ -410,7 +410,6 @@ static void parse_pattern_list(char *patch_funcs, char *def_mod, enum uftrace_pa char *name; int j; struct patt_list *pl; - bool all_negative = true; strv_split(&funcs, patch_funcs, ";"); @@ -421,10 +420,8 @@ static void parse_pattern_list(char *patch_funcs, char *def_mod, enum uftrace_pa if (name[0] == '!') name++; - else { + else pl->positive = true; - all_negative = false; - } delim = strchr(name, '@'); if (delim == NULL) { @@ -439,20 +436,6 @@ static void parse_pattern_list(char *patch_funcs, char *def_mod, enum uftrace_pa list_add_tail(&pl->list, &patterns); } - /* prepend match-all pattern, if all patterns are negative */ - if (all_negative) { - pl = xzalloc(sizeof(*pl)); - pl->positive = true; - pl->module = xstrdup(def_mod); - - if (ptype == PATT_REGEX) - init_filter_pattern(ptype, &pl->patt, "."); - else - init_filter_pattern(PATT_GLOB, &pl->patt, "*"); - - list_add(&pl->list, &patterns); - } - strv_free(&funcs); } From 8882c3fc32f766832ea58bfa1893030fe6bb9557 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Tue, 21 Mar 2023 10:18:20 -0400 Subject: [PATCH 2/3] dynamic: Don't unpatch unmatched functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Libmcount would try to unpatch by default any function that is not matched by the user patch or unpatch options. We remove this implicit behavior, so the user explicitly choses which function to unpatch. Signed-off-by: Clément Guidi --- libmcount/dynamic.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/libmcount/dynamic.c b/libmcount/dynamic.c index c180cc4f5..aa786bfbd 100644 --- a/libmcount/dynamic.c +++ b/libmcount/dynamic.c @@ -470,11 +470,8 @@ static bool skip_sym(struct uftrace_symbol *sym, struct mcount_dynamic_info *mdi if (sym->type != ST_LOCAL_FUNC && sym->type != ST_GLOBAL_FUNC && sym->type != ST_WEAK_FUNC) return true; - if (!match_pattern_list(map, soname, sym->name)) { - if (mcount_unpatch_func(mdi, sym, &disasm) == 0) - stats.unpatch++; + if (!match_pattern_list(map, soname, sym->name)) return true; - } return false; } From 151fb6119d90d1834b548bb203f273430a929b0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Guidi?= Date: Mon, 10 Apr 2023 15:10:21 -0400 Subject: [PATCH 3/3] dynamic: Refactor 'match_pattern_list' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Return whether a symbol is positively or negatively matched against a pattern list, or not matched at all. Co-authored-by: Gabriel-Andrew Pollo-Guilbert Signed-off-by: Clément Guidi --- libmcount/dynamic.c | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/libmcount/dynamic.c b/libmcount/dynamic.c index aa786bfbd..6e7b56dd9 100644 --- a/libmcount/dynamic.c +++ b/libmcount/dynamic.c @@ -384,10 +384,17 @@ static bool match_pattern_module(char *pathname) return ret; } -static bool match_pattern_list(struct uftrace_mmap *map, char *soname, char *sym_name) +/** + * match_pattern_list - match a symbol name against a pattern list + * @map - memory map of the symbol + * @soname - name of the module + * @sym_name - name of the symbol + * @return - -1 if match negative, 1 if match positive, 0 if no match + */ +static int match_pattern_list(struct uftrace_mmap *map, char *soname, char *sym_name) { struct patt_list *pl; - bool ret = false; + int ret = 0; char *libname = basename(map->libname); list_for_each_entry(pl, &patterns, list) { @@ -398,7 +405,7 @@ static bool match_pattern_list(struct uftrace_mmap *map, char *soname, char *sym continue; if (match_filter_pattern(&pl->patt, sym_name)) - ret = pl->positive; + ret = pl->positive ? 1 : -1; } return ret; @@ -470,9 +477,6 @@ static bool skip_sym(struct uftrace_symbol *sym, struct mcount_dynamic_info *mdi if (sym->type != ST_LOCAL_FUNC && sym->type != ST_GLOBAL_FUNC && sym->type != ST_WEAK_FUNC) return true; - if (!match_pattern_list(map, soname, sym->name)) - return true; - return false; } @@ -539,6 +543,7 @@ static void patch_normal_func_matched(struct mcount_dynamic_info *mdi, struct uf unsigned i; struct uftrace_symbol *sym; bool found = false; + int match; char *soname = get_soname(map->libname); symtab = &map->mod->symtab; @@ -548,9 +553,15 @@ static void patch_normal_func_matched(struct mcount_dynamic_info *mdi, struct uf if (skip_sym(sym, mdi, map, soname)) continue; - found = true; - mcount_patch_func_with_stats(mdi, sym); + + match = match_pattern_list(map, soname, sym->name); + if (!match) + continue; + else if (match == 1) + mcount_patch_func_with_stats(mdi, sym); + else + mcount_unpatch_func(mdi, sym, NULL); } if (!found) @@ -826,27 +837,27 @@ TEST_CASE(dynamic_pattern_list) pr_dbg("check simple match with default module\n"); parse_pattern_list("abc;!def", "main", PATT_SIMPLE); - TEST_EQ(match_pattern_list(main_map, NULL, "abc"), true); - TEST_EQ(match_pattern_list(main_map, NULL, "def"), false); - TEST_EQ(match_pattern_list(other_map, NULL, "xyz"), false); + TEST_EQ(match_pattern_list(main_map, NULL, "abc"), 1); + TEST_EQ(match_pattern_list(main_map, NULL, "def"), -1); + TEST_EQ(match_pattern_list(other_map, NULL, "xyz"), 0); release_pattern_list(); pr_dbg("check negative regex match with default module\n"); parse_pattern_list("!^a", "main", PATT_REGEX); - TEST_EQ(match_pattern_list(main_map, NULL, "abc"), false); - TEST_EQ(match_pattern_list(main_map, NULL, "def"), true); - TEST_EQ(match_pattern_list(other_map, NULL, "xyz"), false); + TEST_EQ(match_pattern_list(main_map, NULL, "abc"), -1); + TEST_EQ(match_pattern_list(main_map, NULL, "def"), 0); + TEST_EQ(match_pattern_list(other_map, NULL, "xyz"), 0); release_pattern_list(); pr_dbg("check wildcard match with other module\n"); parse_pattern_list("*@other", "main", PATT_GLOB); - TEST_EQ(match_pattern_list(main_map, NULL, "abc"), false); - TEST_EQ(match_pattern_list(main_map, NULL, "def"), false); - TEST_EQ(match_pattern_list(other_map, NULL, "xyz"), true); + TEST_EQ(match_pattern_list(main_map, NULL, "abc"), 0); + TEST_EQ(match_pattern_list(main_map, NULL, "def"), 0); + TEST_EQ(match_pattern_list(other_map, NULL, "xyz"), 1); release_pattern_list();