[PATCH 0/4] perf-probe: Improbe non-C language support

[PATCH 0/4] perf-probe: Improbe non-C language support

[Masami Hiramatsu (Google)]

Hi,

Here is a series of patches for perf probe to improve non-C language
(e.g. Rust, Go) support.

The non-C symbols are demangled style in debuginfo, e.g. golang stores

----
$ ./perf probe -x /work/go/example/outyet/main -F main*
main.(*Server).ServeHTTP
main.(*Server).ServeHTTP.Print.func1
main.(*Server).poll
...
-----

And Rust stores
-----
$ ./perf probe -x /work/cro3/target/x86_64-unknown-linux-gnu/debug/cro3 -F cro3::cmd::servo*
cro3::cmd::servo::run
cro3::cmd::servo::run::CALLSITE
cro3::cmd::servo::run::CALLSITE::META
cro3::cmd::servo::run_control
-----

These symbols are not parsed correctly because it looks like a file name or
including line numbers (`:` caused it.) So, I decided to introduce the changes

 - filename MUST start from '@'. (so it is able to distinguish the filename
   and the function name)
 - Fix to allow backslash to escape to --lines option.
 - Introduce quotation mark support.
 - Replace non-alnum character to '_' for event name (for non-C symbols).

With these changes, we can run -L (--lines) on golang;

------
$ perf probe -x goexample/hello/hello -L \"main.main\"
<main.main@/work/goexample/hello/hello.go:0>
      0  func main() {
                // Configure logging for a command-line program.
      2         log.SetFlags(0)
      3         log.SetPrefix("hello: ")

                // Parse flags.
      6         flag.Usage = usage
      7         flag.Parse()
------

And Rust
------
$ perf probe -x cro3 -L \"cro3::cmd::servo::run_show\"
<run_show@/work/cro3/src/cmd/servo.rs:0>
      0  fn run_show(args: &ArgsShow) -> Result<()> {
      1      let list = ServoList::discover()?;
      2      let s = list.find_by_serial(&args.servo)?;
      3      if args.json {
      4          println!("{s}");
------

And event name are created automatically like below;

$ ./perf probe -x /work/go/example/outyet/main -D 'main.(*Server).poll'
p:probe_main/main_Server_poll /work/go/example/outyet/main:0x353040

$ ./perf probe -x cro3 -D \"cro3::cmd::servo::run_show\"
p:probe_cro3/cro3_cmd_servo_run_show /work/cro3/target/x86_64-unknown-linux-gnu/debug/cro3:0x197530

We still need some more work, but these shows how perf-probe can work
with other languages.

Thank you,

---

Masami Hiramatsu (Google) (4):
      perf-probe: Fix to ignore escaped characters in --lines option
      perf-probe: Require '@' prefix for filename always
      perf-probe: Introduce quotation marks support
      perf-probe: Replace unacceptable characters when generating event name


 tools/perf/util/probe-event.c  |  136 ++++++++++++++++++++++------------------
 tools/perf/util/probe-finder.c |    3 +
 tools/perf/util/string.c       |  100 +++++++++++++++++++++++++++++
 tools/perf/util/string2.h      |    2 +
 4 files changed, 180 insertions(+), 61 deletions(-)

--
Masami Hiramatsu (Google) <mhiramat@kernel.org>

[PATCH 1/4] perf-probe: Fix to ignore escaped characters in --lines option

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Use strbprk_esc() and strdup_esc() to ignore escaped characters in
--lines option. This has been done for other options, but only --lines
option doesn't.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 tools/perf/util/probe-event.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index a17c9b8a7a79..665dcce482e1 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1355,7 +1355,7 @@ int parse_line_range_desc(const char *arg, struct line_range *lr)
 	lr->start = 0;
 	lr->end = INT_MAX;
 
-	range = strchr(name, ':');
+	range = strpbrk_esc(name, ":");
 	if (range) {
 		*range++ = '\0';
 
@@ -1396,16 +1396,16 @@ int parse_line_range_desc(const char *arg, struct line_range *lr)
 		}
 	}
 
-	file = strchr(name, '@');
+	file = strpbrk_esc(name, "@");
 	if (file) {
 		*file = '\0';
-		lr->file = strdup(++file);
+		lr->file = strdup_esc(++file);
 		if (lr->file == NULL) {
 			err = -ENOMEM;
 			goto err;
 		}
 		lr->function = name;
-	} else if (strchr(name, '/') || strchr(name, '.'))
+	} else if (strpbrk_esc(name, "/."))
 		lr->file = name;
 	else if (is_c_func_name(name))/* We reuse it for checking funcname */
 		lr->function = name;

[PATCH 2/4] perf-probe: Require '@' prefix for filename always

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Currently perf probe allows user to specify probing place without '@'
prefix, for example, both `-V file:line` and `-V function:line` are
allowed. But this makes a problem if a (demangled) function name is
hard to be distinguished from a file name.

This changes the perf-probe to require '@' prefix for filename even
without function name. For example, `-V @file:line` and
`-V function:line` are acceptable.

With this change, users can specify filename or function correctly.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 tools/perf/util/probe-event.c |   31 +++++++++----------------------
 1 file changed, 9 insertions(+), 22 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 665dcce482e1..913a27cbb5d9 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1341,7 +1341,7 @@ static bool is_c_func_name(const char *name)
  * Stuff 'lr' according to the line range described by 'arg'.
  * The line range syntax is described by:
  *
- *         SRC[:SLN[+NUM|-ELN]]
+ *         @SRC[:SLN[+NUM|-ELN]]
  *         FNC[@SRC][:SLN[+NUM|-ELN]]
  */
 int parse_line_range_desc(const char *arg, struct line_range *lr)
@@ -1404,16 +1404,10 @@ int parse_line_range_desc(const char *arg, struct line_range *lr)
 			err = -ENOMEM;
 			goto err;
 		}
+		if (*name != '\0')
+			lr->function = name;
+	} else
 		lr->function = name;
-	} else if (strpbrk_esc(name, "/."))
-		lr->file = name;
-	else if (is_c_func_name(name))/* We reuse it for checking funcname */
-		lr->function = name;
-	else {	/* Invalid name */
-		semantic_error("'%s' is not a valid function name.\n", name);
-		err = -EINVAL;
-		goto err;
-	}
 
 	return 0;
 err:
@@ -1463,7 +1457,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 
 	/*
 	 * <Syntax>
-	 * perf probe [GRP:][EVENT=]SRC[:LN|;PTN]
+	 * perf probe [GRP:][EVENT=]@SRC[:LN|;PTN]
 	 * perf probe [GRP:][EVENT=]FUNC[@SRC][+OFFS|%return|:LN|;PAT]
 	 * perf probe %[GRP:]SDT_EVENT
 	 */
@@ -1516,19 +1510,12 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 	/*
 	 * Check arg is function or file name and copy it.
 	 *
-	 * We consider arg to be a file spec if and only if it satisfies
-	 * all of the below criteria::
-	 * - it does not include any of "+@%",
-	 * - it includes one of ":;", and
-	 * - it has a period '.' in the name.
-	 *
+	 * We consider arg to be a file spec if it starts with '@'.
 	 * Otherwise, we consider arg to be a function specification.
 	 */
-	if (!strpbrk_esc(arg, "+@%")) {
-		ptr = strpbrk_esc(arg, ";:");
-		/* This is a file spec if it includes a '.' before ; or : */
-		if (ptr && memchr(arg, '.', ptr - arg))
-			file_spec = true;
+	if (*arg == '@') {
+		file_spec = true;
+		arg++;
 	}
 
 	ptr = strpbrk_esc(arg, ";:+@%");

[PATCH 3/4] perf-probe: Introduce quotation marks support

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

In non-C languages, it is possible to have ':' in the function names.
It is possible to escape it with backslashes, but if there are too many
backslashes, it is annoying.
This introduce quotation marks (`"` or `'`) support.

For example, without quotes, we have to pass it as below

$ perf probe -x cro3 -L "cro3\:\:cmd\:\:servo\:\:run_show"
<run_show@/work/cro3/src/cmd/servo.rs:0>
      0  fn run_show(args: &ArgsShow) -> Result<()> {
      1      let list = ServoList::discover()?;
      2      let s = list.find_by_serial(&args.servo)?;
      3      if args.json {
      4          println!("{s}");

With quotes, we can more naturally write the function name as below;

$ ./perf probe -x cro3 -L \"cro3::cmd::servo::run_show\"
<run_show@/work/cro3/src/cmd/servo.rs:0>
      0  fn run_show(args: &ArgsShow) -> Result<()> {
      1      let list = ServoList::discover()?;
      2      let s = list.find_by_serial(&args.servo)?;
      3      if args.json {
      4          println!("{s}");

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 tools/perf/util/probe-event.c  |   76 ++++++++++++++++--------------
 tools/perf/util/probe-finder.c |    3 +
 tools/perf/util/string.c       |  100 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/string2.h      |    2 +
 4 files changed, 145 insertions(+), 36 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 913a27cbb5d9..bcba8273204d 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1065,6 +1065,7 @@ static int __show_line_range(struct line_range *lr, const char *module,
 
 	ret = debuginfo__find_line_range(dinfo, lr);
 	if (!ret) {	/* Not found, retry with an alternative */
+		pr_debug2("Failed to find line range in debuginfo. Fallback to alternative\n");
 		ret = get_alternative_line_range(dinfo, lr, module, user);
 		if (!ret)
 			ret = debuginfo__find_line_range(dinfo, lr);
@@ -1078,7 +1079,7 @@ static int __show_line_range(struct line_range *lr, const char *module,
 		pr_warning("Specified source line is not found.\n");
 		return -ENOENT;
 	} else if (ret < 0) {
-		pr_warning("Debuginfo analysis failed.\n");
+		pr_warning("Debuginfo analysis failed (%d).\n", ret);
 		return ret;
 	}
 
@@ -1187,7 +1188,7 @@ static int show_available_vars_at(struct debuginfo *dinfo,
 			pr_err("Failed to find the address of %s\n", buf);
 			ret = -ENOENT;
 		} else
-			pr_warning("Debuginfo analysis failed.\n");
+			pr_warning("Debuginfo analysis failed(2).\n");
 		goto end;
 	}
 
@@ -1343,30 +1344,39 @@ static bool is_c_func_name(const char *name)
  *
  *         @SRC[:SLN[+NUM|-ELN]]
  *         FNC[@SRC][:SLN[+NUM|-ELN]]
+ *
+ * SRC and FUNC can be quoted by double/single quotes.
  */
 int parse_line_range_desc(const char *arg, struct line_range *lr)
 {
-	char *range, *file, *name = strdup(arg);
+	char *buf = strdup(arg);
+	char *p;
 	int err;
 
-	if (!name)
+	if (!buf)
 		return -ENOMEM;
 
 	lr->start = 0;
 	lr->end = INT_MAX;
 
-	range = strpbrk_esc(name, ":");
-	if (range) {
-		*range++ = '\0';
+	pr_debug2("Input line range: %s\n", buf);
+	p = strpbrk_esq(buf, ":");
+	if (p) {
+		if (p == buf) {
+			semantic_error("No file/function name in '%s'.\n", p);
+			err = -EINVAL;
+			goto err;
+		}
+		*(p++) = '\0';
 
-		err = parse_line_num(&range, &lr->start, "start line");
+		err = parse_line_num(&p, &lr->start, "start line");
 		if (err)
 			goto err;
 
-		if (*range == '+' || *range == '-') {
-			const char c = *range++;
+		if (*p == '+' || *p == '-') {
+			const char c = *(p++);
 
-			err = parse_line_num(&range, &lr->end, "end line");
+			err = parse_line_num(&p, &lr->end, "end line");
 			if (err)
 				goto err;
 
@@ -1390,28 +1400,22 @@ int parse_line_range_desc(const char *arg, struct line_range *lr)
 				       " than end line.\n");
 			goto err;
 		}
-		if (*range != '\0') {
-			semantic_error("Tailing with invalid str '%s'.\n", range);
+		if (*p != '\0') {
+			semantic_error("Tailing with invalid str '%s'.\n", p);
 			goto err;
 		}
 	}
 
-	file = strpbrk_esc(name, "@");
-	if (file) {
-		*file = '\0';
-		lr->file = strdup_esc(++file);
-		if (lr->file == NULL) {
-			err = -ENOMEM;
-			goto err;
-		}
-		if (*name != '\0')
-			lr->function = name;
-	} else
-		lr->function = name;
+	p = strpbrk_esq(buf, "@");
+	if (p) {
+		*(p++) = '\0';
+		lr->file = strdup_esq(p);
+	}
+	lr->function = strdup_esq(buf);
+	err = 0;
 
-	return 0;
 err:
-	free(name);
+	free(buf);
 	return err;
 }
 
@@ -1419,19 +1423,19 @@ static int parse_perf_probe_event_name(char **arg, struct perf_probe_event *pev)
 {
 	char *ptr;
 
-	ptr = strpbrk_esc(*arg, ":");
+	ptr = strpbrk_esq(*arg, ":");
 	if (ptr) {
 		*ptr = '\0';
 		if (!pev->sdt && !is_c_func_name(*arg))
 			goto ng_name;
-		pev->group = strdup_esc(*arg);
+		pev->group = strdup_esq(*arg);
 		if (!pev->group)
 			return -ENOMEM;
 		*arg = ptr + 1;
 	} else
 		pev->group = NULL;
 
-	pev->event = strdup_esc(*arg);
+	pev->event = strdup_esq(*arg);
 	if (pev->event == NULL)
 		return -ENOMEM;
 
@@ -1470,7 +1474,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 			arg++;
 	}
 
-	ptr = strpbrk_esc(arg, ";=@+%");
+	ptr = strpbrk_esq(arg, ";=@+%");
 	if (pev->sdt) {
 		if (ptr) {
 			if (*ptr != '@') {
@@ -1484,7 +1488,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 				pev->target = build_id_cache__origname(tmp);
 				free(tmp);
 			} else
-				pev->target = strdup_esc(ptr + 1);
+				pev->target = strdup_esq(ptr + 1);
 			if (!pev->target)
 				return -ENOMEM;
 			*ptr = '\0';
@@ -1518,7 +1522,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 		arg++;
 	}
 
-	ptr = strpbrk_esc(arg, ";:+@%");
+	ptr = strpbrk_esq(arg, ";:+@%");
 	if (ptr) {
 		nc = *ptr;
 		*ptr++ = '\0';
@@ -1527,7 +1531,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 	if (arg[0] == '\0')
 		tmp = NULL;
 	else {
-		tmp = strdup_esc(arg);
+		tmp = strdup_esq(arg);
 		if (tmp == NULL)
 			return -ENOMEM;
 	}
@@ -1565,7 +1569,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 				return -ENOMEM;
 			break;
 		}
-		ptr = strpbrk_esc(arg, ";:+@%");
+		ptr = strpbrk_esq(arg, ";:+@%");
 		if (ptr) {
 			nc = *ptr;
 			*ptr++ = '\0';
@@ -1592,7 +1596,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 				semantic_error("SRC@SRC is not allowed.\n");
 				return -EINVAL;
 			}
-			pp->file = strdup_esc(arg);
+			pp->file = strdup_esq(arg);
 			if (pp->file == NULL)
 				return -ENOMEM;
 			break;
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 630e16c54ed5..5462b5541a6d 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1796,6 +1796,7 @@ int debuginfo__find_line_range(struct debuginfo *dbg, struct line_range *lr)
 		struct dwarf_callback_param line_range_param = {
 			.data = (void *)&lf, .retval = 0};
 
+		pr_debug2("Start searching function '%s' in getpubname\n", lr->function);
 		dwarf_getpubnames(dbg->dbg, pubname_search_cb,
 				  &pubname_param, 0);
 		if (pubname_param.found) {
@@ -1803,8 +1804,10 @@ int debuginfo__find_line_range(struct debuginfo *dbg, struct line_range *lr)
 			if (lf.found)
 				goto found;
 		}
+		pr_debug2("Not found, use DIE tree\n");
 	}
 
+	pr_debug2("Search function '%s' in DIE tree\n", lr->function);
 	/* Loop on CUs (Compilation Unit) */
 	while (!lf.found && ret >= 0) {
 		if (dwarf_nextcu(dbg->dbg, off, &noff, &cuhl,
diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
index 116a642ad99d..308fc7ec88cc 100644
--- a/tools/perf/util/string.c
+++ b/tools/perf/util/string.c
@@ -263,6 +263,34 @@ char *strpbrk_esc(char *str, const char *stopset)
 	return ptr;
 }
 
+/* Like strpbrk_esc(), but not break if it is quoted with single/double quotes */
+char *strpbrk_esq(char *str, const char *stopset)
+{
+	char *_stopset = NULL;
+	char *ptr;
+	const char *squote = "'";
+	const char *dquote = "\"";
+
+	if (asprintf(&_stopset, "%s%c%c", stopset, *squote, *dquote) < 0)
+		return NULL;
+
+	do {
+		ptr = strpbrk_esc(str, _stopset);
+		if (!ptr)
+			break;
+		if (*ptr == *squote)
+			ptr = strpbrk_esc(ptr + 1, squote);
+		else if (*ptr == *dquote)
+			ptr = strpbrk_esc(ptr + 1, dquote);
+		else
+			break;
+		str = ptr + 1;
+	} while (ptr);
+
+	free(_stopset);
+	return ptr;
+}
+
 /* Like strdup, but do not copy a single backslash */
 char *strdup_esc(const char *str)
 {
@@ -293,6 +321,78 @@ char *strdup_esc(const char *str)
 	return ret;
 }
 
+/* Remove backslash right before quote and return next quote address. */
+static char *remove_consumed_esc(char *str, int len, int quote)
+{
+	char *ptr = str, *end = str + len;
+
+	while (*ptr != quote && ptr < end) {
+		if (*ptr == '\\' && *(ptr + 1) == quote) {
+			memmove(ptr, ptr + 1, end - (ptr + 1));
+			/* now *ptr is `quote`. */
+			end--;
+		}
+		ptr++;
+	}
+
+	return *ptr == quote ? ptr : NULL;
+}
+
+/*
+ * Like strdup_esc, but keep quoted string as it is (and single backslash
+ * before quote is removed). If there is no closed quote, return NULL.
+ */
+char *strdup_esq(const char *str)
+{
+	char *d, *ret;
+
+	/* If there is no quote, return normal strdup_esc() */
+	d = strpbrk_esc((char *)str, "\"'");
+	if (!d)
+		return strdup_esc(str);
+
+	ret = strdup(str);
+	if (!ret)
+		return NULL;
+
+	d = ret;
+	do {
+		d = strpbrk(d, "\\\"\'");
+		if (!d)
+			break;
+
+		if (*d == '"' || *d == '\'') {
+			/* This is non-escaped quote */
+			int quote = *d;
+			int len = strlen(d + 1) + 1;
+
+			/*
+			 * Remove the start quote and remove consumed escape (backslash
+			 * before quote) and remove the end quote. If there is no end
+			 * quote, it is the input error.
+			 */
+			memmove(d, d + 1, len);
+			d = remove_consumed_esc(d, len, quote);
+			if (!d)
+				goto error;
+			memmove(d, d + 1, strlen(d + 1) + 1);
+		}
+		if (*d == '\\') {
+			memmove(d, d + 1, strlen(d + 1) + 1);
+			if (*d == '\\') {
+				/* double backslash -- keep the second one. */
+				d++;
+			}
+		}
+	} while (*d != '\0');
+
+	return ret;
+
+error:
+	free(ret);
+	return NULL;
+}
+
 unsigned int hex(char c)
 {
 	if (c >= '0' && c <= '9')
diff --git a/tools/perf/util/string2.h b/tools/perf/util/string2.h
index 52cb8ba057c7..4c8bff47cfd3 100644
--- a/tools/perf/util/string2.h
+++ b/tools/perf/util/string2.h
@@ -37,6 +37,8 @@ char *asprintf__tp_filter_pids(size_t npids, pid_t *pids);
 
 char *strpbrk_esc(char *str, const char *stopset);
 char *strdup_esc(const char *str);
+char *strpbrk_esq(char *str, const char *stopset);
+char *strdup_esq(const char *str);
 
 unsigned int hex(char c);
 char *strreplace_chars(char needle, const char *haystack, const char *replace);

[PATCH 4/4] perf-probe: Replace unacceptable characters when generating event name

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Replace unacceptable characters with '_' when generating event name from
the probing function name. This is not for C program. For the C program,
it will continue to remove suffixes.

For example.

$ ./perf probe -x cro3 -D \"cro3::cmd::servo::run_show\"
p:probe_cro3/cro3_cmd_servo_run_show /work/cro3/target/x86_64-unknown-linux-gnu/debug/cro3:0x197530

$ ./perf probe -x /work/go/example/outyet/main -D 'main.(*Server).poll'
p:probe_main/main_Server_poll /work/go/example/outyet/main:0x353040

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 tools/perf/util/probe-event.c |   37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index bcba8273204d..27698b9a8c95 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2729,7 +2729,7 @@ int show_perf_probe_events(struct strfilter *filter)
 
 static int get_new_event_name(char *buf, size_t len, const char *base,
 			      struct strlist *namelist, bool ret_event,
-			      bool allow_suffix)
+			      bool allow_suffix, bool is_C_symname)
 {
 	int i, ret;
 	char *p, *nbase;
@@ -2740,10 +2740,24 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
 	if (!nbase)
 		return -ENOMEM;
 
-	/* Cut off the dot suffixes (e.g. .const, .isra) and version suffixes */
-	p = strpbrk(nbase, ".@");
-	if (p && p != nbase)
-		*p = '\0';
+	if (is_C_symname) {
+		/* Cut off the dot suffixes (e.g. .const, .isra) and version suffixes */
+		p = strpbrk(nbase, ".@");
+		if (p && p != nbase)
+			*p = '\0';
+	} else {
+		/* Replace non-alnum with '_' */
+		char *s, *d;
+
+		s = d = nbase;
+		do {
+			if (*s && !isalnum(*s)) {
+				if (d != nbase && *(d - 1) != '_')
+					*d++ = '_';
+			} else
+				*d++ = *s;
+		} while (*s++);
+	}
 
 	/* Try no suffix number */
 	ret = e_snprintf(buf, len, "%s%s", nbase, ret_event ? "__return" : "");
@@ -2832,6 +2846,7 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
 				       bool allow_suffix)
 {
 	const char *event, *group;
+	bool is_C_symname = false;
 	char buf[64];
 	int ret;
 
@@ -2846,8 +2861,16 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
 			(strncmp(pev->point.function, "0x", 2) != 0) &&
 			!strisglob(pev->point.function))
 			event = pev->point.function;
-		else
+		else {
 			event = tev->point.realname;
+			/*
+			 * `realname` comes from real symbol, which can have a suffix.
+			 * However, if we see some glab-like wildcard in the symbol, it
+			 * might not be a C program.
+			 */
+			if (!strisglob(event))
+				is_C_symname = true;
+		}
 	}
 	if (pev->group && !pev->sdt)
 		group = pev->group;
@@ -2858,7 +2881,7 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
 
 	/* Get an unused new event name */
 	ret = get_new_event_name(buf, sizeof(buf), event, namelist,
-				 tev->point.retprobe, allow_suffix);
+				 tev->point.retprobe, allow_suffix, is_C_symname);
 	if (ret < 0)
 		return ret;
 

Re: [PATCH 0/4] perf-probe: Improbe non-C language support

[Arnaldo Carvalho de Melo]

On Tue, Nov 05, 2024 at 01:17:08AM +0900, Masami Hiramatsu (Google) wrote:
> Hi,
 
> Here is a series of patches for perf probe to improve non-C language
> (e.g. Rust, Go) support.
 
> The non-C symbols are demangled style in debuginfo, e.g. golang stores
 
> ----
> $ ./perf probe -x /work/go/example/outyet/main -F main*
> main.(*Server).ServeHTTP
> main.(*Server).ServeHTTP.Print.func1
> main.(*Server).poll
> ...
> -----

I presented about this last year:

https://tracingsummit.org/ts/2023/bpf-non-c/
https://tracingsummit.org/ts/2023/files/Trying_to_use_uprobes_and_BPF_on_non-C_userspace.pdf
https://www.youtube.com/watch?v=RDFRy1vWyHg&feature=youtu.be

So trying to do some of the things I did while playing with golang, and
with your patches applied, I only had to cope with a minor clash with a
patch by Ian Rogers that is present on perf-tools-next, related to:

        char buf[MAX_EVENT_NAME_LEN];

That in your patch expects the old 64 hard coded value, which will
appear in the my tests:

root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -x main -F github*counter*
github.com/prometheus/client_golang/prometheus.(*counter).Add
github.com/prometheus/client_golang/prometheus.(*counter).AddWithExemplar
github.com/prometheus/client_golang/prometheus.(*counter).Collect
github.com/prometheus/client_golang/prometheus.(*counter).Desc
github.com/prometheus/client_golang/prometheus.(*counter).Describe
github.com/prometheus/client_golang/prometheus.(*counter).Inc
github.com/prometheus/client_golang/prometheus.(*counter).Write
github.com/prometheus/client_golang/prometheus.(*counter).updateExemplar
root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -x main -F github*counter*

Then trying to add for all those:

root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -d *:*
"*:*" does not hit any event.
  Error: Failed to delete events.
root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -l
root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#

root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -x main github*counter*
A function DIE doesn't have decl_line. Maybe broken DWARF?
A function DIE doesn't have decl_line. Maybe broken DWARF?
A function DIE doesn't have decl_line. Maybe broken DWARF?
A function DIE doesn't have decl_line. Maybe broken DWARF?
A function DIE doesn't have decl_line. Maybe broken DWARF?
snprintf() failed: -7; the event name 'github_com_prometheus_client_golang_prometheus_counter_AddWithExemplar' is too long
  Hint: Set a shorter event with syntax "EVENT=PROBEDEF"
        EVENT: Event name (max length: 64 bytes).
  Error: Failed to add events.
root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#

But:

root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -l
  probe_main:github_com_prometheus_client_golang_prometheus_counter_Desc (on github.com/prometheus/client_golang/prometheus.(*counter).Des>
(END)

That pager thing looks odd as well, since there is just one line in the
output...

So it failed to do all those, added just one, maybe state that some were
added but from the problematic one onwards it stopped? Or try all of
them and just state the ones that couldn't be added?

I.e. something like:

root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -x main -F github*counter* | while read probename ; do perf probe -x main $probename ; done
A function DIE doesn't have decl_line. Maybe broken DWARF?
A function DIE doesn't have decl_line. Maybe broken DWARF?
Failed to find debug information for address 0x3287e0
Probe point 'github.com/prometheus/client_golang/prometheus.(*counter).Add' not found.
  Error: Failed to add events.
snprintf() failed: -7; the event name 'github_com_prometheus_client_golang_prometheus_counter_AddWithExemplar' is too long
  Hint: Set a shorter event with syntax "EVENT=PROBEDEF"
        EVENT: Event name (max length: 64 bytes).
  Error: Failed to add events.
A function DIE doesn't have decl_line. Maybe broken DWARF?
Failed to find debug information for address 0x33ab40
Probe point 'github.com/prometheus/client_golang/prometheus.(*counter).Collect' not found.
  Error: Failed to add events.
Error: event "github_com_prometheus_client_golang_prometheus_counter_Desc" already exists.
 Hint: Remove existing event by 'perf probe -d'
       or force duplicates by 'perf probe -f'
       or set 'force=yes' in BPF source.
  Error: Failed to add events.
A function DIE doesn't have decl_line. Maybe broken DWARF?
Failed to find debug information for address 0x33aba0
Probe point 'github.com/prometheus/client_golang/prometheus.(*counter).Describe' not found.
  Error: Failed to add events.
Added new event:
  probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc (on github.com/prometheus/client_golang/prometheus.(*counter).Inc in /home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)

You can now use it in all perf tools, such as:

	perf record -e probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc -aR sleep 1

Added new event:
  probe_main:github_com_prometheus_client_golang_prometheus_counter_Write (on github.com/prometheus/client_golang/prometheus.(*counter).Write in /home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)

You can now use it in all perf tools, such as:

	perf record -e probe_main:github_com_prometheus_client_golang_prometheus_counter_Write -aR sleep 1

snprintf() failed: -7; the event name 'github_com_prometheus_client_golang_prometheus_counter_updateExemplar' is too long
  Hint: Set a shorter event with syntax "EVENT=PROBEDEF"
        EVENT: Event name (max length: 64 bytes).
  Error: Failed to add events.
root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#

In the end we get:

root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -l
  probe_main:github_com_prometheus_client_golang_prometheus_counter_Desc (on github.com/prometheus/client_golang/prometheus.(*counter).Desc@prometheus/counter.go in /home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
  probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc (on github.com/prometheus/client_golang/prometheus.(*counter).Inc@prometheus/counter.go in /home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
  probe_main:github_com_prometheus_client_golang_prometheus_counter_Write (on github.com/prometheus/client_golang/prometheus.(*counter).Write@prometheus/counter.go in /home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#

That also explains the pager kicking in: I had to reduce font size in my
xterm (gnome-terminal really) and then the perf pager wasn't used (no
(END) at the last line waiting for me to press 'q').

The ones that got installed are working:

root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf trace -e probe_main:*
     0.000 main/616840 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
  1001.043 main/616926 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
  1001.080 main/616926 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
  4000.994 main/616926 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
^Croot@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf trace -e probe_main:*/max-stack=8/
     0.000 main/616926 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
                                       github.com/prometheus/client_golang/prometheus.(*counter).Inc (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
                                       runtime.goexit.abi0 (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
     0.030 main/616926 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
                                       github.com/prometheus/client_golang/prometheus.(*counter).Inc (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
                                       runtime.goexit.abi0 (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
  3000.166 main/616840 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
                                       github.com/prometheus/client_golang/prometheus.(*counter).Inc (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
                                       runtime.goexit.abi0 (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
^Croot@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#

I'll test this some more later/tomorrow, just wanted to give this first
reaction, thanks for working on this!

Btw, some more info about the environment (fedora 40):

root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# readelf -wi main | head -20
Contents of the .debug_info section:

  Compilation Unit @ offset 0:
   Length:        0x506 (32-bit)
   Version:       4
   Abbrev Offset: 0
   Pointer Size:  8
 <0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
    <c>   DW_AT_name        : google.golang.org/protobuf/internal/strs
    <35>   DW_AT_language    : 22	(Go)
    <36>   DW_AT_stmt_list   : 0
    <3a>   DW_AT_low_pc      : 0x667c40
    <42>   DW_AT_ranges      : 0
    <46>   DW_AT_comp_dir    : .
    <48>   DW_AT_producer    : Go cmd/compile go1.22.7; regabi
    <68>   Unknown AT value: 2905: strs
 <1><6d>: Abbrev Number: 5 (DW_TAG_subprogram)
    <6e>   DW_AT_name        : google.golang.org/protobuf/internal/strs.isASCIILower
    <a4>   DW_AT_inline      : 1	(inlined)
    <a5>   DW_AT_decl_line   : 188
root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#

- Arnaldo
 
> And Rust stores
> -----
> $ ./perf probe -x /work/cro3/target/x86_64-unknown-linux-gnu/debug/cro3 -F cro3::cmd::servo*
> cro3::cmd::servo::run
> cro3::cmd::servo::run::CALLSITE
> cro3::cmd::servo::run::CALLSITE::META
> cro3::cmd::servo::run_control
> -----
> 
> These symbols are not parsed correctly because it looks like a file name or
> including line numbers (`:` caused it.) So, I decided to introduce the changes
> 
>  - filename MUST start from '@'. (so it is able to distinguish the filename
>    and the function name)
>  - Fix to allow backslash to escape to --lines option.
>  - Introduce quotation mark support.
>  - Replace non-alnum character to '_' for event name (for non-C symbols).
> 
> With these changes, we can run -L (--lines) on golang;
> 
> ------
> $ perf probe -x goexample/hello/hello -L \"main.main\"
> <main.main@/work/goexample/hello/hello.go:0>
>       0  func main() {
>                 // Configure logging for a command-line program.
>       2         log.SetFlags(0)
>       3         log.SetPrefix("hello: ")
> 
>                 // Parse flags.
>       6         flag.Usage = usage
>       7         flag.Parse()
> ------
> 
> And Rust
> ------
> $ perf probe -x cro3 -L \"cro3::cmd::servo::run_show\"
> <run_show@/work/cro3/src/cmd/servo.rs:0>
>       0  fn run_show(args: &ArgsShow) -> Result<()> {
>       1      let list = ServoList::discover()?;
>       2      let s = list.find_by_serial(&args.servo)?;
>       3      if args.json {
>       4          println!("{s}");
> ------
> 
> And event name are created automatically like below;
> 
> $ ./perf probe -x /work/go/example/outyet/main -D 'main.(*Server).poll'
> p:probe_main/main_Server_poll /work/go/example/outyet/main:0x353040
> 
> $ ./perf probe -x cro3 -D \"cro3::cmd::servo::run_show\"
> p:probe_cro3/cro3_cmd_servo_run_show /work/cro3/target/x86_64-unknown-linux-gnu/debug/cro3:0x197530
> 
> We still need some more work, but these shows how perf-probe can work
> with other languages.
> 
> Thank you,
> 
> ---
> 
> Masami Hiramatsu (Google) (4):
>       perf-probe: Fix to ignore escaped characters in --lines option
>       perf-probe: Require '@' prefix for filename always
>       perf-probe: Introduce quotation marks support
>       perf-probe: Replace unacceptable characters when generating event name
> 
> 
>  tools/perf/util/probe-event.c  |  136 ++++++++++++++++++++++------------------
>  tools/perf/util/probe-finder.c |    3 +
>  tools/perf/util/string.c       |  100 +++++++++++++++++++++++++++++
>  tools/perf/util/string2.h      |    2 +
>  4 files changed, 180 insertions(+), 61 deletions(-)
> 
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH 0/4] perf-probe: Improbe non-C language support

[Arnaldo Carvalho de Melo]

On Mon, Nov 04, 2024 at 03:56:00PM -0300, Arnaldo Carvalho de Melo wrote:
> On Tue, Nov 05, 2024 at 01:17:08AM +0900, Masami Hiramatsu (Google) wrote:
> > Hi,

I also now noticed that the cover letter subject has a typo, it should
be "Improve", not "Improbe" :-)

- Arnaldo
  
> > Here is a series of patches for perf probe to improve non-C language
> > (e.g. Rust, Go) support.
>  
> > The non-C symbols are demangled style in debuginfo, e.g. golang stores
>  
> > ----
> > $ ./perf probe -x /work/go/example/outyet/main -F main*
> > main.(*Server).ServeHTTP
> > main.(*Server).ServeHTTP.Print.func1
> > main.(*Server).poll
> > ...
> > -----
> 
> I presented about this last year:
> 
> https://tracingsummit.org/ts/2023/bpf-non-c/
> https://tracingsummit.org/ts/2023/files/Trying_to_use_uprobes_and_BPF_on_non-C_userspace.pdf
> https://www.youtube.com/watch?v=RDFRy1vWyHg&feature=youtu.be
> 
> So trying to do some of the things I did while playing with golang, and
> with your patches applied, I only had to cope with a minor clash with a
> patch by Ian Rogers that is present on perf-tools-next, related to:
> 
>         char buf[MAX_EVENT_NAME_LEN];
> 
> That in your patch expects the old 64 hard coded value, which will
> appear in the my tests:
> 
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -x main -F github*counter*
> github.com/prometheus/client_golang/prometheus.(*counter).Add
> github.com/prometheus/client_golang/prometheus.(*counter).AddWithExemplar
> github.com/prometheus/client_golang/prometheus.(*counter).Collect
> github.com/prometheus/client_golang/prometheus.(*counter).Desc
> github.com/prometheus/client_golang/prometheus.(*counter).Describe
> github.com/prometheus/client_golang/prometheus.(*counter).Inc
> github.com/prometheus/client_golang/prometheus.(*counter).Write
> github.com/prometheus/client_golang/prometheus.(*counter).updateExemplar
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -x main -F github*counter*
> 
> Then trying to add for all those:
> 
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -d *:*
> "*:*" does not hit any event.
>   Error: Failed to delete events.
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -l
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#
> 
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -x main github*counter*
> A function DIE doesn't have decl_line. Maybe broken DWARF?
> A function DIE doesn't have decl_line. Maybe broken DWARF?
> A function DIE doesn't have decl_line. Maybe broken DWARF?
> A function DIE doesn't have decl_line. Maybe broken DWARF?
> A function DIE doesn't have decl_line. Maybe broken DWARF?
> snprintf() failed: -7; the event name 'github_com_prometheus_client_golang_prometheus_counter_AddWithExemplar' is too long
>   Hint: Set a shorter event with syntax "EVENT=PROBEDEF"
>         EVENT: Event name (max length: 64 bytes).
>   Error: Failed to add events.
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#
> 
> But:
> 
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -l
>   probe_main:github_com_prometheus_client_golang_prometheus_counter_Desc (on github.com/prometheus/client_golang/prometheus.(*counter).Des>
> (END)
> 
> That pager thing looks odd as well, since there is just one line in the
> output...
> 
> So it failed to do all those, added just one, maybe state that some were
> added but from the problematic one onwards it stopped? Or try all of
> them and just state the ones that couldn't be added?
> 
> I.e. something like:
> 
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -x main -F github*counter* | while read probename ; do perf probe -x main $probename ; done
> A function DIE doesn't have decl_line. Maybe broken DWARF?
> A function DIE doesn't have decl_line. Maybe broken DWARF?
> Failed to find debug information for address 0x3287e0
> Probe point 'github.com/prometheus/client_golang/prometheus.(*counter).Add' not found.
>   Error: Failed to add events.
> snprintf() failed: -7; the event name 'github_com_prometheus_client_golang_prometheus_counter_AddWithExemplar' is too long
>   Hint: Set a shorter event with syntax "EVENT=PROBEDEF"
>         EVENT: Event name (max length: 64 bytes).
>   Error: Failed to add events.
> A function DIE doesn't have decl_line. Maybe broken DWARF?
> Failed to find debug information for address 0x33ab40
> Probe point 'github.com/prometheus/client_golang/prometheus.(*counter).Collect' not found.
>   Error: Failed to add events.
> Error: event "github_com_prometheus_client_golang_prometheus_counter_Desc" already exists.
>  Hint: Remove existing event by 'perf probe -d'
>        or force duplicates by 'perf probe -f'
>        or set 'force=yes' in BPF source.
>   Error: Failed to add events.
> A function DIE doesn't have decl_line. Maybe broken DWARF?
> Failed to find debug information for address 0x33aba0
> Probe point 'github.com/prometheus/client_golang/prometheus.(*counter).Describe' not found.
>   Error: Failed to add events.
> Added new event:
>   probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc (on github.com/prometheus/client_golang/prometheus.(*counter).Inc in /home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
> 
> You can now use it in all perf tools, such as:
> 
> 	perf record -e probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc -aR sleep 1
> 
> Added new event:
>   probe_main:github_com_prometheus_client_golang_prometheus_counter_Write (on github.com/prometheus/client_golang/prometheus.(*counter).Write in /home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
> 
> You can now use it in all perf tools, such as:
> 
> 	perf record -e probe_main:github_com_prometheus_client_golang_prometheus_counter_Write -aR sleep 1
> 
> snprintf() failed: -7; the event name 'github_com_prometheus_client_golang_prometheus_counter_updateExemplar' is too long
>   Hint: Set a shorter event with syntax "EVENT=PROBEDEF"
>         EVENT: Event name (max length: 64 bytes).
>   Error: Failed to add events.
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#
> 
> In the end we get:
> 
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -l
>   probe_main:github_com_prometheus_client_golang_prometheus_counter_Desc (on github.com/prometheus/client_golang/prometheus.(*counter).Desc@prometheus/counter.go in /home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
>   probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc (on github.com/prometheus/client_golang/prometheus.(*counter).Inc@prometheus/counter.go in /home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
>   probe_main:github_com_prometheus_client_golang_prometheus_counter_Write (on github.com/prometheus/client_golang/prometheus.(*counter).Write@prometheus/counter.go in /home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#
> 
> That also explains the pager kicking in: I had to reduce font size in my
> xterm (gnome-terminal really) and then the perf pager wasn't used (no
> (END) at the last line waiting for me to press 'q').
> 
> The ones that got installed are working:
> 
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf trace -e probe_main:*
>      0.000 main/616840 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
>   1001.043 main/616926 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
>   1001.080 main/616926 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
>   4000.994 main/616926 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
> ^Croot@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf trace -e probe_main:*/max-stack=8/
>      0.000 main/616926 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
>                                        github.com/prometheus/client_golang/prometheus.(*counter).Inc (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
>                                        runtime.goexit.abi0 (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
>      0.030 main/616926 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
>                                        github.com/prometheus/client_golang/prometheus.(*counter).Inc (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
>                                        runtime.goexit.abi0 (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
>   3000.166 main/616840 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
>                                        github.com/prometheus/client_golang/prometheus.(*counter).Inc (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
>                                        runtime.goexit.abi0 (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
> ^Croot@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#
> 
> I'll test this some more later/tomorrow, just wanted to give this first
> reaction, thanks for working on this!
> 
> Btw, some more info about the environment (fedora 40):
> 
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# readelf -wi main | head -20
> Contents of the .debug_info section:
> 
>   Compilation Unit @ offset 0:
>    Length:        0x506 (32-bit)
>    Version:       4
>    Abbrev Offset: 0
>    Pointer Size:  8
>  <0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
>     <c>   DW_AT_name        : google.golang.org/protobuf/internal/strs
>     <35>   DW_AT_language    : 22	(Go)
>     <36>   DW_AT_stmt_list   : 0
>     <3a>   DW_AT_low_pc      : 0x667c40
>     <42>   DW_AT_ranges      : 0
>     <46>   DW_AT_comp_dir    : .
>     <48>   DW_AT_producer    : Go cmd/compile go1.22.7; regabi
>     <68>   Unknown AT value: 2905: strs
>  <1><6d>: Abbrev Number: 5 (DW_TAG_subprogram)
>     <6e>   DW_AT_name        : google.golang.org/protobuf/internal/strs.isASCIILower
>     <a4>   DW_AT_inline      : 1	(inlined)
>     <a5>   DW_AT_decl_line   : 188
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#
> 
> - Arnaldo
>  
> > And Rust stores
> > -----
> > $ ./perf probe -x /work/cro3/target/x86_64-unknown-linux-gnu/debug/cro3 -F cro3::cmd::servo*
> > cro3::cmd::servo::run
> > cro3::cmd::servo::run::CALLSITE
> > cro3::cmd::servo::run::CALLSITE::META
> > cro3::cmd::servo::run_control
> > -----
> > 
> > These symbols are not parsed correctly because it looks like a file name or
> > including line numbers (`:` caused it.) So, I decided to introduce the changes
> > 
> >  - filename MUST start from '@'. (so it is able to distinguish the filename
> >    and the function name)
> >  - Fix to allow backslash to escape to --lines option.
> >  - Introduce quotation mark support.
> >  - Replace non-alnum character to '_' for event name (for non-C symbols).
> > 
> > With these changes, we can run -L (--lines) on golang;
> > 
> > ------
> > $ perf probe -x goexample/hello/hello -L \"main.main\"
> > <main.main@/work/goexample/hello/hello.go:0>
> >       0  func main() {
> >                 // Configure logging for a command-line program.
> >       2         log.SetFlags(0)
> >       3         log.SetPrefix("hello: ")
> > 
> >                 // Parse flags.
> >       6         flag.Usage = usage
> >       7         flag.Parse()
> > ------
> > 
> > And Rust
> > ------
> > $ perf probe -x cro3 -L \"cro3::cmd::servo::run_show\"
> > <run_show@/work/cro3/src/cmd/servo.rs:0>
> >       0  fn run_show(args: &ArgsShow) -> Result<()> {
> >       1      let list = ServoList::discover()?;
> >       2      let s = list.find_by_serial(&args.servo)?;
> >       3      if args.json {
> >       4          println!("{s}");
> > ------
> > 
> > And event name are created automatically like below;
> > 
> > $ ./perf probe -x /work/go/example/outyet/main -D 'main.(*Server).poll'
> > p:probe_main/main_Server_poll /work/go/example/outyet/main:0x353040
> > 
> > $ ./perf probe -x cro3 -D \"cro3::cmd::servo::run_show\"
> > p:probe_cro3/cro3_cmd_servo_run_show /work/cro3/target/x86_64-unknown-linux-gnu/debug/cro3:0x197530
> > 
> > We still need some more work, but these shows how perf-probe can work
> > with other languages.
> > 
> > Thank you,
> > 
> > ---
> > 
> > Masami Hiramatsu (Google) (4):
> >       perf-probe: Fix to ignore escaped characters in --lines option
> >       perf-probe: Require '@' prefix for filename always
> >       perf-probe: Introduce quotation marks support
> >       perf-probe: Replace unacceptable characters when generating event name
> > 
> > 
> >  tools/perf/util/probe-event.c  |  136 ++++++++++++++++++++++------------------
> >  tools/perf/util/probe-finder.c |    3 +
> >  tools/perf/util/string.c       |  100 +++++++++++++++++++++++++++++
> >  tools/perf/util/string2.h      |    2 +
> >  4 files changed, 180 insertions(+), 61 deletions(-)
> > 
> > --
> > Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH 2/4] perf-probe: Require '@' prefix for filename always

[Arnaldo Carvalho de Melo]

On Tue, Nov 05, 2024 at 01:17:26AM +0900, Masami Hiramatsu (Google) wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
 
> Currently perf probe allows user to specify probing place without '@'
> prefix, for example, both `-V file:line` and `-V function:line` are
> allowed. But this makes a problem if a (demangled) function name is
> hard to be distinguished from a file name.
 
> This changes the perf-probe to require '@' prefix for filename even
> without function name. For example, `-V @file:line` and
> `-V function:line` are acceptable.
 
> With this change, users can specify filename or function correctly.

Well, this will break scripts that use perf probe for a given file,
probably the right thing not to break backwards compatibility is to
continue accepting and when there is a real conflict, an ambiguity that
makes differentiating from file to function names, then refuse it,
stating that it is ambiguous, probably spelling out the names of the
files and functions that match so and stating that to make it
unambiguoius, prefix file names with @.

- Arnaldo
 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  tools/perf/util/probe-event.c |   31 +++++++++----------------------
>  1 file changed, 9 insertions(+), 22 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 665dcce482e1..913a27cbb5d9 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -1341,7 +1341,7 @@ static bool is_c_func_name(const char *name)
>   * Stuff 'lr' according to the line range described by 'arg'.
>   * The line range syntax is described by:
>   *
> - *         SRC[:SLN[+NUM|-ELN]]
> + *         @SRC[:SLN[+NUM|-ELN]]
>   *         FNC[@SRC][:SLN[+NUM|-ELN]]
>   */
>  int parse_line_range_desc(const char *arg, struct line_range *lr)
> @@ -1404,16 +1404,10 @@ int parse_line_range_desc(const char *arg, struct line_range *lr)
>  			err = -ENOMEM;
>  			goto err;
>  		}
> +		if (*name != '\0')
> +			lr->function = name;
> +	} else
>  		lr->function = name;
> -	} else if (strpbrk_esc(name, "/."))
> -		lr->file = name;
> -	else if (is_c_func_name(name))/* We reuse it for checking funcname */
> -		lr->function = name;
> -	else {	/* Invalid name */
> -		semantic_error("'%s' is not a valid function name.\n", name);
> -		err = -EINVAL;
> -		goto err;
> -	}
>  
>  	return 0;
>  err:
> @@ -1463,7 +1457,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  
>  	/*
>  	 * <Syntax>
> -	 * perf probe [GRP:][EVENT=]SRC[:LN|;PTN]
> +	 * perf probe [GRP:][EVENT=]@SRC[:LN|;PTN]
>  	 * perf probe [GRP:][EVENT=]FUNC[@SRC][+OFFS|%return|:LN|;PAT]
>  	 * perf probe %[GRP:]SDT_EVENT
>  	 */
> @@ -1516,19 +1510,12 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  	/*
>  	 * Check arg is function or file name and copy it.
>  	 *
> -	 * We consider arg to be a file spec if and only if it satisfies
> -	 * all of the below criteria::
> -	 * - it does not include any of "+@%",
> -	 * - it includes one of ":;", and
> -	 * - it has a period '.' in the name.
> -	 *
> +	 * We consider arg to be a file spec if it starts with '@'.
>  	 * Otherwise, we consider arg to be a function specification.
>  	 */
> -	if (!strpbrk_esc(arg, "+@%")) {
> -		ptr = strpbrk_esc(arg, ";:");
> -		/* This is a file spec if it includes a '.' before ; or : */
> -		if (ptr && memchr(arg, '.', ptr - arg))
> -			file_spec = true;
> +	if (*arg == '@') {
> +		file_spec = true;
> +		arg++;
>  	}
>  
>  	ptr = strpbrk_esc(arg, ";:+@%");

Re: [PATCH 3/4] perf-probe: Introduce quotation marks support

[Arnaldo Carvalho de Melo]

On Tue, Nov 05, 2024 at 01:17:35AM +0900, Masami Hiramatsu (Google) wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> In non-C languages, it is possible to have ':' in the function names.
> It is possible to escape it with backslashes, but if there are too many
> backslashes, it is annoying.
> This introduce quotation marks (`"` or `'`) support.

Can you please split the patch so that strpbrk_esq() and strdup_esq()
are introduced in different patches?

- Arnaldo
 
> For example, without quotes, we have to pass it as below
> 
> $ perf probe -x cro3 -L "cro3\:\:cmd\:\:servo\:\:run_show"
> <run_show@/work/cro3/src/cmd/servo.rs:0>
>       0  fn run_show(args: &ArgsShow) -> Result<()> {
>       1      let list = ServoList::discover()?;
>       2      let s = list.find_by_serial(&args.servo)?;
>       3      if args.json {
>       4          println!("{s}");
> 
> With quotes, we can more naturally write the function name as below;
> 
> $ ./perf probe -x cro3 -L \"cro3::cmd::servo::run_show\"
> <run_show@/work/cro3/src/cmd/servo.rs:0>
>       0  fn run_show(args: &ArgsShow) -> Result<()> {
>       1      let list = ServoList::discover()?;
>       2      let s = list.find_by_serial(&args.servo)?;
>       3      if args.json {
>       4          println!("{s}");
> 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  tools/perf/util/probe-event.c  |   76 ++++++++++++++++--------------
>  tools/perf/util/probe-finder.c |    3 +
>  tools/perf/util/string.c       |  100 ++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/string2.h      |    2 +
>  4 files changed, 145 insertions(+), 36 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 913a27cbb5d9..bcba8273204d 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -1065,6 +1065,7 @@ static int __show_line_range(struct line_range *lr, const char *module,
>  
>  	ret = debuginfo__find_line_range(dinfo, lr);
>  	if (!ret) {	/* Not found, retry with an alternative */
> +		pr_debug2("Failed to find line range in debuginfo. Fallback to alternative\n");
>  		ret = get_alternative_line_range(dinfo, lr, module, user);
>  		if (!ret)
>  			ret = debuginfo__find_line_range(dinfo, lr);
> @@ -1078,7 +1079,7 @@ static int __show_line_range(struct line_range *lr, const char *module,
>  		pr_warning("Specified source line is not found.\n");
>  		return -ENOENT;
>  	} else if (ret < 0) {
> -		pr_warning("Debuginfo analysis failed.\n");
> +		pr_warning("Debuginfo analysis failed (%d).\n", ret);
>  		return ret;
>  	}
>  
> @@ -1187,7 +1188,7 @@ static int show_available_vars_at(struct debuginfo *dinfo,
>  			pr_err("Failed to find the address of %s\n", buf);
>  			ret = -ENOENT;
>  		} else
> -			pr_warning("Debuginfo analysis failed.\n");
> +			pr_warning("Debuginfo analysis failed(2).\n");
>  		goto end;
>  	}
>  
> @@ -1343,30 +1344,39 @@ static bool is_c_func_name(const char *name)
>   *
>   *         @SRC[:SLN[+NUM|-ELN]]
>   *         FNC[@SRC][:SLN[+NUM|-ELN]]
> + *
> + * SRC and FUNC can be quoted by double/single quotes.
>   */
>  int parse_line_range_desc(const char *arg, struct line_range *lr)
>  {
> -	char *range, *file, *name = strdup(arg);
> +	char *buf = strdup(arg);
> +	char *p;
>  	int err;
>  
> -	if (!name)
> +	if (!buf)
>  		return -ENOMEM;
>  
>  	lr->start = 0;
>  	lr->end = INT_MAX;
>  
> -	range = strpbrk_esc(name, ":");
> -	if (range) {
> -		*range++ = '\0';
> +	pr_debug2("Input line range: %s\n", buf);
> +	p = strpbrk_esq(buf, ":");
> +	if (p) {
> +		if (p == buf) {
> +			semantic_error("No file/function name in '%s'.\n", p);
> +			err = -EINVAL;
> +			goto err;
> +		}
> +		*(p++) = '\0';
>  
> -		err = parse_line_num(&range, &lr->start, "start line");
> +		err = parse_line_num(&p, &lr->start, "start line");
>  		if (err)
>  			goto err;
>  
> -		if (*range == '+' || *range == '-') {
> -			const char c = *range++;
> +		if (*p == '+' || *p == '-') {
> +			const char c = *(p++);
>  
> -			err = parse_line_num(&range, &lr->end, "end line");
> +			err = parse_line_num(&p, &lr->end, "end line");
>  			if (err)
>  				goto err;
>  
> @@ -1390,28 +1400,22 @@ int parse_line_range_desc(const char *arg, struct line_range *lr)
>  				       " than end line.\n");
>  			goto err;
>  		}
> -		if (*range != '\0') {
> -			semantic_error("Tailing with invalid str '%s'.\n", range);
> +		if (*p != '\0') {
> +			semantic_error("Tailing with invalid str '%s'.\n", p);
>  			goto err;
>  		}
>  	}
>  
> -	file = strpbrk_esc(name, "@");
> -	if (file) {
> -		*file = '\0';
> -		lr->file = strdup_esc(++file);
> -		if (lr->file == NULL) {
> -			err = -ENOMEM;
> -			goto err;
> -		}
> -		if (*name != '\0')
> -			lr->function = name;
> -	} else
> -		lr->function = name;
> +	p = strpbrk_esq(buf, "@");
> +	if (p) {
> +		*(p++) = '\0';
> +		lr->file = strdup_esq(p);
> +	}
> +	lr->function = strdup_esq(buf);
> +	err = 0;
>  
> -	return 0;
>  err:
> -	free(name);
> +	free(buf);
>  	return err;
>  }
>  
> @@ -1419,19 +1423,19 @@ static int parse_perf_probe_event_name(char **arg, struct perf_probe_event *pev)
>  {
>  	char *ptr;
>  
> -	ptr = strpbrk_esc(*arg, ":");
> +	ptr = strpbrk_esq(*arg, ":");
>  	if (ptr) {
>  		*ptr = '\0';
>  		if (!pev->sdt && !is_c_func_name(*arg))
>  			goto ng_name;
> -		pev->group = strdup_esc(*arg);
> +		pev->group = strdup_esq(*arg);
>  		if (!pev->group)
>  			return -ENOMEM;
>  		*arg = ptr + 1;
>  	} else
>  		pev->group = NULL;
>  
> -	pev->event = strdup_esc(*arg);
> +	pev->event = strdup_esq(*arg);
>  	if (pev->event == NULL)
>  		return -ENOMEM;
>  
> @@ -1470,7 +1474,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  			arg++;
>  	}
>  
> -	ptr = strpbrk_esc(arg, ";=@+%");
> +	ptr = strpbrk_esq(arg, ";=@+%");
>  	if (pev->sdt) {
>  		if (ptr) {
>  			if (*ptr != '@') {
> @@ -1484,7 +1488,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  				pev->target = build_id_cache__origname(tmp);
>  				free(tmp);
>  			} else
> -				pev->target = strdup_esc(ptr + 1);
> +				pev->target = strdup_esq(ptr + 1);
>  			if (!pev->target)
>  				return -ENOMEM;
>  			*ptr = '\0';
> @@ -1518,7 +1522,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  		arg++;
>  	}
>  
> -	ptr = strpbrk_esc(arg, ";:+@%");
> +	ptr = strpbrk_esq(arg, ";:+@%");
>  	if (ptr) {
>  		nc = *ptr;
>  		*ptr++ = '\0';
> @@ -1527,7 +1531,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  	if (arg[0] == '\0')
>  		tmp = NULL;
>  	else {
> -		tmp = strdup_esc(arg);
> +		tmp = strdup_esq(arg);
>  		if (tmp == NULL)
>  			return -ENOMEM;
>  	}
> @@ -1565,7 +1569,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  				return -ENOMEM;
>  			break;
>  		}
> -		ptr = strpbrk_esc(arg, ";:+@%");
> +		ptr = strpbrk_esq(arg, ";:+@%");
>  		if (ptr) {
>  			nc = *ptr;
>  			*ptr++ = '\0';
> @@ -1592,7 +1596,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
>  				semantic_error("SRC@SRC is not allowed.\n");
>  				return -EINVAL;
>  			}
> -			pp->file = strdup_esc(arg);
> +			pp->file = strdup_esq(arg);
>  			if (pp->file == NULL)
>  				return -ENOMEM;
>  			break;
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 630e16c54ed5..5462b5541a6d 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -1796,6 +1796,7 @@ int debuginfo__find_line_range(struct debuginfo *dbg, struct line_range *lr)
>  		struct dwarf_callback_param line_range_param = {
>  			.data = (void *)&lf, .retval = 0};
>  
> +		pr_debug2("Start searching function '%s' in getpubname\n", lr->function);
>  		dwarf_getpubnames(dbg->dbg, pubname_search_cb,
>  				  &pubname_param, 0);
>  		if (pubname_param.found) {
> @@ -1803,8 +1804,10 @@ int debuginfo__find_line_range(struct debuginfo *dbg, struct line_range *lr)
>  			if (lf.found)
>  				goto found;
>  		}
> +		pr_debug2("Not found, use DIE tree\n");
>  	}
>  
> +	pr_debug2("Search function '%s' in DIE tree\n", lr->function);
>  	/* Loop on CUs (Compilation Unit) */
>  	while (!lf.found && ret >= 0) {
>  		if (dwarf_nextcu(dbg->dbg, off, &noff, &cuhl,
> diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
> index 116a642ad99d..308fc7ec88cc 100644
> --- a/tools/perf/util/string.c
> +++ b/tools/perf/util/string.c
> @@ -263,6 +263,34 @@ char *strpbrk_esc(char *str, const char *stopset)
>  	return ptr;
>  }
>  
> +/* Like strpbrk_esc(), but not break if it is quoted with single/double quotes */
> +char *strpbrk_esq(char *str, const char *stopset)
> +{
> +	char *_stopset = NULL;
> +	char *ptr;
> +	const char *squote = "'";
> +	const char *dquote = "\"";
> +
> +	if (asprintf(&_stopset, "%s%c%c", stopset, *squote, *dquote) < 0)
> +		return NULL;
> +
> +	do {
> +		ptr = strpbrk_esc(str, _stopset);
> +		if (!ptr)
> +			break;
> +		if (*ptr == *squote)
> +			ptr = strpbrk_esc(ptr + 1, squote);
> +		else if (*ptr == *dquote)
> +			ptr = strpbrk_esc(ptr + 1, dquote);
> +		else
> +			break;
> +		str = ptr + 1;
> +	} while (ptr);
> +
> +	free(_stopset);
> +	return ptr;
> +}
> +
>  /* Like strdup, but do not copy a single backslash */
>  char *strdup_esc(const char *str)
>  {
> @@ -293,6 +321,78 @@ char *strdup_esc(const char *str)
>  	return ret;
>  }
>  
> +/* Remove backslash right before quote and return next quote address. */
> +static char *remove_consumed_esc(char *str, int len, int quote)
> +{
> +	char *ptr = str, *end = str + len;
> +
> +	while (*ptr != quote && ptr < end) {
> +		if (*ptr == '\\' && *(ptr + 1) == quote) {
> +			memmove(ptr, ptr + 1, end - (ptr + 1));
> +			/* now *ptr is `quote`. */
> +			end--;
> +		}
> +		ptr++;
> +	}
> +
> +	return *ptr == quote ? ptr : NULL;
> +}
> +
> +/*
> + * Like strdup_esc, but keep quoted string as it is (and single backslash
> + * before quote is removed). If there is no closed quote, return NULL.
> + */
> +char *strdup_esq(const char *str)
> +{
> +	char *d, *ret;
> +
> +	/* If there is no quote, return normal strdup_esc() */
> +	d = strpbrk_esc((char *)str, "\"'");
> +	if (!d)
> +		return strdup_esc(str);
> +
> +	ret = strdup(str);
> +	if (!ret)
> +		return NULL;
> +
> +	d = ret;
> +	do {
> +		d = strpbrk(d, "\\\"\'");
> +		if (!d)
> +			break;
> +
> +		if (*d == '"' || *d == '\'') {
> +			/* This is non-escaped quote */
> +			int quote = *d;
> +			int len = strlen(d + 1) + 1;
> +
> +			/*
> +			 * Remove the start quote and remove consumed escape (backslash
> +			 * before quote) and remove the end quote. If there is no end
> +			 * quote, it is the input error.
> +			 */
> +			memmove(d, d + 1, len);
> +			d = remove_consumed_esc(d, len, quote);
> +			if (!d)
> +				goto error;
> +			memmove(d, d + 1, strlen(d + 1) + 1);
> +		}
> +		if (*d == '\\') {
> +			memmove(d, d + 1, strlen(d + 1) + 1);
> +			if (*d == '\\') {
> +				/* double backslash -- keep the second one. */
> +				d++;
> +			}
> +		}
> +	} while (*d != '\0');
> +
> +	return ret;
> +
> +error:
> +	free(ret);
> +	return NULL;
> +}
> +
>  unsigned int hex(char c)
>  {
>  	if (c >= '0' && c <= '9')
> diff --git a/tools/perf/util/string2.h b/tools/perf/util/string2.h
> index 52cb8ba057c7..4c8bff47cfd3 100644
> --- a/tools/perf/util/string2.h
> +++ b/tools/perf/util/string2.h
> @@ -37,6 +37,8 @@ char *asprintf__tp_filter_pids(size_t npids, pid_t *pids);
>  
>  char *strpbrk_esc(char *str, const char *stopset);
>  char *strdup_esc(const char *str);
> +char *strpbrk_esq(char *str, const char *stopset);
> +char *strdup_esq(const char *str);
>  
>  unsigned int hex(char c);
>  char *strreplace_chars(char needle, const char *haystack, const char *replace);

Re: [PATCH 4/4] perf-probe: Replace unacceptable characters when generating event name

[Arnaldo Carvalho de Melo]

On Tue, Nov 05, 2024 at 01:17:44AM +0900, Masami Hiramatsu (Google) wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> Replace unacceptable characters with '_' when generating event name from
> the probing function name. This is not for C program. For the C program,
> it will continue to remove suffixes.
> 
> For example.
> 
> $ ./perf probe -x cro3 -D \"cro3::cmd::servo::run_show\"
> p:probe_cro3/cro3_cmd_servo_run_show /work/cro3/target/x86_64-unknown-linux-gnu/debug/cro3:0x197530
> 
> $ ./perf probe -x /work/go/example/outyet/main -D 'main.(*Server).poll'
> p:probe_main/main_Server_poll /work/go/example/outyet/main:0x353040
> 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  tools/perf/util/probe-event.c |   37 ++++++++++++++++++++++++++++++-------
>  1 file changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index bcba8273204d..27698b9a8c95 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2729,7 +2729,7 @@ int show_perf_probe_events(struct strfilter *filter)
>  
>  static int get_new_event_name(char *buf, size_t len, const char *base,
>  			      struct strlist *namelist, bool ret_event,
> -			      bool allow_suffix)
> +			      bool allow_suffix, bool is_C_symname)
>  {
>  	int i, ret;
>  	char *p, *nbase;
> @@ -2740,10 +2740,24 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
>  	if (!nbase)
>  		return -ENOMEM;
>  
> -	/* Cut off the dot suffixes (e.g. .const, .isra) and version suffixes */
> -	p = strpbrk(nbase, ".@");
> -	if (p && p != nbase)
> -		*p = '\0';
> +	if (is_C_symname) {
> +		/* Cut off the dot suffixes (e.g. .const, .isra) and version suffixes */
> +		p = strpbrk(nbase, ".@");
> +		if (p && p != nbase)
> +			*p = '\0';
> +	} else {
> +		/* Replace non-alnum with '_' */
> +		char *s, *d;
> +
> +		s = d = nbase;
> +		do {
> +			if (*s && !isalnum(*s)) {
> +				if (d != nbase && *(d - 1) != '_')
> +					*d++ = '_';
> +			} else
> +				*d++ = *s;
> +		} while (*s++);
> +	}
>  
>  	/* Try no suffix number */
>  	ret = e_snprintf(buf, len, "%s%s", nbase, ret_event ? "__return" : "");
> @@ -2832,6 +2846,7 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
>  				       bool allow_suffix)
>  {
>  	const char *event, *group;
> +	bool is_C_symname = false;
>  	char buf[64];
>  	int ret;
>  
> @@ -2846,8 +2861,16 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
>  			(strncmp(pev->point.function, "0x", 2) != 0) &&
>  			!strisglob(pev->point.function))
>  			event = pev->point.function;
> -		else
> +		else {
>  			event = tev->point.realname;
> +			/*
> +			 * `realname` comes from real symbol, which can have a suffix.
> +			 * However, if we see some glab-like wildcard in the symbol, it
                                                   "glob"?
> +			 * might not be a C program.
> +			 */
> +			if (!strisglob(event))
> +				is_C_symname = true;

				non_C_symname would be a bit more clear,
                                i.e. inverting the logic, as a C symname is
				valid in other languages :-)

				Also since we _can_ know if it comes
from a C, as we have:

root@x1:~# readelf -wi /usr/lib/debug/lib/modules/6.11.4-201.fc40.x86_64/vmlinux | grep -m10 DW_AT_language
    <12>   DW_AT_language    : 29	(C11)
    <2b0a5>   DW_AT_language    : 29	(C11)
    <2f3a4>   DW_AT_language    : 29	(C11)
    <573b0>   DW_AT_language    : 29	(C11)
    <6dd73>   DW_AT_language    : 29	(C11)
    <879eb>   DW_AT_language    : 29	(C11)
    <8c094>   DW_AT_language    : 29	(C11)
    <9ce09>   DW_AT_language    : 29	(C11)
    <9d8fb>   DW_AT_language    : 29	(C11)
    <b877a>   DW_AT_language    : 29	(C11)
root@x1:~# 
root@x1:~# readelf -wi /usr/lib/debug/lib/modules/6.11.4-201.fc40.x86_64/vmlinux | grep -m1 -B5 DW_AT_language
   Unit Type:     DW_UT_compile (1)
   Abbrev Offset: 0
   Pointer Size:  8
 <0><c>: Abbrev Number: 246 (DW_TAG_compile_unit)
    <e>   DW_AT_producer    : (indirect string, offset: 0x4edb56): GNU C11 14.2.1 20240912 (Red Hat 14.2.1-3) -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx -m64 -mno-80387 -mno-fp-ret-in-387 -mpreferred-stack-boundary=3 -mskip-rax-setup -mtune=generic -mno-red-zone -mcmodel=kernel -mindirect-branch=thunk-extern -mindirect-branch-register -mindirect-branch-cs-prefix -mfunction-return=thunk-extern -mharden-sls=all -mrecord-mcount -mfentry -march=x86-64 -g -O2 -std=gnu11 -p -fshort-wchar -funsigned-char -fno-common -fno-PIE -fno-strict-aliasing -fcf-protection=branch -falign-jumps=1 -falign-loops=1 -fno-asynchronous-unwind-tables -fno-jump-tables -fpatchable-function-entry=16,16 -fno-delete-null-pointer-checks -fno-allow-store-data-races -fstack-protector-strong -ftrivial-auto-var-init=zero -fno-stack-clash-protection -fmin-function-alignment=16 -fstrict-flex-arrays=3 -fno-strict-overflow -fstack-check=no -fconserve-stack -fno-function-sections -fno-data-sections -fsanitize=bounds-strict -fsanitize=shift
    <12>   DW_AT_language    : 29	(C11)
root@x1:~#

	Wouldn't it be better to use that to decide how to deal with
symbols in a CU?

	The advantage of using just the symbol name and that heuristic
about finding glob characters in the symbols is when we don't have
access to the DWARF info, having just the ELF symbol table, when we
don't have the lang code for the CU.

- Arnaldo

> +		}
>  	}
>  	if (pev->group && !pev->sdt)
>  		group = pev->group;
> @@ -2858,7 +2881,7 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
>  
>  	/* Get an unused new event name */
>  	ret = get_new_event_name(buf, sizeof(buf), event, namelist,
> -				 tev->point.retprobe, allow_suffix);
> +				 tev->point.retprobe, allow_suffix, is_C_symname);
>  	if (ret < 0)
>  		return ret;
>  

Re: [PATCH 4/4] perf-probe: Replace unacceptable characters when generating event name

[Masami Hiramatsu]

On Mon, 4 Nov 2024 17:34:32 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> On Tue, Nov 05, 2024 at 01:17:44AM +0900, Masami Hiramatsu (Google) wrote:
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > 
> > Replace unacceptable characters with '_' when generating event name from
> > the probing function name. This is not for C program. For the C program,
> > it will continue to remove suffixes.
> > 
> > For example.
> > 
> > $ ./perf probe -x cro3 -D \"cro3::cmd::servo::run_show\"
> > p:probe_cro3/cro3_cmd_servo_run_show /work/cro3/target/x86_64-unknown-linux-gnu/debug/cro3:0x197530
> > 
> > $ ./perf probe -x /work/go/example/outyet/main -D 'main.(*Server).poll'
> > p:probe_main/main_Server_poll /work/go/example/outyet/main:0x353040
> > 
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > ---
> >  tools/perf/util/probe-event.c |   37 ++++++++++++++++++++++++++++++-------
> >  1 file changed, 30 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index bcba8273204d..27698b9a8c95 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -2729,7 +2729,7 @@ int show_perf_probe_events(struct strfilter *filter)
> >  
> >  static int get_new_event_name(char *buf, size_t len, const char *base,
> >  			      struct strlist *namelist, bool ret_event,
> > -			      bool allow_suffix)
> > +			      bool allow_suffix, bool is_C_symname)
> >  {
> >  	int i, ret;
> >  	char *p, *nbase;
> > @@ -2740,10 +2740,24 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
> >  	if (!nbase)
> >  		return -ENOMEM;
> >  
> > -	/* Cut off the dot suffixes (e.g. .const, .isra) and version suffixes */
> > -	p = strpbrk(nbase, ".@");
> > -	if (p && p != nbase)
> > -		*p = '\0';
> > +	if (is_C_symname) {
> > +		/* Cut off the dot suffixes (e.g. .const, .isra) and version suffixes */
> > +		p = strpbrk(nbase, ".@");
> > +		if (p && p != nbase)
> > +			*p = '\0';
> > +	} else {
> > +		/* Replace non-alnum with '_' */
> > +		char *s, *d;
> > +
> > +		s = d = nbase;
> > +		do {
> > +			if (*s && !isalnum(*s)) {
> > +				if (d != nbase && *(d - 1) != '_')
> > +					*d++ = '_';
> > +			} else
> > +				*d++ = *s;
> > +		} while (*s++);
> > +	}
> >  
> >  	/* Try no suffix number */
> >  	ret = e_snprintf(buf, len, "%s%s", nbase, ret_event ? "__return" : "");
> > @@ -2832,6 +2846,7 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
> >  				       bool allow_suffix)
> >  {
> >  	const char *event, *group;
> > +	bool is_C_symname = false;
> >  	char buf[64];
> >  	int ret;
> >  
> > @@ -2846,8 +2861,16 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
> >  			(strncmp(pev->point.function, "0x", 2) != 0) &&
> >  			!strisglob(pev->point.function))
> >  			event = pev->point.function;
> > -		else
> > +		else {
> >  			event = tev->point.realname;
> > +			/*
> > +			 * `realname` comes from real symbol, which can have a suffix.
> > +			 * However, if we see some glab-like wildcard in the symbol, it
>                                                    "glob"?
> > +			 * might not be a C program.
> > +			 */
> > +			if (!strisglob(event))
> > +				is_C_symname = true;
> 
> 				non_C_symname would be a bit more clear,
>                                 i.e. inverting the logic, as a C symname is
> 				valid in other languages :-)

OK.

> 
> 				Also since we _can_ know if it comes
> from a C, as we have:
> 
> root@x1:~# readelf -wi /usr/lib/debug/lib/modules/6.11.4-201.fc40.x86_64/vmlinux | grep -m10 DW_AT_language
>     <12>   DW_AT_language    : 29	(C11)
>     <2b0a5>   DW_AT_language    : 29	(C11)
>     <2f3a4>   DW_AT_language    : 29	(C11)
>     <573b0>   DW_AT_language    : 29	(C11)
>     <6dd73>   DW_AT_language    : 29	(C11)
>     <879eb>   DW_AT_language    : 29	(C11)
>     <8c094>   DW_AT_language    : 29	(C11)
>     <9ce09>   DW_AT_language    : 29	(C11)
>     <9d8fb>   DW_AT_language    : 29	(C11)
>     <b877a>   DW_AT_language    : 29	(C11)
> root@x1:~# 
> root@x1:~# readelf -wi /usr/lib/debug/lib/modules/6.11.4-201.fc40.x86_64/vmlinux | grep -m1 -B5 DW_AT_language
>    Unit Type:     DW_UT_compile (1)
>    Abbrev Offset: 0
>    Pointer Size:  8
>  <0><c>: Abbrev Number: 246 (DW_TAG_compile_unit)
>     <e>   DW_AT_producer    : (indirect string, offset: 0x4edb56): GNU C11 14.2.1 20240912 (Red Hat 14.2.1-3) -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx -m64 -mno-80387 -mno-fp-ret-in-387 -mpreferred-stack-boundary=3 -mskip-rax-setup -mtune=generic -mno-red-zone -mcmodel=kernel -mindirect-branch=thunk-extern -mindirect-branch-register -mindirect-branch-cs-prefix -mfunction-return=thunk-extern -mharden-sls=all -mrecord-mcount -mfentry -march=x86-64 -g -O2 -std=gnu11 -p -fshort-wchar -funsigned-char -fno-common -fno-PIE -fno-strict-aliasing -fcf-protection=branch -falign-jumps=1 -falign-loops=1 -fno-asynchronous-unwind-tables -fno-jump-tables -fpatchable-function-entry=16,16 -fno-delete-null-pointer-checks -fno-allow-store-data-races -fstack-protector-strong -ftrivial-auto-var-init=zero -fno-stack-clash-protection -fmin-function-alignment=16 -fstrict-flex-arrays=3 -fno-strict-overflow -fstack-check=no -fconserve-stack -fno-function-sections -fno-data-sections -fsanitize=bounds-strict -fsanitize=shift
>     <12>   DW_AT_language    : 29	(C11)
> root@x1:~#
> 
> 	Wouldn't it be better to use that to decide how to deal with
> symbols in a CU?

Yes, it also works, but only with debuginfo. Maybe we can use
this as a hint flag to ensure this is non-C.

> 
> 	The advantage of using just the symbol name and that heuristic
> about finding glob characters in the symbols is when we don't have
> access to the DWARF info, having just the ELF symbol table, when we
> don't have the lang code for the CU.

Agreed. So if we can use DWARF and found the DW_AT_language, it will
be passed as a hint flag which ensure that this code comes from C or
not. BTW, Rust is 28 and C11 is 29, thus if the compiler supports the
newer version of C, it will have unknown code. So the hint flag should
be "the source code is a known C version".

Thank you,

> 
> - Arnaldo
> 
> > +		}
> >  	}
> >  	if (pev->group && !pev->sdt)
> >  		group = pev->group;
> > @@ -2858,7 +2881,7 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
> >  
> >  	/* Get an unused new event name */
> >  	ret = get_new_event_name(buf, sizeof(buf), event, namelist,
> > -				 tev->point.retprobe, allow_suffix);
> > +				 tev->point.retprobe, allow_suffix, is_C_symname);
> >  	if (ret < 0)
> >  		return ret;
> >  
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH 2/4] perf-probe: Require '@' prefix for filename always

[Masami Hiramatsu]

On Mon, 4 Nov 2024 17:10:41 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> On Tue, Nov 05, 2024 at 01:17:26AM +0900, Masami Hiramatsu (Google) wrote:
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>  
> > Currently perf probe allows user to specify probing place without '@'
> > prefix, for example, both `-V file:line` and `-V function:line` are
> > allowed. But this makes a problem if a (demangled) function name is
> > hard to be distinguished from a file name.
>  
> > This changes the perf-probe to require '@' prefix for filename even
> > without function name. For example, `-V @file:line` and
> > `-V function:line` are acceptable.
>  
> > With this change, users can specify filename or function correctly.
> 
> Well, this will break scripts that use perf probe for a given file,
> probably the right thing not to break backwards compatibility is to
> continue accepting and when there is a real conflict, an ambiguity that
> makes differentiating from file to function names, then refuse it,
> stating that it is ambiguous, probably spelling out the names of the
> files and functions that match so and stating that to make it
> unambiguoius, prefix file names with @.

The problem is that the ambiguous function name. For example, Go's
main routine is `main.main`, and this is not likely to the C function
name, so currently perf probe treats it as a filename and failed to
find that.

I think one possible solution is to run search loop twice internally
(search it as file name, if fails, search it as function name) if it
looks like a file name but it does not start from `@`.
This takes costs a bit but can keep backward compatibility.

Thank you,

> 
> - Arnaldo
>  
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > ---
> >  tools/perf/util/probe-event.c |   31 +++++++++----------------------
> >  1 file changed, 9 insertions(+), 22 deletions(-)
> > 
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 665dcce482e1..913a27cbb5d9 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -1341,7 +1341,7 @@ static bool is_c_func_name(const char *name)
> >   * Stuff 'lr' according to the line range described by 'arg'.
> >   * The line range syntax is described by:
> >   *
> > - *         SRC[:SLN[+NUM|-ELN]]
> > + *         @SRC[:SLN[+NUM|-ELN]]
> >   *         FNC[@SRC][:SLN[+NUM|-ELN]]
> >   */
> >  int parse_line_range_desc(const char *arg, struct line_range *lr)
> > @@ -1404,16 +1404,10 @@ int parse_line_range_desc(const char *arg, struct line_range *lr)
> >  			err = -ENOMEM;
> >  			goto err;
> >  		}
> > +		if (*name != '\0')
> > +			lr->function = name;
> > +	} else
> >  		lr->function = name;
> > -	} else if (strpbrk_esc(name, "/."))
> > -		lr->file = name;
> > -	else if (is_c_func_name(name))/* We reuse it for checking funcname */
> > -		lr->function = name;
> > -	else {	/* Invalid name */
> > -		semantic_error("'%s' is not a valid function name.\n", name);
> > -		err = -EINVAL;
> > -		goto err;
> > -	}
> >  
> >  	return 0;
> >  err:
> > @@ -1463,7 +1457,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >  
> >  	/*
> >  	 * <Syntax>
> > -	 * perf probe [GRP:][EVENT=]SRC[:LN|;PTN]
> > +	 * perf probe [GRP:][EVENT=]@SRC[:LN|;PTN]
> >  	 * perf probe [GRP:][EVENT=]FUNC[@SRC][+OFFS|%return|:LN|;PAT]
> >  	 * perf probe %[GRP:]SDT_EVENT
> >  	 */
> > @@ -1516,19 +1510,12 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >  	/*
> >  	 * Check arg is function or file name and copy it.
> >  	 *
> > -	 * We consider arg to be a file spec if and only if it satisfies
> > -	 * all of the below criteria::
> > -	 * - it does not include any of "+@%",
> > -	 * - it includes one of ":;", and
> > -	 * - it has a period '.' in the name.
> > -	 *
> > +	 * We consider arg to be a file spec if it starts with '@'.
> >  	 * Otherwise, we consider arg to be a function specification.
> >  	 */
> > -	if (!strpbrk_esc(arg, "+@%")) {
> > -		ptr = strpbrk_esc(arg, ";:");
> > -		/* This is a file spec if it includes a '.' before ; or : */
> > -		if (ptr && memchr(arg, '.', ptr - arg))
> > -			file_spec = true;
> > +	if (*arg == '@') {
> > +		file_spec = true;
> > +		arg++;
> >  	}
> >  
> >  	ptr = strpbrk_esc(arg, ";:+@%");


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH 3/4] perf-probe: Introduce quotation marks support

[Masami Hiramatsu]

On Mon, 4 Nov 2024 17:23:11 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> On Tue, Nov 05, 2024 at 01:17:35AM +0900, Masami Hiramatsu (Google) wrote:
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > 
> > In non-C languages, it is possible to have ':' in the function names.
> > It is possible to escape it with backslashes, but if there are too many
> > backslashes, it is annoying.
> > This introduce quotation marks (`"` or `'`) support.
> 
> Can you please split the patch so that strpbrk_esq() and strdup_esq()
> are introduced in different patches?

Yeah, OK. Let me split it.

Thank you,

> 
> - Arnaldo
>  
> > For example, without quotes, we have to pass it as below
> > 
> > $ perf probe -x cro3 -L "cro3\:\:cmd\:\:servo\:\:run_show"
> > <run_show@/work/cro3/src/cmd/servo.rs:0>
> >       0  fn run_show(args: &ArgsShow) -> Result<()> {
> >       1      let list = ServoList::discover()?;
> >       2      let s = list.find_by_serial(&args.servo)?;
> >       3      if args.json {
> >       4          println!("{s}");
> > 
> > With quotes, we can more naturally write the function name as below;
> > 
> > $ ./perf probe -x cro3 -L \"cro3::cmd::servo::run_show\"
> > <run_show@/work/cro3/src/cmd/servo.rs:0>
> >       0  fn run_show(args: &ArgsShow) -> Result<()> {
> >       1      let list = ServoList::discover()?;
> >       2      let s = list.find_by_serial(&args.servo)?;
> >       3      if args.json {
> >       4          println!("{s}");
> > 
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > ---
> >  tools/perf/util/probe-event.c  |   76 ++++++++++++++++--------------
> >  tools/perf/util/probe-finder.c |    3 +
> >  tools/perf/util/string.c       |  100 ++++++++++++++++++++++++++++++++++++++++
> >  tools/perf/util/string2.h      |    2 +
> >  4 files changed, 145 insertions(+), 36 deletions(-)
> > 
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 913a27cbb5d9..bcba8273204d 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -1065,6 +1065,7 @@ static int __show_line_range(struct line_range *lr, const char *module,
> >  
> >  	ret = debuginfo__find_line_range(dinfo, lr);
> >  	if (!ret) {	/* Not found, retry with an alternative */
> > +		pr_debug2("Failed to find line range in debuginfo. Fallback to alternative\n");
> >  		ret = get_alternative_line_range(dinfo, lr, module, user);
> >  		if (!ret)
> >  			ret = debuginfo__find_line_range(dinfo, lr);
> > @@ -1078,7 +1079,7 @@ static int __show_line_range(struct line_range *lr, const char *module,
> >  		pr_warning("Specified source line is not found.\n");
> >  		return -ENOENT;
> >  	} else if (ret < 0) {
> > -		pr_warning("Debuginfo analysis failed.\n");
> > +		pr_warning("Debuginfo analysis failed (%d).\n", ret);
> >  		return ret;
> >  	}
> >  
> > @@ -1187,7 +1188,7 @@ static int show_available_vars_at(struct debuginfo *dinfo,
> >  			pr_err("Failed to find the address of %s\n", buf);
> >  			ret = -ENOENT;
> >  		} else
> > -			pr_warning("Debuginfo analysis failed.\n");
> > +			pr_warning("Debuginfo analysis failed(2).\n");
> >  		goto end;
> >  	}
> >  
> > @@ -1343,30 +1344,39 @@ static bool is_c_func_name(const char *name)
> >   *
> >   *         @SRC[:SLN[+NUM|-ELN]]
> >   *         FNC[@SRC][:SLN[+NUM|-ELN]]
> > + *
> > + * SRC and FUNC can be quoted by double/single quotes.
> >   */
> >  int parse_line_range_desc(const char *arg, struct line_range *lr)
> >  {
> > -	char *range, *file, *name = strdup(arg);
> > +	char *buf = strdup(arg);
> > +	char *p;
> >  	int err;
> >  
> > -	if (!name)
> > +	if (!buf)
> >  		return -ENOMEM;
> >  
> >  	lr->start = 0;
> >  	lr->end = INT_MAX;
> >  
> > -	range = strpbrk_esc(name, ":");
> > -	if (range) {
> > -		*range++ = '\0';
> > +	pr_debug2("Input line range: %s\n", buf);
> > +	p = strpbrk_esq(buf, ":");
> > +	if (p) {
> > +		if (p == buf) {
> > +			semantic_error("No file/function name in '%s'.\n", p);
> > +			err = -EINVAL;
> > +			goto err;
> > +		}
> > +		*(p++) = '\0';
> >  
> > -		err = parse_line_num(&range, &lr->start, "start line");
> > +		err = parse_line_num(&p, &lr->start, "start line");
> >  		if (err)
> >  			goto err;
> >  
> > -		if (*range == '+' || *range == '-') {
> > -			const char c = *range++;
> > +		if (*p == '+' || *p == '-') {
> > +			const char c = *(p++);
> >  
> > -			err = parse_line_num(&range, &lr->end, "end line");
> > +			err = parse_line_num(&p, &lr->end, "end line");
> >  			if (err)
> >  				goto err;
> >  
> > @@ -1390,28 +1400,22 @@ int parse_line_range_desc(const char *arg, struct line_range *lr)
> >  				       " than end line.\n");
> >  			goto err;
> >  		}
> > -		if (*range != '\0') {
> > -			semantic_error("Tailing with invalid str '%s'.\n", range);
> > +		if (*p != '\0') {
> > +			semantic_error("Tailing with invalid str '%s'.\n", p);
> >  			goto err;
> >  		}
> >  	}
> >  
> > -	file = strpbrk_esc(name, "@");
> > -	if (file) {
> > -		*file = '\0';
> > -		lr->file = strdup_esc(++file);
> > -		if (lr->file == NULL) {
> > -			err = -ENOMEM;
> > -			goto err;
> > -		}
> > -		if (*name != '\0')
> > -			lr->function = name;
> > -	} else
> > -		lr->function = name;
> > +	p = strpbrk_esq(buf, "@");
> > +	if (p) {
> > +		*(p++) = '\0';
> > +		lr->file = strdup_esq(p);
> > +	}
> > +	lr->function = strdup_esq(buf);
> > +	err = 0;
> >  
> > -	return 0;
> >  err:
> > -	free(name);
> > +	free(buf);
> >  	return err;
> >  }
> >  
> > @@ -1419,19 +1423,19 @@ static int parse_perf_probe_event_name(char **arg, struct perf_probe_event *pev)
> >  {
> >  	char *ptr;
> >  
> > -	ptr = strpbrk_esc(*arg, ":");
> > +	ptr = strpbrk_esq(*arg, ":");
> >  	if (ptr) {
> >  		*ptr = '\0';
> >  		if (!pev->sdt && !is_c_func_name(*arg))
> >  			goto ng_name;
> > -		pev->group = strdup_esc(*arg);
> > +		pev->group = strdup_esq(*arg);
> >  		if (!pev->group)
> >  			return -ENOMEM;
> >  		*arg = ptr + 1;
> >  	} else
> >  		pev->group = NULL;
> >  
> > -	pev->event = strdup_esc(*arg);
> > +	pev->event = strdup_esq(*arg);
> >  	if (pev->event == NULL)
> >  		return -ENOMEM;
> >  
> > @@ -1470,7 +1474,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >  			arg++;
> >  	}
> >  
> > -	ptr = strpbrk_esc(arg, ";=@+%");
> > +	ptr = strpbrk_esq(arg, ";=@+%");
> >  	if (pev->sdt) {
> >  		if (ptr) {
> >  			if (*ptr != '@') {
> > @@ -1484,7 +1488,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >  				pev->target = build_id_cache__origname(tmp);
> >  				free(tmp);
> >  			} else
> > -				pev->target = strdup_esc(ptr + 1);
> > +				pev->target = strdup_esq(ptr + 1);
> >  			if (!pev->target)
> >  				return -ENOMEM;
> >  			*ptr = '\0';
> > @@ -1518,7 +1522,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >  		arg++;
> >  	}
> >  
> > -	ptr = strpbrk_esc(arg, ";:+@%");
> > +	ptr = strpbrk_esq(arg, ";:+@%");
> >  	if (ptr) {
> >  		nc = *ptr;
> >  		*ptr++ = '\0';
> > @@ -1527,7 +1531,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >  	if (arg[0] == '\0')
> >  		tmp = NULL;
> >  	else {
> > -		tmp = strdup_esc(arg);
> > +		tmp = strdup_esq(arg);
> >  		if (tmp == NULL)
> >  			return -ENOMEM;
> >  	}
> > @@ -1565,7 +1569,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >  				return -ENOMEM;
> >  			break;
> >  		}
> > -		ptr = strpbrk_esc(arg, ";:+@%");
> > +		ptr = strpbrk_esq(arg, ";:+@%");
> >  		if (ptr) {
> >  			nc = *ptr;
> >  			*ptr++ = '\0';
> > @@ -1592,7 +1596,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> >  				semantic_error("SRC@SRC is not allowed.\n");
> >  				return -EINVAL;
> >  			}
> > -			pp->file = strdup_esc(arg);
> > +			pp->file = strdup_esq(arg);
> >  			if (pp->file == NULL)
> >  				return -ENOMEM;
> >  			break;
> > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > index 630e16c54ed5..5462b5541a6d 100644
> > --- a/tools/perf/util/probe-finder.c
> > +++ b/tools/perf/util/probe-finder.c
> > @@ -1796,6 +1796,7 @@ int debuginfo__find_line_range(struct debuginfo *dbg, struct line_range *lr)
> >  		struct dwarf_callback_param line_range_param = {
> >  			.data = (void *)&lf, .retval = 0};
> >  
> > +		pr_debug2("Start searching function '%s' in getpubname\n", lr->function);
> >  		dwarf_getpubnames(dbg->dbg, pubname_search_cb,
> >  				  &pubname_param, 0);
> >  		if (pubname_param.found) {
> > @@ -1803,8 +1804,10 @@ int debuginfo__find_line_range(struct debuginfo *dbg, struct line_range *lr)
> >  			if (lf.found)
> >  				goto found;
> >  		}
> > +		pr_debug2("Not found, use DIE tree\n");
> >  	}
> >  
> > +	pr_debug2("Search function '%s' in DIE tree\n", lr->function);
> >  	/* Loop on CUs (Compilation Unit) */
> >  	while (!lf.found && ret >= 0) {
> >  		if (dwarf_nextcu(dbg->dbg, off, &noff, &cuhl,
> > diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
> > index 116a642ad99d..308fc7ec88cc 100644
> > --- a/tools/perf/util/string.c
> > +++ b/tools/perf/util/string.c
> > @@ -263,6 +263,34 @@ char *strpbrk_esc(char *str, const char *stopset)
> >  	return ptr;
> >  }
> >  
> > +/* Like strpbrk_esc(), but not break if it is quoted with single/double quotes */
> > +char *strpbrk_esq(char *str, const char *stopset)
> > +{
> > +	char *_stopset = NULL;
> > +	char *ptr;
> > +	const char *squote = "'";
> > +	const char *dquote = "\"";
> > +
> > +	if (asprintf(&_stopset, "%s%c%c", stopset, *squote, *dquote) < 0)
> > +		return NULL;
> > +
> > +	do {
> > +		ptr = strpbrk_esc(str, _stopset);
> > +		if (!ptr)
> > +			break;
> > +		if (*ptr == *squote)
> > +			ptr = strpbrk_esc(ptr + 1, squote);
> > +		else if (*ptr == *dquote)
> > +			ptr = strpbrk_esc(ptr + 1, dquote);
> > +		else
> > +			break;
> > +		str = ptr + 1;
> > +	} while (ptr);
> > +
> > +	free(_stopset);
> > +	return ptr;
> > +}
> > +
> >  /* Like strdup, but do not copy a single backslash */
> >  char *strdup_esc(const char *str)
> >  {
> > @@ -293,6 +321,78 @@ char *strdup_esc(const char *str)
> >  	return ret;
> >  }
> >  
> > +/* Remove backslash right before quote and return next quote address. */
> > +static char *remove_consumed_esc(char *str, int len, int quote)
> > +{
> > +	char *ptr = str, *end = str + len;
> > +
> > +	while (*ptr != quote && ptr < end) {
> > +		if (*ptr == '\\' && *(ptr + 1) == quote) {
> > +			memmove(ptr, ptr + 1, end - (ptr + 1));
> > +			/* now *ptr is `quote`. */
> > +			end--;
> > +		}
> > +		ptr++;
> > +	}
> > +
> > +	return *ptr == quote ? ptr : NULL;
> > +}
> > +
> > +/*
> > + * Like strdup_esc, but keep quoted string as it is (and single backslash
> > + * before quote is removed). If there is no closed quote, return NULL.
> > + */
> > +char *strdup_esq(const char *str)
> > +{
> > +	char *d, *ret;
> > +
> > +	/* If there is no quote, return normal strdup_esc() */
> > +	d = strpbrk_esc((char *)str, "\"'");
> > +	if (!d)
> > +		return strdup_esc(str);
> > +
> > +	ret = strdup(str);
> > +	if (!ret)
> > +		return NULL;
> > +
> > +	d = ret;
> > +	do {
> > +		d = strpbrk(d, "\\\"\'");
> > +		if (!d)
> > +			break;
> > +
> > +		if (*d == '"' || *d == '\'') {
> > +			/* This is non-escaped quote */
> > +			int quote = *d;
> > +			int len = strlen(d + 1) + 1;
> > +
> > +			/*
> > +			 * Remove the start quote and remove consumed escape (backslash
> > +			 * before quote) and remove the end quote. If there is no end
> > +			 * quote, it is the input error.
> > +			 */
> > +			memmove(d, d + 1, len);
> > +			d = remove_consumed_esc(d, len, quote);
> > +			if (!d)
> > +				goto error;
> > +			memmove(d, d + 1, strlen(d + 1) + 1);
> > +		}
> > +		if (*d == '\\') {
> > +			memmove(d, d + 1, strlen(d + 1) + 1);
> > +			if (*d == '\\') {
> > +				/* double backslash -- keep the second one. */
> > +				d++;
> > +			}
> > +		}
> > +	} while (*d != '\0');
> > +
> > +	return ret;
> > +
> > +error:
> > +	free(ret);
> > +	return NULL;
> > +}
> > +
> >  unsigned int hex(char c)
> >  {
> >  	if (c >= '0' && c <= '9')
> > diff --git a/tools/perf/util/string2.h b/tools/perf/util/string2.h
> > index 52cb8ba057c7..4c8bff47cfd3 100644
> > --- a/tools/perf/util/string2.h
> > +++ b/tools/perf/util/string2.h
> > @@ -37,6 +37,8 @@ char *asprintf__tp_filter_pids(size_t npids, pid_t *pids);
> >  
> >  char *strpbrk_esc(char *str, const char *stopset);
> >  char *strdup_esc(const char *str);
> > +char *strpbrk_esq(char *str, const char *stopset);
> > +char *strdup_esq(const char *str);
> >  
> >  unsigned int hex(char c);
> >  char *strreplace_chars(char needle, const char *haystack, const char *replace);


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH 2/4] perf-probe: Require '@' prefix for filename always

[Masami Hiramatsu]

On Tue, 5 Nov 2024 18:28:30 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> On Mon, 4 Nov 2024 17:10:41 -0300
> Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> > On Tue, Nov 05, 2024 at 01:17:26AM +0900, Masami Hiramatsu (Google) wrote:
> > > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> >  
> > > Currently perf probe allows user to specify probing place without '@'
> > > prefix, for example, both `-V file:line` and `-V function:line` are
> > > allowed. But this makes a problem if a (demangled) function name is
> > > hard to be distinguished from a file name.
> >  
> > > This changes the perf-probe to require '@' prefix for filename even
> > > without function name. For example, `-V @file:line` and
> > > `-V function:line` are acceptable.
> >  
> > > With this change, users can specify filename or function correctly.
> > 
> > Well, this will break scripts that use perf probe for a given file,
> > probably the right thing not to break backwards compatibility is to
> > continue accepting and when there is a real conflict, an ambiguity that
> > makes differentiating from file to function names, then refuse it,
> > stating that it is ambiguous, probably spelling out the names of the
> > files and functions that match so and stating that to make it
> > unambiguoius, prefix file names with @.
> 
> The problem is that the ambiguous function name. For example, Go's
> main routine is `main.main`, and this is not likely to the C function
> name, so currently perf probe treats it as a filename and failed to
> find that.
> 
> I think one possible solution is to run search loop twice internally
> (search it as file name, if fails, search it as function name) if it
> looks like a file name but it does not start from `@`.
> This takes costs a bit but can keep backward compatibility.

I found another good idea, support `@*` :) 

For example, if the `main.main` is not a file, we can

 perf probe -x execfile -L 'main.main@*'

Then it is clear that `main.main` is a function name.

Thank you!

> 
> Thank you,
> 
> > 
> > - Arnaldo
> >  
> > > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > > ---
> > >  tools/perf/util/probe-event.c |   31 +++++++++----------------------
> > >  1 file changed, 9 insertions(+), 22 deletions(-)
> > > 
> > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > > index 665dcce482e1..913a27cbb5d9 100644
> > > --- a/tools/perf/util/probe-event.c
> > > +++ b/tools/perf/util/probe-event.c
> > > @@ -1341,7 +1341,7 @@ static bool is_c_func_name(const char *name)
> > >   * Stuff 'lr' according to the line range described by 'arg'.
> > >   * The line range syntax is described by:
> > >   *
> > > - *         SRC[:SLN[+NUM|-ELN]]
> > > + *         @SRC[:SLN[+NUM|-ELN]]
> > >   *         FNC[@SRC][:SLN[+NUM|-ELN]]
> > >   */
> > >  int parse_line_range_desc(const char *arg, struct line_range *lr)
> > > @@ -1404,16 +1404,10 @@ int parse_line_range_desc(const char *arg, struct line_range *lr)
> > >  			err = -ENOMEM;
> > >  			goto err;
> > >  		}
> > > +		if (*name != '\0')
> > > +			lr->function = name;
> > > +	} else
> > >  		lr->function = name;
> > > -	} else if (strpbrk_esc(name, "/."))
> > > -		lr->file = name;
> > > -	else if (is_c_func_name(name))/* We reuse it for checking funcname */
> > > -		lr->function = name;
> > > -	else {	/* Invalid name */
> > > -		semantic_error("'%s' is not a valid function name.\n", name);
> > > -		err = -EINVAL;
> > > -		goto err;
> > > -	}
> > >  
> > >  	return 0;
> > >  err:
> > > @@ -1463,7 +1457,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> > >  
> > >  	/*
> > >  	 * <Syntax>
> > > -	 * perf probe [GRP:][EVENT=]SRC[:LN|;PTN]
> > > +	 * perf probe [GRP:][EVENT=]@SRC[:LN|;PTN]
> > >  	 * perf probe [GRP:][EVENT=]FUNC[@SRC][+OFFS|%return|:LN|;PAT]
> > >  	 * perf probe %[GRP:]SDT_EVENT
> > >  	 */
> > > @@ -1516,19 +1510,12 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> > >  	/*
> > >  	 * Check arg is function or file name and copy it.
> > >  	 *
> > > -	 * We consider arg to be a file spec if and only if it satisfies
> > > -	 * all of the below criteria::
> > > -	 * - it does not include any of "+@%",
> > > -	 * - it includes one of ":;", and
> > > -	 * - it has a period '.' in the name.
> > > -	 *
> > > +	 * We consider arg to be a file spec if it starts with '@'.
> > >  	 * Otherwise, we consider arg to be a function specification.
> > >  	 */
> > > -	if (!strpbrk_esc(arg, "+@%")) {
> > > -		ptr = strpbrk_esc(arg, ";:");
> > > -		/* This is a file spec if it includes a '.' before ; or : */
> > > -		if (ptr && memchr(arg, '.', ptr - arg))
> > > -			file_spec = true;
> > > +	if (*arg == '@') {
> > > +		file_spec = true;
> > > +		arg++;
> > >  	}
> > >  
> > >  	ptr = strpbrk_esc(arg, ";:+@%");
> 
> 
> -- 
> Masami Hiramatsu (Google) <mhiramat@kernel.org>


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH 0/4] perf-probe: Improbe non-C language support

[Masami Hiramatsu]

On Mon, 4 Nov 2024 17:08:05 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> On Mon, Nov 04, 2024 at 03:56:00PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Tue, Nov 05, 2024 at 01:17:08AM +0900, Masami Hiramatsu (Google) wrote:
> > > Hi,
> 
> I also now noticed that the cover letter subject has a typo, it should
> be "Improve", not "Improbe" :-)

Lol, good catch! My finger learned that the next character of "pro" is "b".

Thank you,

> 
> - Arnaldo
>   
> > > Here is a series of patches for perf probe to improve non-C language
> > > (e.g. Rust, Go) support.
> >  
> > > The non-C symbols are demangled style in debuginfo, e.g. golang stores
> >  
> > > ----
> > > $ ./perf probe -x /work/go/example/outyet/main -F main*
> > > main.(*Server).ServeHTTP
> > > main.(*Server).ServeHTTP.Print.func1
> > > main.(*Server).poll
> > > ...
> > > -----
> > 
> > I presented about this last year:
> > 
> > https://tracingsummit.org/ts/2023/bpf-non-c/
> > https://tracingsummit.org/ts/2023/files/Trying_to_use_uprobes_and_BPF_on_non-C_userspace.pdf
> > https://www.youtube.com/watch?v=RDFRy1vWyHg&feature=youtu.be
> > 
> > So trying to do some of the things I did while playing with golang, and
> > with your patches applied, I only had to cope with a minor clash with a
> > patch by Ian Rogers that is present on perf-tools-next, related to:
> > 
> >         char buf[MAX_EVENT_NAME_LEN];
> > 
> > That in your patch expects the old 64 hard coded value, which will
> > appear in the my tests:
> > 
> > root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -x main -F github*counter*
> > github.com/prometheus/client_golang/prometheus.(*counter).Add
> > github.com/prometheus/client_golang/prometheus.(*counter).AddWithExemplar
> > github.com/prometheus/client_golang/prometheus.(*counter).Collect
> > github.com/prometheus/client_golang/prometheus.(*counter).Desc
> > github.com/prometheus/client_golang/prometheus.(*counter).Describe
> > github.com/prometheus/client_golang/prometheus.(*counter).Inc
> > github.com/prometheus/client_golang/prometheus.(*counter).Write
> > github.com/prometheus/client_golang/prometheus.(*counter).updateExemplar
> > root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -x main -F github*counter*
> > 
> > Then trying to add for all those:
> > 
> > root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -d *:*
> > "*:*" does not hit any event.
> >   Error: Failed to delete events.
> > root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -l
> > root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#
> > 
> > root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -x main github*counter*
> > A function DIE doesn't have decl_line. Maybe broken DWARF?
> > A function DIE doesn't have decl_line. Maybe broken DWARF?
> > A function DIE doesn't have decl_line. Maybe broken DWARF?
> > A function DIE doesn't have decl_line. Maybe broken DWARF?
> > A function DIE doesn't have decl_line. Maybe broken DWARF?
> > snprintf() failed: -7; the event name 'github_com_prometheus_client_golang_prometheus_counter_AddWithExemplar' is too long
> >   Hint: Set a shorter event with syntax "EVENT=PROBEDEF"
> >         EVENT: Event name (max length: 64 bytes).
> >   Error: Failed to add events.
> > root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#
> > 
> > But:
> > 
> > root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -l
> >   probe_main:github_com_prometheus_client_golang_prometheus_counter_Desc (on github.com/prometheus/client_golang/prometheus.(*counter).Des>
> > (END)
> > 
> > That pager thing looks odd as well, since there is just one line in the
> > output...
> > 
> > So it failed to do all those, added just one, maybe state that some were
> > added but from the problematic one onwards it stopped? Or try all of
> > them and just state the ones that couldn't be added?
> > 
> > I.e. something like:
> > 
> > root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -x main -F github*counter* | while read probename ; do perf probe -x main $probename ; done
> > A function DIE doesn't have decl_line. Maybe broken DWARF?
> > A function DIE doesn't have decl_line. Maybe broken DWARF?
> > Failed to find debug information for address 0x3287e0
> > Probe point 'github.com/prometheus/client_golang/prometheus.(*counter).Add' not found.
> >   Error: Failed to add events.
> > snprintf() failed: -7; the event name 'github_com_prometheus_client_golang_prometheus_counter_AddWithExemplar' is too long
> >   Hint: Set a shorter event with syntax "EVENT=PROBEDEF"
> >         EVENT: Event name (max length: 64 bytes).
> >   Error: Failed to add events.
> > A function DIE doesn't have decl_line. Maybe broken DWARF?
> > Failed to find debug information for address 0x33ab40
> > Probe point 'github.com/prometheus/client_golang/prometheus.(*counter).Collect' not found.
> >   Error: Failed to add events.
> > Error: event "github_com_prometheus_client_golang_prometheus_counter_Desc" already exists.
> >  Hint: Remove existing event by 'perf probe -d'
> >        or force duplicates by 'perf probe -f'
> >        or set 'force=yes' in BPF source.
> >   Error: Failed to add events.
> > A function DIE doesn't have decl_line. Maybe broken DWARF?
> > Failed to find debug information for address 0x33aba0
> > Probe point 'github.com/prometheus/client_golang/prometheus.(*counter).Describe' not found.
> >   Error: Failed to add events.
> > Added new event:
> >   probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc (on github.com/prometheus/client_golang/prometheus.(*counter).Inc in /home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
> > 
> > You can now use it in all perf tools, such as:
> > 
> > 	perf record -e probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc -aR sleep 1
> > 
> > Added new event:
> >   probe_main:github_com_prometheus_client_golang_prometheus_counter_Write (on github.com/prometheus/client_golang/prometheus.(*counter).Write in /home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
> > 
> > You can now use it in all perf tools, such as:
> > 
> > 	perf record -e probe_main:github_com_prometheus_client_golang_prometheus_counter_Write -aR sleep 1
> > 
> > snprintf() failed: -7; the event name 'github_com_prometheus_client_golang_prometheus_counter_updateExemplar' is too long
> >   Hint: Set a shorter event with syntax "EVENT=PROBEDEF"
> >         EVENT: Event name (max length: 64 bytes).
> >   Error: Failed to add events.
> > root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#
> > 
> > In the end we get:
> > 
> > root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -l
> >   probe_main:github_com_prometheus_client_golang_prometheus_counter_Desc (on github.com/prometheus/client_golang/prometheus.(*counter).Desc@prometheus/counter.go in /home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
> >   probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc (on github.com/prometheus/client_golang/prometheus.(*counter).Inc@prometheus/counter.go in /home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
> >   probe_main:github_com_prometheus_client_golang_prometheus_counter_Write (on github.com/prometheus/client_golang/prometheus.(*counter).Write@prometheus/counter.go in /home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
> > root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#
> > 
> > That also explains the pager kicking in: I had to reduce font size in my
> > xterm (gnome-terminal really) and then the perf pager wasn't used (no
> > (END) at the last line waiting for me to press 'q').
> > 
> > The ones that got installed are working:
> > 
> > root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf trace -e probe_main:*
> >      0.000 main/616840 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
> >   1001.043 main/616926 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
> >   1001.080 main/616926 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
> >   4000.994 main/616926 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
> > ^Croot@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf trace -e probe_main:*/max-stack=8/
> >      0.000 main/616926 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
> >                                        github.com/prometheus/client_golang/prometheus.(*counter).Inc (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
> >                                        runtime.goexit.abi0 (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
> >      0.030 main/616926 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
> >                                        github.com/prometheus/client_golang/prometheus.(*counter).Inc (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
> >                                        runtime.goexit.abi0 (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
> >   3000.166 main/616840 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
> >                                        github.com/prometheus/client_golang/prometheus.(*counter).Inc (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
> >                                        runtime.goexit.abi0 (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
> > ^Croot@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#
> > 
> > I'll test this some more later/tomorrow, just wanted to give this first
> > reaction, thanks for working on this!
> > 
> > Btw, some more info about the environment (fedora 40):
> > 
> > root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# readelf -wi main | head -20
> > Contents of the .debug_info section:
> > 
> >   Compilation Unit @ offset 0:
> >    Length:        0x506 (32-bit)
> >    Version:       4
> >    Abbrev Offset: 0
> >    Pointer Size:  8
> >  <0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
> >     <c>   DW_AT_name        : google.golang.org/protobuf/internal/strs
> >     <35>   DW_AT_language    : 22	(Go)
> >     <36>   DW_AT_stmt_list   : 0
> >     <3a>   DW_AT_low_pc      : 0x667c40
> >     <42>   DW_AT_ranges      : 0
> >     <46>   DW_AT_comp_dir    : .
> >     <48>   DW_AT_producer    : Go cmd/compile go1.22.7; regabi
> >     <68>   Unknown AT value: 2905: strs
> >  <1><6d>: Abbrev Number: 5 (DW_TAG_subprogram)
> >     <6e>   DW_AT_name        : google.golang.org/protobuf/internal/strs.isASCIILower
> >     <a4>   DW_AT_inline      : 1	(inlined)
> >     <a5>   DW_AT_decl_line   : 188
> > root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#
> > 
> > - Arnaldo
> >  
> > > And Rust stores
> > > -----
> > > $ ./perf probe -x /work/cro3/target/x86_64-unknown-linux-gnu/debug/cro3 -F cro3::cmd::servo*
> > > cro3::cmd::servo::run
> > > cro3::cmd::servo::run::CALLSITE
> > > cro3::cmd::servo::run::CALLSITE::META
> > > cro3::cmd::servo::run_control
> > > -----
> > > 
> > > These symbols are not parsed correctly because it looks like a file name or
> > > including line numbers (`:` caused it.) So, I decided to introduce the changes
> > > 
> > >  - filename MUST start from '@'. (so it is able to distinguish the filename
> > >    and the function name)
> > >  - Fix to allow backslash to escape to --lines option.
> > >  - Introduce quotation mark support.
> > >  - Replace non-alnum character to '_' for event name (for non-C symbols).
> > > 
> > > With these changes, we can run -L (--lines) on golang;
> > > 
> > > ------
> > > $ perf probe -x goexample/hello/hello -L \"main.main\"
> > > <main.main@/work/goexample/hello/hello.go:0>
> > >       0  func main() {
> > >                 // Configure logging for a command-line program.
> > >       2         log.SetFlags(0)
> > >       3         log.SetPrefix("hello: ")
> > > 
> > >                 // Parse flags.
> > >       6         flag.Usage = usage
> > >       7         flag.Parse()
> > > ------
> > > 
> > > And Rust
> > > ------
> > > $ perf probe -x cro3 -L \"cro3::cmd::servo::run_show\"
> > > <run_show@/work/cro3/src/cmd/servo.rs:0>
> > >       0  fn run_show(args: &ArgsShow) -> Result<()> {
> > >       1      let list = ServoList::discover()?;
> > >       2      let s = list.find_by_serial(&args.servo)?;
> > >       3      if args.json {
> > >       4          println!("{s}");
> > > ------
> > > 
> > > And event name are created automatically like below;
> > > 
> > > $ ./perf probe -x /work/go/example/outyet/main -D 'main.(*Server).poll'
> > > p:probe_main/main_Server_poll /work/go/example/outyet/main:0x353040
> > > 
> > > $ ./perf probe -x cro3 -D \"cro3::cmd::servo::run_show\"
> > > p:probe_cro3/cro3_cmd_servo_run_show /work/cro3/target/x86_64-unknown-linux-gnu/debug/cro3:0x197530
> > > 
> > > We still need some more work, but these shows how perf-probe can work
> > > with other languages.
> > > 
> > > Thank you,
> > > 
> > > ---
> > > 
> > > Masami Hiramatsu (Google) (4):
> > >       perf-probe: Fix to ignore escaped characters in --lines option
> > >       perf-probe: Require '@' prefix for filename always
> > >       perf-probe: Introduce quotation marks support
> > >       perf-probe: Replace unacceptable characters when generating event name
> > > 
> > > 
> > >  tools/perf/util/probe-event.c  |  136 ++++++++++++++++++++++------------------
> > >  tools/perf/util/probe-finder.c |    3 +
> > >  tools/perf/util/string.c       |  100 +++++++++++++++++++++++++++++
> > >  tools/perf/util/string2.h      |    2 +
> > >  4 files changed, 180 insertions(+), 61 deletions(-)
> > > 
> > > --
> > > Masami Hiramatsu (Google) <mhiramat@kernel.org>


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH 0/4] perf-probe: Improbe non-C language support

[Masami Hiramatsu]

On Mon, 4 Nov 2024 15:56:00 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> On Tue, Nov 05, 2024 at 01:17:08AM +0900, Masami Hiramatsu (Google) wrote:
> > Hi,
>  
> > Here is a series of patches for perf probe to improve non-C language
> > (e.g. Rust, Go) support.
>  
> > The non-C symbols are demangled style in debuginfo, e.g. golang stores
>  
> > ----
> > $ ./perf probe -x /work/go/example/outyet/main -F main*
> > main.(*Server).ServeHTTP
> > main.(*Server).ServeHTTP.Print.func1
> > main.(*Server).poll
> > ...
> > -----
> 
> I presented about this last year:
> 
> https://tracingsummit.org/ts/2023/bpf-non-c/
> https://tracingsummit.org/ts/2023/files/Trying_to_use_uprobes_and_BPF_on_non-C_userspace.pdf
> https://www.youtube.com/watch?v=RDFRy1vWyHg&feature=youtu.be

Nice!

> 
> So trying to do some of the things I did while playing with golang, and
> with your patches applied, I only had to cope with a minor clash with a
> patch by Ian Rogers that is present on perf-tools-next, related to:
> 
>         char buf[MAX_EVENT_NAME_LEN];
> 
> That in your patch expects the old 64 hard coded value, which will
> appear in the my tests:

Ah, OK let me rebase on it.

> 
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -x main -F github*counter*
> github.com/prometheus/client_golang/prometheus.(*counter).Add
> github.com/prometheus/client_golang/prometheus.(*counter).AddWithExemplar
> github.com/prometheus/client_golang/prometheus.(*counter).Collect
> github.com/prometheus/client_golang/prometheus.(*counter).Desc
> github.com/prometheus/client_golang/prometheus.(*counter).Describe
> github.com/prometheus/client_golang/prometheus.(*counter).Inc
> github.com/prometheus/client_golang/prometheus.(*counter).Write
> github.com/prometheus/client_golang/prometheus.(*counter).updateExemplar
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -x main -F github*counter*
> 
> Then trying to add for all those:
> 
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -d *:*
> "*:*" does not hit any event.
>   Error: Failed to delete events.
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -l
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#
> 
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -x main github*counter*
> A function DIE doesn't have decl_line. Maybe broken DWARF?
> A function DIE doesn't have decl_line. Maybe broken DWARF?
> A function DIE doesn't have decl_line. Maybe broken DWARF?
> A function DIE doesn't have decl_line. Maybe broken DWARF?
> A function DIE doesn't have decl_line. Maybe broken DWARF?
> snprintf() failed: -7; the event name 'github_com_prometheus_client_golang_prometheus_counter_AddWithExemplar' is too long
>   Hint: Set a shorter event with syntax "EVENT=PROBEDEF"
>         EVENT: Event name (max length: 64 bytes).
>   Error: Failed to add events.
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#
> 
> But:
> 
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -l
>   probe_main:github_com_prometheus_client_golang_prometheus_counter_Desc (on github.com/prometheus/client_golang/prometheus.(*counter).Des>
> (END)
> 
> That pager thing looks odd as well, since there is just one line in the
> output...
> 
> So it failed to do all those, added just one, maybe state that some were
> added but from the problematic one onwards it stopped? Or try all of
> them and just state the ones that couldn't be added?

OK, yes, it should show what events are actually added.

> 
> I.e. something like:
> 
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -x main -F github*counter* | while read probename ; do perf probe -x main $probename ; done
> A function DIE doesn't have decl_line. Maybe broken DWARF?
> A function DIE doesn't have decl_line. Maybe broken DWARF?
> Failed to find debug information for address 0x3287e0
> Probe point 'github.com/prometheus/client_golang/prometheus.(*counter).Add' not found.
>   Error: Failed to add events.
> snprintf() failed: -7; the event name 'github_com_prometheus_client_golang_prometheus_counter_AddWithExemplar' is too long
>   Hint: Set a shorter event with syntax "EVENT=PROBEDEF"
>         EVENT: Event name (max length: 64 bytes).
>   Error: Failed to add events.
> A function DIE doesn't have decl_line. Maybe broken DWARF?
> Failed to find debug information for address 0x33ab40
> Probe point 'github.com/prometheus/client_golang/prometheus.(*counter).Collect' not found.
>   Error: Failed to add events.
> Error: event "github_com_prometheus_client_golang_prometheus_counter_Desc" already exists.
>  Hint: Remove existing event by 'perf probe -d'
>        or force duplicates by 'perf probe -f'
>        or set 'force=yes' in BPF source.
>   Error: Failed to add events.
> A function DIE doesn't have decl_line. Maybe broken DWARF?
> Failed to find debug information for address 0x33aba0
> Probe point 'github.com/prometheus/client_golang/prometheus.(*counter).Describe' not found.
>   Error: Failed to add events.
> Added new event:
>   probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc (on github.com/prometheus/client_golang/prometheus.(*counter).Inc in /home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
> 
> You can now use it in all perf tools, such as:
> 
> 	perf record -e probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc -aR sleep 1
> 
> Added new event:
>   probe_main:github_com_prometheus_client_golang_prometheus_counter_Write (on github.com/prometheus/client_golang/prometheus.(*counter).Write in /home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
> 
> You can now use it in all perf tools, such as:
> 
> 	perf record -e probe_main:github_com_prometheus_client_golang_prometheus_counter_Write -aR sleep 1
> 
> snprintf() failed: -7; the event name 'github_com_prometheus_client_golang_prometheus_counter_updateExemplar' is too long
>   Hint: Set a shorter event with syntax "EVENT=PROBEDEF"
>         EVENT: Event name (max length: 64 bytes).
>   Error: Failed to add events.
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#
> 
> In the end we get:
> 
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf probe -l
>   probe_main:github_com_prometheus_client_golang_prometheus_counter_Desc (on github.com/prometheus/client_golang/prometheus.(*counter).Desc@prometheus/counter.go in /home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
>   probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc (on github.com/prometheus/client_golang/prometheus.(*counter).Inc@prometheus/counter.go in /home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
>   probe_main:github_com_prometheus_client_golang_prometheus_counter_Write (on github.com/prometheus/client_golang/prometheus.(*counter).Write@prometheus/counter.go in /home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#
> 
> That also explains the pager kicking in: I had to reduce font size in my
> xterm (gnome-terminal really) and then the perf pager wasn't used (no
> (END) at the last line waiting for me to press 'q').
> 
> The ones that got installed are working:
> 
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf trace -e probe_main:*
>      0.000 main/616840 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
>   1001.043 main/616926 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
>   1001.080 main/616926 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
>   4000.994 main/616926 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
> ^Croot@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# perf trace -e probe_main:*/max-stack=8/
>      0.000 main/616926 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
>                                        github.com/prometheus/client_golang/prometheus.(*counter).Inc (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
>                                        runtime.goexit.abi0 (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
>      0.030 main/616926 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
>                                        github.com/prometheus/client_golang/prometheus.(*counter).Inc (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
>                                        runtime.goexit.abi0 (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
>   3000.166 main/616840 probe_main:github_com_prometheus_client_golang_prometheus_counter_Inc(__probe_ip: 7506464)
>                                        github.com/prometheus/client_golang/prometheus.(*counter).Inc (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
>                                        runtime.goexit.abi0 (/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus/main)
> ^Croot@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#
> 
> I'll test this some more later/tomorrow, just wanted to give this first
> reaction, thanks for working on this!

Thanks for testing and the information!

Since the event name limitation is in the kernel too, user should specify
abbreviated event name in this case (or use only the last few words).

Thank you,

> 
> Btw, some more info about the environment (fedora 40):
> 
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus# readelf -wi main | head -20
> Contents of the .debug_info section:
> 
>   Compilation Unit @ offset 0:
>    Length:        0x506 (32-bit)
>    Version:       4
>    Abbrev Offset: 0
>    Pointer Size:  8
>  <0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
>     <c>   DW_AT_name        : google.golang.org/protobuf/internal/strs
>     <35>   DW_AT_language    : 22	(Go)
>     <36>   DW_AT_stmt_list   : 0
>     <3a>   DW_AT_low_pc      : 0x667c40
>     <42>   DW_AT_ranges      : 0
>     <46>   DW_AT_comp_dir    : .
>     <48>   DW_AT_producer    : Go cmd/compile go1.22.7; regabi
>     <68>   Unknown AT value: 2905: strs
>  <1><6d>: Abbrev Number: 5 (DW_TAG_subprogram)
>     <6e>   DW_AT_name        : google.golang.org/protobuf/internal/strs.isASCIILower
>     <a4>   DW_AT_inline      : 1	(inlined)
>     <a5>   DW_AT_decl_line   : 188
> root@x1:/home/acme/git/libbpf-bootstrap/examples/c/tests/prometheus#
> 
> - Arnaldo
>  
> > And Rust stores
> > -----
> > $ ./perf probe -x /work/cro3/target/x86_64-unknown-linux-gnu/debug/cro3 -F cro3::cmd::servo*
> > cro3::cmd::servo::run
> > cro3::cmd::servo::run::CALLSITE
> > cro3::cmd::servo::run::CALLSITE::META
> > cro3::cmd::servo::run_control
> > -----
> > 
> > These symbols are not parsed correctly because it looks like a file name or
> > including line numbers (`:` caused it.) So, I decided to introduce the changes
> > 
> >  - filename MUST start from '@'. (so it is able to distinguish the filename
> >    and the function name)
> >  - Fix to allow backslash to escape to --lines option.
> >  - Introduce quotation mark support.
> >  - Replace non-alnum character to '_' for event name (for non-C symbols).
> > 
> > With these changes, we can run -L (--lines) on golang;
> > 
> > ------
> > $ perf probe -x goexample/hello/hello -L \"main.main\"
> > <main.main@/work/goexample/hello/hello.go:0>
> >       0  func main() {
> >                 // Configure logging for a command-line program.
> >       2         log.SetFlags(0)
> >       3         log.SetPrefix("hello: ")
> > 
> >                 // Parse flags.
> >       6         flag.Usage = usage
> >       7         flag.Parse()
> > ------
> > 
> > And Rust
> > ------
> > $ perf probe -x cro3 -L \"cro3::cmd::servo::run_show\"
> > <run_show@/work/cro3/src/cmd/servo.rs:0>
> >       0  fn run_show(args: &ArgsShow) -> Result<()> {
> >       1      let list = ServoList::discover()?;
> >       2      let s = list.find_by_serial(&args.servo)?;
> >       3      if args.json {
> >       4          println!("{s}");
> > ------
> > 
> > And event name are created automatically like below;
> > 
> > $ ./perf probe -x /work/go/example/outyet/main -D 'main.(*Server).poll'
> > p:probe_main/main_Server_poll /work/go/example/outyet/main:0x353040
> > 
> > $ ./perf probe -x cro3 -D \"cro3::cmd::servo::run_show\"
> > p:probe_cro3/cro3_cmd_servo_run_show /work/cro3/target/x86_64-unknown-linux-gnu/debug/cro3:0x197530
> > 
> > We still need some more work, but these shows how perf-probe can work
> > with other languages.
> > 
> > Thank you,
> > 
> > ---
> > 
> > Masami Hiramatsu (Google) (4):
> >       perf-probe: Fix to ignore escaped characters in --lines option
> >       perf-probe: Require '@' prefix for filename always
> >       perf-probe: Introduce quotation marks support
> >       perf-probe: Replace unacceptable characters when generating event name
> > 
> > 
> >  tools/perf/util/probe-event.c  |  136 ++++++++++++++++++++++------------------
> >  tools/perf/util/probe-finder.c |    3 +
> >  tools/perf/util/string.c       |  100 +++++++++++++++++++++++++++++
> >  tools/perf/util/string2.h      |    2 +
> >  4 files changed, 180 insertions(+), 61 deletions(-)
> > 
> > --
> > Masami Hiramatsu (Google) <mhiramat@kernel.org>


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>