linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/6] perf-probe: Improbe non-C language support
@ 2024-11-07 14:52 Masami Hiramatsu (Google)
  2024-11-07 14:52 ` [PATCH v2 1/6] perf-probe: Fix error message for failing to find line range Masami Hiramatsu (Google)
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Masami Hiramatsu (Google) @ 2024-11-07 14:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim
  Cc: Peter Zijlstra, Ingo Molnar, Masami Hiramatsu, Ian Rogers,
	Dima Kogan, Alexander Lobakin, Przemek Kitszel, linux-perf-users,
	linux-kernel

Hi,

Here is the 2nd version of patches for perf probe to improve non-C language
(e.g. Rust, Go) support. The previous version is here;

https://lore.kernel.org/all/173073702882.2098439.13342508872190995896.stgit@mhiramat.roam.corp.google.com/

In this version, Add a new error message [1/6], introduced `@*`
support[3/6], split str*_esq()[4/6], and use dwarf srclang to identify
the source code language[6/6].

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) (6):
      perf-probe: Fix error message for failing to find line range
      perf-probe: Fix to ignore escaped characters in --lines option
      perf-probe: Accept FUNC@* to specify function name explicitly
      perf: Add strpbrk_esq() and strdup_esq() for escape and quote
      perf-probe: Introduce quotation marks support
      perf-probe: Replace unacceptable characters when generating event name


 tools/perf/util/probe-event.c  |  135 ++++++++++++++++++++++++++++------------
 tools/perf/util/probe-event.h  |    3 +
 tools/perf/util/probe-finder.c |   15 ++++
 tools/perf/util/probe-finder.h |    6 +-
 tools/perf/util/string.c       |  100 ++++++++++++++++++++++++++++++
 tools/perf/util/string2.h      |    2 +
 6 files changed, 217 insertions(+), 44 deletions(-)

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/6] perf-probe: Fix error message for failing to find line range
  2024-11-07 14:52 [PATCH v2 0/6] perf-probe: Improbe non-C language support Masami Hiramatsu (Google)
@ 2024-11-07 14:52 ` Masami Hiramatsu (Google)
  2024-11-07 14:52 ` [PATCH v2 2/6] perf-probe: Fix to ignore escaped characters in --lines option Masami Hiramatsu (Google)
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Masami Hiramatsu (Google) @ 2024-11-07 14:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim
  Cc: Peter Zijlstra, Ingo Molnar, Masami Hiramatsu, Ian Rogers,
	Dima Kogan, Alexander Lobakin, Przemek Kitszel, linux-perf-users,
	linux-kernel

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

With --lines option, if perf-probe fails to find the specified line,
it warns as "Debuginfo analysis failed." but this misleads user as
the debuginfo is broken.
Fix this message to "Specified source line(LINESPEC) is not found."
so that user can understand the error correctly.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v2:
  - Added new patch.
---
 tools/perf/util/probe-event.c |   16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index a17c9b8a7a79..6bfe7ead3681 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1036,6 +1036,17 @@ static int _show_one_line(FILE *fp, int l, bool skip, bool show_num)
 	return rv;
 }
 
+static int sprint_line_description(char *sbuf, size_t size, struct line_range *lr)
+{
+	if (!lr->function)
+		return snprintf(sbuf, size, "file: %s, line: %d", lr->file, lr->start);
+
+	if (lr->file)
+		return snprintf(sbuf, size, "function: %s, file:%s, line: %d", lr->function, lr->file, lr->start);
+
+	return snprintf(sbuf, size, "function: %s, line:%d", lr->function, lr->start);
+}
+
 #define show_one_line_with_num(f,l)	_show_one_line(f,l,false,true)
 #define show_one_line(f,l)		_show_one_line(f,l,false,false)
 #define skip_one_line(f,l)		_show_one_line(f,l,true,false)
@@ -1068,6 +1079,8 @@ static int __show_line_range(struct line_range *lr, const char *module,
 		ret = get_alternative_line_range(dinfo, lr, module, user);
 		if (!ret)
 			ret = debuginfo__find_line_range(dinfo, lr);
+		else /* Ignore error, we just failed to find it. */
+			ret = -ENOENT;
 	}
 	if (dinfo->build_id) {
 		build_id__init(&bid, dinfo->build_id, BUILD_ID_SIZE);
@@ -1075,7 +1088,8 @@ static int __show_line_range(struct line_range *lr, const char *module,
 	}
 	debuginfo__delete(dinfo);
 	if (ret == 0 || ret == -ENOENT) {
-		pr_warning("Specified source line is not found.\n");
+		sprint_line_description(sbuf, sizeof(sbuf), lr);
+		pr_warning("Specified source line(%s) is not found.\n", sbuf);
 		return -ENOENT;
 	} else if (ret < 0) {
 		pr_warning("Debuginfo analysis failed.\n");


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/6] perf-probe: Fix to ignore escaped characters in --lines option
  2024-11-07 14:52 [PATCH v2 0/6] perf-probe: Improbe non-C language support Masami Hiramatsu (Google)
  2024-11-07 14:52 ` [PATCH v2 1/6] perf-probe: Fix error message for failing to find line range Masami Hiramatsu (Google)
@ 2024-11-07 14:52 ` Masami Hiramatsu (Google)
  2024-11-07 14:52 ` [PATCH v2 3/6] perf-probe: Accept FUNC@* to specify function name explicitly Masami Hiramatsu (Google)
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Masami Hiramatsu (Google) @ 2024-11-07 14:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim
  Cc: Peter Zijlstra, Ingo Molnar, Masami Hiramatsu, Ian Rogers,
	Dima Kogan, Alexander Lobakin, Przemek Kitszel, linux-perf-users,
	linux-kernel

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 |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 6bfe7ead3681..edc205e4b0ee 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1369,7 +1369,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';
 
@@ -1410,7 +1410,7 @@ 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);
@@ -1419,7 +1419,7 @@ int parse_line_range_desc(const char *arg, struct line_range *lr)
 			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;


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/6] perf-probe: Accept FUNC@* to specify function name explicitly
  2024-11-07 14:52 [PATCH v2 0/6] perf-probe: Improbe non-C language support Masami Hiramatsu (Google)
  2024-11-07 14:52 ` [PATCH v2 1/6] perf-probe: Fix error message for failing to find line range Masami Hiramatsu (Google)
  2024-11-07 14:52 ` [PATCH v2 2/6] perf-probe: Fix to ignore escaped characters in --lines option Masami Hiramatsu (Google)
@ 2024-11-07 14:52 ` Masami Hiramatsu (Google)
  2024-11-07 14:52 ` [PATCH v2 4/6] perf: Add strpbrk_esq() and strdup_esq() for escape and quote Masami Hiramatsu (Google)
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Masami Hiramatsu (Google) @ 2024-11-07 14:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim
  Cc: Peter Zijlstra, Ingo Molnar, Masami Hiramatsu, Ian Rogers,
	Dima Kogan, Alexander Lobakin, Przemek Kitszel, linux-perf-users,
	linux-kernel

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

In Golang, the function name will have the '.', and perf probe
misinterpret it is a file name. To mitigate this situation, introduce
`function@*` so that user can explicitly specify that is a function
name.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 Changes in v2:
  - Added new patch.
---
 tools/perf/util/probe-event.c |   22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index edc205e4b0ee..777ef00f0d3f 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1357,6 +1357,8 @@ static bool is_c_func_name(const char *name)
  *
  *         SRC[:SLN[+NUM|-ELN]]
  *         FNC[@SRC][:SLN[+NUM|-ELN]]
+ *
+ * FNC@SRC accepts `FNC@*` which forcibly specify FNC as function name.
  */
 int parse_line_range_desc(const char *arg, struct line_range *lr)
 {
@@ -1412,13 +1414,21 @@ int parse_line_range_desc(const char *arg, struct line_range *lr)
 
 	file = strpbrk_esc(name, "@");
 	if (file) {
-		*file = '\0';
-		lr->file = strdup(++file);
-		if (lr->file == NULL) {
-			err = -ENOMEM;
+		*file++ = '\0';
+		if (strcmp(file, "*")) {
+			lr->file = strdup_esc(file);
+			if (lr->file == NULL) {
+				err = -ENOMEM;
+				goto err;
+			}
+		}
+		if (*name != '\0')
+			lr->function = name;
+		if (!lr->function && !lr->file) {
+			semantic_error("Only '@*' is not allowed.\n");
+			err = -EINVAL;
 			goto err;
 		}
-		lr->function = name;
 	} else if (strpbrk_esc(name, "/."))
 		lr->file = name;
 	else if (is_c_func_name(name))/* We reuse it for checking funcname */
@@ -1619,6 +1629,8 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 				semantic_error("SRC@SRC is not allowed.\n");
 				return -EINVAL;
 			}
+			if (!strcmp(arg, "*"))
+				break;
 			pp->file = strdup_esc(arg);
 			if (pp->file == NULL)
 				return -ENOMEM;


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 4/6] perf: Add strpbrk_esq() and strdup_esq() for escape and quote
  2024-11-07 14:52 [PATCH v2 0/6] perf-probe: Improbe non-C language support Masami Hiramatsu (Google)
                   ` (2 preceding siblings ...)
  2024-11-07 14:52 ` [PATCH v2 3/6] perf-probe: Accept FUNC@* to specify function name explicitly Masami Hiramatsu (Google)
@ 2024-11-07 14:52 ` Masami Hiramatsu (Google)
  2024-11-07 14:52 ` [PATCH v2 5/6] perf-probe: Introduce quotation marks support Masami Hiramatsu (Google)
  2024-11-07 14:52 ` [PATCH v2 6/6] perf-probe: Replace unacceptable characters when generating event name Masami Hiramatsu (Google)
  5 siblings, 0 replies; 10+ messages in thread
From: Masami Hiramatsu (Google) @ 2024-11-07 14:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim
  Cc: Peter Zijlstra, Ingo Molnar, Masami Hiramatsu, Ian Rogers,
	Dima Kogan, Alexander Lobakin, Przemek Kitszel, linux-perf-users,
	linux-kernel

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

strpbrk_esq() and strdup_esq() are new variants for strpbrk() and
strdup() which handles escaped characters and quoted strings.

- strpbrk_esq() searches specified set of characters but ignores the
  escaped characters and quoted strings.
  e.g. strpbrk_esq("'quote\d' \queue quiz", "qd") returns "quiz".

- strdup_esq() duplicates string but removes backslash and quotes which
  is used for quotation. It also keeps the string (including backslash)
  in the quoted part.
  e.g. strdup_esq("'quote\d' \queue quiz") returns "quote\d queue quiz".

The (single, double) quotes in the quoted part should be escaped by
backslash. In this case, strdup_esq() removes that backslash.
The same quotes must be paired. If you use double quotation, you need
to use the double quotation to close the quoted part.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 Changes in v2:
  - Added new patch.
---
 tools/perf/util/string.c  |  100 +++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/string2.h |    2 +
 2 files changed, 102 insertions(+)

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 5/6] perf-probe: Introduce quotation marks support
  2024-11-07 14:52 [PATCH v2 0/6] perf-probe: Improbe non-C language support Masami Hiramatsu (Google)
                   ` (3 preceding siblings ...)
  2024-11-07 14:52 ` [PATCH v2 4/6] perf: Add strpbrk_esq() and strdup_esq() for escape and quote Masami Hiramatsu (Google)
@ 2024-11-07 14:52 ` Masami Hiramatsu (Google)
  2024-11-07 14:52 ` [PATCH v2 6/6] perf-probe: Replace unacceptable characters when generating event name Masami Hiramatsu (Google)
  5 siblings, 0 replies; 10+ messages in thread
From: Masami Hiramatsu (Google) @ 2024-11-07 14:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim
  Cc: Peter Zijlstra, Ingo Molnar, Masami Hiramatsu, Ian Rogers,
	Dima Kogan, Alexander Lobakin, Przemek Kitszel, linux-perf-users,
	linux-kernel

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>
---
 Changes in v2:
  - split str*_esq() functions from this patch.
  - update for the changes in previous patches.
---
 tools/perf/util/probe-event.c |   75 ++++++++++++++++++++++-------------------
 1 file changed, 41 insertions(+), 34 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 777ef00f0d3f..31e257c84cd1 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1076,6 +1076,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);
@@ -1359,30 +1360,37 @@ static bool is_c_func_name(const char *name)
  *         FNC[@SRC][:SLN[+NUM|-ELN]]
  *
  * FNC@SRC accepts `FNC@*` which forcibly specify FNC as function name.
+ * 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';
+	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;
 
@@ -1406,42 +1414,41 @@ 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';
-		if (strcmp(file, "*")) {
-			lr->file = strdup_esc(file);
+	p = strpbrk_esq(buf, "@");
+	if (p) {
+		*p++ = '\0';
+		if (strcmp(p, "*")) {
+			lr->file = strdup_esq(p);
 			if (lr->file == NULL) {
 				err = -ENOMEM;
 				goto err;
 			}
 		}
-		if (*name != '\0')
-			lr->function = name;
+		if (*buf != '\0')
+			lr->function = strdup_esq(buf);
 		if (!lr->function && !lr->file) {
 			semantic_error("Only '@*' is not allowed.\n");
 			err = -EINVAL;
 			goto err;
 		}
-	} 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 if (strpbrk_esq(buf, "/."))
+		lr->file = strdup_esq(buf);
+	else if (is_c_func_name(buf))/* We reuse it for checking funcname */
+		lr->function = strdup_esq(buf);
 	else {	/* Invalid name */
-		semantic_error("'%s' is not a valid function name.\n", name);
+		semantic_error("'%s' is not a valid function name.\n", buf);
 		err = -EINVAL;
 		goto err;
 	}
 
-	return 0;
 err:
-	free(name);
+	free(buf);
 	return err;
 }
 
@@ -1449,19 +1456,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;
 
@@ -1500,7 +1507,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 != '@') {
@@ -1514,7 +1521,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';
@@ -1555,7 +1562,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 			file_spec = true;
 	}
 
-	ptr = strpbrk_esc(arg, ";:+@%");
+	ptr = strpbrk_esq(arg, ";:+@%");
 	if (ptr) {
 		nc = *ptr;
 		*ptr++ = '\0';
@@ -1564,7 +1571,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;
 	}
@@ -1602,7 +1609,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';
@@ -1631,7 +1638,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 			}
 			if (!strcmp(arg, "*"))
 				break;
-			pp->file = strdup_esc(arg);
+			pp->file = strdup_esq(arg);
 			if (pp->file == NULL)
 				return -ENOMEM;
 			break;


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 6/6] perf-probe: Replace unacceptable characters when generating event name
  2024-11-07 14:52 [PATCH v2 0/6] perf-probe: Improbe non-C language support Masami Hiramatsu (Google)
                   ` (4 preceding siblings ...)
  2024-11-07 14:52 ` [PATCH v2 5/6] perf-probe: Introduce quotation marks support Masami Hiramatsu (Google)
@ 2024-11-07 14:52 ` Masami Hiramatsu (Google)
  2024-11-12 17:28   ` Arnaldo Carvalho de Melo
  5 siblings, 1 reply; 10+ messages in thread
From: Masami Hiramatsu (Google) @ 2024-11-07 14:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim
  Cc: Peter Zijlstra, Ingo Molnar, Masami Hiramatsu, Ian Rogers,
	Dima Kogan, Alexander Lobakin, Przemek Kitszel, linux-perf-users,
	linux-kernel

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.
Note that this language checking depends on the debuginfo. So without the
debuginfo, perf probe will always replaces unacceptable characters with
'_'.

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>
---
 Changes in v2:
  - Check dwarf language instead of checking wildcards.
---
 tools/perf/util/probe-event.c  |   32 +++++++++++++++++++++++++-------
 tools/perf/util/probe-event.h  |    3 ++-
 tools/perf/util/probe-finder.c |   15 +++++++++++++++
 tools/perf/util/probe-finder.h |    6 +++++-
 4 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 31e257c84cd1..9eaf0fc7975a 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2771,7 +2771,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 not_C_symname)
 {
 	int i, ret;
 	char *p, *nbase;
@@ -2782,10 +2782,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 (not_C_symname) {
+		/* 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++);
+	} else {
+		/* Cut off the dot suffixes (e.g. .const, .isra) and version suffixes */
+		p = strpbrk(nbase, ".@");
+		if (p && p != nbase)
+			*p = '\0';
+	}
 
 	/* Try no suffix number */
 	ret = e_snprintf(buf, len, "%s%s", nbase, ret_event ? "__return" : "");
@@ -2874,6 +2888,7 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
 				       bool allow_suffix)
 {
 	const char *event, *group;
+	bool not_C_symname = true;
 	char buf[64];
 	int ret;
 
@@ -2888,8 +2903,10 @@ 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;
+			not_C_symname = !is_known_C_lang(tev->lang);
+		}
 	}
 	if (pev->group && !pev->sdt)
 		group = pev->group;
@@ -2900,7 +2917,8 @@ 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,
+				 not_C_symname);
 	if (ret < 0)
 		return ret;
 
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 7e3b6c3d1f74..6516105f43e2 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -57,7 +57,8 @@ struct probe_trace_event {
 	char				*event;	/* Event name */
 	char				*group;	/* Group name */
 	struct probe_trace_point	point;	/* Trace point */
-	int				nargs;	/* Number of args */
+	int					nargs;	/* Number of args */
+	int					lang;	/* Dwarf language code */
 	bool				uprobes;	/* uprobes only */
 	struct probe_trace_arg		*args;	/* Arguments */
 };
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 630e16c54ed5..13ff45d3d6a4 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -35,6 +35,19 @@
 /* Kprobe tracer basic type is up to u64 */
 #define MAX_BASIC_TYPE_BITS	64
 
+bool is_known_C_lang(int lang)
+{
+	switch (lang) {
+	case DW_LANG_C89:
+	case DW_LANG_C:
+	case DW_LANG_C99:
+	case DW_LANG_C11:
+		return true;
+	default:
+		return false;
+	}
+}
+
 /*
  * Probe finder related functions
  */
@@ -1272,6 +1285,8 @@ static int add_probe_trace_event(Dwarf_Die *sc_die, struct probe_finder *pf)
 		ret = -ENOMEM;
 		goto end;
 	}
+	tev->lang = dwarf_srclang(dwarf_diecu(sc_die, &pf->cu_die,
+										  NULL, NULL));
 
 	pr_debug("Probe point found: %s+%lu\n", tev->point.symbol,
 		 tev->point.offset);
diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
index 3add5ff516e1..04a52d5fd568 100644
--- a/tools/perf/util/probe-finder.h
+++ b/tools/perf/util/probe-finder.h
@@ -26,6 +26,9 @@ static inline int is_c_varname(const char *name)
 #include "dwarf-aux.h"
 #include "debuginfo.h"
 
+/* Check the language code is known C */
+bool is_known_C_lang(int lang);
+
 /* Find probe_trace_events specified by perf_probe_event from debuginfo */
 int debuginfo__find_trace_events(struct debuginfo *dbg,
 				 struct perf_probe_event *pev,
@@ -103,7 +106,8 @@ struct line_finder {
 	Dwarf_Die		sp_die;
 	int			found;
 };
-
+#else
+#define is_known_C_lang(lang) (false)
 #endif /* HAVE_DWARF_SUPPORT */
 
 #endif /*_PROBE_FINDER_H */


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 6/6] perf-probe: Replace unacceptable characters when generating event name
  2024-11-07 14:52 ` [PATCH v2 6/6] perf-probe: Replace unacceptable characters when generating event name Masami Hiramatsu (Google)
@ 2024-11-12 17:28   ` Arnaldo Carvalho de Melo
  2024-11-12 23:38     ` Masami Hiramatsu
  0 siblings, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-11-12 17:28 UTC (permalink / raw)
  To: Masami Hiramatsu (Google)
  Cc: Namhyung Kim, Peter Zijlstra, Ingo Molnar, Ian Rogers, Dima Kogan,
	Alexander Lobakin, Przemek Kitszel, linux-perf-users,
	linux-kernel

On Thu, Nov 07, 2024 at 11:52:58PM +0900, Masami Hiramatsu (Google) wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

This last patch isn't applying, to make progress I appled 1-5, please
take a look at the tmp.perf-tools-next branch at:

https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git tmp.perf-tools-next

https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/log/?h=tmp.perf-tools-next

- Arnaldo

Applying: perf-probe: Replace unacceptable characters when generating event name
error: patch failed: tools/perf/util/probe-event.c:2874
error: tools/perf/util/probe-event.c: patch does not apply
error: patch failed: tools/perf/util/probe-finder.h:103
error: tools/perf/util/probe-finder.h: patch does not apply
Patch failed at 0006 perf-probe: Replace unacceptable characters when generating event name
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
⬢ [acme@toolbox perf-tools-next]$ 

 
> 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.
> Note that this language checking depends on the debuginfo. So without the
> debuginfo, perf probe will always replaces unacceptable characters with
> '_'.
> 
> 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>
> ---
>  Changes in v2:
>   - Check dwarf language instead of checking wildcards.
> ---
>  tools/perf/util/probe-event.c  |   32 +++++++++++++++++++++++++-------
>  tools/perf/util/probe-event.h  |    3 ++-
>  tools/perf/util/probe-finder.c |   15 +++++++++++++++
>  tools/perf/util/probe-finder.h |    6 +++++-
>  4 files changed, 47 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 31e257c84cd1..9eaf0fc7975a 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2771,7 +2771,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 not_C_symname)
>  {
>  	int i, ret;
>  	char *p, *nbase;
> @@ -2782,10 +2782,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 (not_C_symname) {
> +		/* 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++);
> +	} else {
> +		/* Cut off the dot suffixes (e.g. .const, .isra) and version suffixes */
> +		p = strpbrk(nbase, ".@");
> +		if (p && p != nbase)
> +			*p = '\0';
> +	}
>  
>  	/* Try no suffix number */
>  	ret = e_snprintf(buf, len, "%s%s", nbase, ret_event ? "__return" : "");
> @@ -2874,6 +2888,7 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
>  				       bool allow_suffix)
>  {
>  	const char *event, *group;
> +	bool not_C_symname = true;
>  	char buf[64];
>  	int ret;
>  
> @@ -2888,8 +2903,10 @@ 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;
> +			not_C_symname = !is_known_C_lang(tev->lang);
> +		}
>  	}
>  	if (pev->group && !pev->sdt)
>  		group = pev->group;
> @@ -2900,7 +2917,8 @@ 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,
> +				 not_C_symname);
>  	if (ret < 0)
>  		return ret;
>  
> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index 7e3b6c3d1f74..6516105f43e2 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -57,7 +57,8 @@ struct probe_trace_event {
>  	char				*event;	/* Event name */
>  	char				*group;	/* Group name */
>  	struct probe_trace_point	point;	/* Trace point */
> -	int				nargs;	/* Number of args */
> +	int					nargs;	/* Number of args */
> +	int					lang;	/* Dwarf language code */
>  	bool				uprobes;	/* uprobes only */
>  	struct probe_trace_arg		*args;	/* Arguments */
>  };
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 630e16c54ed5..13ff45d3d6a4 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -35,6 +35,19 @@
>  /* Kprobe tracer basic type is up to u64 */
>  #define MAX_BASIC_TYPE_BITS	64
>  
> +bool is_known_C_lang(int lang)
> +{
> +	switch (lang) {
> +	case DW_LANG_C89:
> +	case DW_LANG_C:
> +	case DW_LANG_C99:
> +	case DW_LANG_C11:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
>  /*
>   * Probe finder related functions
>   */
> @@ -1272,6 +1285,8 @@ static int add_probe_trace_event(Dwarf_Die *sc_die, struct probe_finder *pf)
>  		ret = -ENOMEM;
>  		goto end;
>  	}
> +	tev->lang = dwarf_srclang(dwarf_diecu(sc_die, &pf->cu_die,
> +										  NULL, NULL));
>  
>  	pr_debug("Probe point found: %s+%lu\n", tev->point.symbol,
>  		 tev->point.offset);
> diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
> index 3add5ff516e1..04a52d5fd568 100644
> --- a/tools/perf/util/probe-finder.h
> +++ b/tools/perf/util/probe-finder.h
> @@ -26,6 +26,9 @@ static inline int is_c_varname(const char *name)
>  #include "dwarf-aux.h"
>  #include "debuginfo.h"
>  
> +/* Check the language code is known C */
> +bool is_known_C_lang(int lang);
> +
>  /* Find probe_trace_events specified by perf_probe_event from debuginfo */
>  int debuginfo__find_trace_events(struct debuginfo *dbg,
>  				 struct perf_probe_event *pev,
> @@ -103,7 +106,8 @@ struct line_finder {
>  	Dwarf_Die		sp_die;
>  	int			found;
>  };
> -
> +#else
> +#define is_known_C_lang(lang) (false)
>  #endif /* HAVE_DWARF_SUPPORT */
>  
>  #endif /*_PROBE_FINDER_H */

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 6/6] perf-probe: Replace unacceptable characters when generating event name
  2024-11-12 17:28   ` Arnaldo Carvalho de Melo
@ 2024-11-12 23:38     ` Masami Hiramatsu
  2024-11-13  0:12       ` Masami Hiramatsu
  0 siblings, 1 reply; 10+ messages in thread
From: Masami Hiramatsu @ 2024-11-12 23:38 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Peter Zijlstra, Ingo Molnar, Ian Rogers, Dima Kogan,
	Alexander Lobakin, Przemek Kitszel, linux-perf-users,
	linux-kernel

On Tue, 12 Nov 2024 14:28:55 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> On Thu, Nov 07, 2024 at 11:52:58PM +0900, Masami Hiramatsu (Google) wrote:
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> This last patch isn't applying, to make progress I appled 1-5, please
> take a look at the tmp.perf-tools-next branch at:
> 

OK, let me resend on top of that branch.

Thank you!

> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git tmp.perf-tools-next
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/log/?h=tmp.perf-tools-next
> 
> - Arnaldo
> 
> Applying: perf-probe: Replace unacceptable characters when generating event name
> error: patch failed: tools/perf/util/probe-event.c:2874
> error: tools/perf/util/probe-event.c: patch does not apply
> error: patch failed: tools/perf/util/probe-finder.h:103
> error: tools/perf/util/probe-finder.h: patch does not apply
> Patch failed at 0006 perf-probe: Replace unacceptable characters when generating event name
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> hint: When you have resolved this problem, run "git am --continue".
> hint: If you prefer to skip this patch, run "git am --skip" instead.
> hint: To restore the original branch and stop patching, run "git am --abort".
> hint: Disable this message with "git config advice.mergeConflict false"
> ⬢ [acme@toolbox perf-tools-next]$ 
> 
>  
> > 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.
> > Note that this language checking depends on the debuginfo. So without the
> > debuginfo, perf probe will always replaces unacceptable characters with
> > '_'.
> > 
> > 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>
> > ---
> >  Changes in v2:
> >   - Check dwarf language instead of checking wildcards.
> > ---
> >  tools/perf/util/probe-event.c  |   32 +++++++++++++++++++++++++-------
> >  tools/perf/util/probe-event.h  |    3 ++-
> >  tools/perf/util/probe-finder.c |   15 +++++++++++++++
> >  tools/perf/util/probe-finder.h |    6 +++++-
> >  4 files changed, 47 insertions(+), 9 deletions(-)
> > 
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 31e257c84cd1..9eaf0fc7975a 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -2771,7 +2771,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 not_C_symname)
> >  {
> >  	int i, ret;
> >  	char *p, *nbase;
> > @@ -2782,10 +2782,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 (not_C_symname) {
> > +		/* 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++);
> > +	} else {
> > +		/* Cut off the dot suffixes (e.g. .const, .isra) and version suffixes */
> > +		p = strpbrk(nbase, ".@");
> > +		if (p && p != nbase)
> > +			*p = '\0';
> > +	}
> >  
> >  	/* Try no suffix number */
> >  	ret = e_snprintf(buf, len, "%s%s", nbase, ret_event ? "__return" : "");
> > @@ -2874,6 +2888,7 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
> >  				       bool allow_suffix)
> >  {
> >  	const char *event, *group;
> > +	bool not_C_symname = true;
> >  	char buf[64];
> >  	int ret;
> >  
> > @@ -2888,8 +2903,10 @@ 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;
> > +			not_C_symname = !is_known_C_lang(tev->lang);
> > +		}
> >  	}
> >  	if (pev->group && !pev->sdt)
> >  		group = pev->group;
> > @@ -2900,7 +2917,8 @@ 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,
> > +				 not_C_symname);
> >  	if (ret < 0)
> >  		return ret;
> >  
> > diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> > index 7e3b6c3d1f74..6516105f43e2 100644
> > --- a/tools/perf/util/probe-event.h
> > +++ b/tools/perf/util/probe-event.h
> > @@ -57,7 +57,8 @@ struct probe_trace_event {
> >  	char				*event;	/* Event name */
> >  	char				*group;	/* Group name */
> >  	struct probe_trace_point	point;	/* Trace point */
> > -	int				nargs;	/* Number of args */
> > +	int					nargs;	/* Number of args */
> > +	int					lang;	/* Dwarf language code */
> >  	bool				uprobes;	/* uprobes only */
> >  	struct probe_trace_arg		*args;	/* Arguments */
> >  };
> > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > index 630e16c54ed5..13ff45d3d6a4 100644
> > --- a/tools/perf/util/probe-finder.c
> > +++ b/tools/perf/util/probe-finder.c
> > @@ -35,6 +35,19 @@
> >  /* Kprobe tracer basic type is up to u64 */
> >  #define MAX_BASIC_TYPE_BITS	64
> >  
> > +bool is_known_C_lang(int lang)
> > +{
> > +	switch (lang) {
> > +	case DW_LANG_C89:
> > +	case DW_LANG_C:
> > +	case DW_LANG_C99:
> > +	case DW_LANG_C11:
> > +		return true;
> > +	default:
> > +		return false;
> > +	}
> > +}
> > +
> >  /*
> >   * Probe finder related functions
> >   */
> > @@ -1272,6 +1285,8 @@ static int add_probe_trace_event(Dwarf_Die *sc_die, struct probe_finder *pf)
> >  		ret = -ENOMEM;
> >  		goto end;
> >  	}
> > +	tev->lang = dwarf_srclang(dwarf_diecu(sc_die, &pf->cu_die,
> > +										  NULL, NULL));
> >  
> >  	pr_debug("Probe point found: %s+%lu\n", tev->point.symbol,
> >  		 tev->point.offset);
> > diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
> > index 3add5ff516e1..04a52d5fd568 100644
> > --- a/tools/perf/util/probe-finder.h
> > +++ b/tools/perf/util/probe-finder.h
> > @@ -26,6 +26,9 @@ static inline int is_c_varname(const char *name)
> >  #include "dwarf-aux.h"
> >  #include "debuginfo.h"
> >  
> > +/* Check the language code is known C */
> > +bool is_known_C_lang(int lang);
> > +
> >  /* Find probe_trace_events specified by perf_probe_event from debuginfo */
> >  int debuginfo__find_trace_events(struct debuginfo *dbg,
> >  				 struct perf_probe_event *pev,
> > @@ -103,7 +106,8 @@ struct line_finder {
> >  	Dwarf_Die		sp_die;
> >  	int			found;
> >  };
> > -
> > +#else
> > +#define is_known_C_lang(lang) (false)
> >  #endif /* HAVE_DWARF_SUPPORT */
> >  
> >  #endif /*_PROBE_FINDER_H */


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 6/6] perf-probe: Replace unacceptable characters when generating event name
  2024-11-12 23:38     ` Masami Hiramatsu
@ 2024-11-13  0:12       ` Masami Hiramatsu
  0 siblings, 0 replies; 10+ messages in thread
From: Masami Hiramatsu @ 2024-11-13  0:12 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Masami Hiramatsu
  Cc: Namhyung Kim, Peter Zijlstra, Ingo Molnar, Ian Rogers, Dima Kogan,
	Alexander Lobakin, Przemek Kitszel, linux-perf-users,
	linux-kernel

Hi Arnaldo,

On Wed, 13 Nov 2024 08:38:47 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> On Tue, 12 Nov 2024 14:28:55 -0300
> Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> > On Thu, Nov 07, 2024 at 11:52:58PM +0900, Masami Hiramatsu (Google) wrote:
> > > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > 
> > This last patch isn't applying, to make progress I appled 1-5, please
> > take a look at the tmp.perf-tools-next branch at:
> > 
> 
> OK, let me resend on top of that branch.
> 
> Thank you!
> 
> > https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git tmp.perf-tools-next
> > 

It seems that this branch can not be compiled without below patch.
(in LIBCAPSTONE=n but LIBLLVM=y case.)

diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
index df6c172c9c7f..88f71f6e5e70 100644
--- a/tools/perf/util/disasm.c
+++ b/tools/perf/util/disasm.c
@@ -1422,14 +1422,14 @@ read_symbol(const char *filename, struct map *map, struct symbol *sym,
 	free(buf);
 	return NULL;
 }
-#else // defined(HAVE_LIBCAPSTONE_SUPPORT) || defined(HAVE_LIBLLVM_SUPPORT)
+#endif // defined(HAVE_LIBCAPSTONE_SUPPORT) || defined(HAVE_LIBLLVM_SUPPORT)
+
 static void symbol__disassembler_missing(const char *disassembler, const char *filename,
 					 struct symbol *sym)
 {
 	pr_debug("The %s disassembler isn't linked in for %s in %s\n",
 		 disassembler, sym->name, filename);
 }
-#endif
 
 #ifdef HAVE_LIBCAPSTONE_SUPPORT
 static void print_capstone_detail(cs_insn *insn, char *buf, size_t len,
@@ -1724,7 +1724,8 @@ static int symbol__disassemble_capstone(char *filename, struct symbol *sym,
 }
 #else // HAVE_LIBCAPSTONE_SUPPORT
 static int symbol__disassemble_capstone(char *filename, struct symbol *sym,
-					struct annotate_args *args)
+					struct annotate_args *args __maybe_unused)
+{
 	symbol__disassembler_missing("capstone", filename, sym);
 	return -1;
 }

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-11-13  0:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-07 14:52 [PATCH v2 0/6] perf-probe: Improbe non-C language support Masami Hiramatsu (Google)
2024-11-07 14:52 ` [PATCH v2 1/6] perf-probe: Fix error message for failing to find line range Masami Hiramatsu (Google)
2024-11-07 14:52 ` [PATCH v2 2/6] perf-probe: Fix to ignore escaped characters in --lines option Masami Hiramatsu (Google)
2024-11-07 14:52 ` [PATCH v2 3/6] perf-probe: Accept FUNC@* to specify function name explicitly Masami Hiramatsu (Google)
2024-11-07 14:52 ` [PATCH v2 4/6] perf: Add strpbrk_esq() and strdup_esq() for escape and quote Masami Hiramatsu (Google)
2024-11-07 14:52 ` [PATCH v2 5/6] perf-probe: Introduce quotation marks support Masami Hiramatsu (Google)
2024-11-07 14:52 ` [PATCH v2 6/6] perf-probe: Replace unacceptable characters when generating event name Masami Hiramatsu (Google)
2024-11-12 17:28   ` Arnaldo Carvalho de Melo
2024-11-12 23:38     ` Masami Hiramatsu
2024-11-13  0:12       ` Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).