[PATCH 3/5] perf/sdt/x86: Move OP parser to tools/perf/arch/x86/

[Ravi Bangoria]

SDT marker argument is in N@OP format. N is the size of argument and
OP is the actual assembly operand. OP is arch dependent component and
hence it's parsing logic also should be placed under tools/perf/arch/.

Signed-off-by: Ravi Bangoria <ravi.bangoria@xxxxxxxxxxxxxxxxxx>
---
 tools/perf/arch/x86/util/perf_regs.c |  93 ++++++++++++++++++++++++-
 tools/perf/util/perf_regs.c          |   9 ++-
 tools/perf/util/perf_regs.h          |   7 +-
 tools/perf/util/probe-file.c         | 127 +++++++++--------------------------
 4 files changed, 134 insertions(+), 102 deletions(-)

diff --git a/tools/perf/arch/x86/util/perf_regs.c b/tools/perf/arch/x86/util/perf_regs.c
index d8a8dcf..34fcb0d 100644
--- a/tools/perf/arch/x86/util/perf_regs.c
+++ b/tools/perf/arch/x86/util/perf_regs.c
@@ -3,6 +3,7 @@
 #include "../../perf.h"
 #include "../../util/util.h"
 #include "../../util/perf_regs.h"
+#include "../../util/debug.h"
 
 const struct sample_reg sample_reg_masks[] = {
 	SMPL_REG(AX, PERF_REG_X86_AX),
@@ -87,7 +88,16 @@ static const struct sdt_name_reg sdt_reg_renamings[] = {
 	SDT_NAME_REG_END,
 };
 
-int sdt_rename_register(char **pdesc, char *old_name)
+bool arch_sdt_probe_arg_supp(void)
+{
+	return true;
+}
+
+/*
+ * The table sdt_reg_renamings is used for adjusting gcc/gas-generated
+ * registers before filling the uprobe tracer interface.
+ */
+static int sdt_rename_register(char **pdesc, char *old_name)
 {
 	const struct sdt_name_reg *rnames = sdt_reg_renamings;
 	char *new_desc, *old_desc = *pdesc;
@@ -129,3 +139,84 @@ int sdt_rename_register(char **pdesc, char *old_name)
 
 	return 0;
 }
+
+/*
+ * x86 specific implementation
+ * return value:
+ *	<0 : error
+ *	 0 : success
+ *	>0 : skip
+ */
+int arch_sdt_probe_parse_op(char **desc, const char **prefix)
+{
+	char *tmp;
+	int ret = 0;
+
+	/*
+	 * The uprobe tracer format does not support all the addressing
+	 * modes (notably: in x86 the scaled mode); so, we detect ','
+	 * characters, if there is just one, there is no use converting
+	 * the sdt arg into a uprobe one.
+	 *
+	 * Also it does not support constants; if we find one in the
+	 * current argument, let's skip the argument.
+	 */
+	if (strchr(*desc, ',') || strchr(*desc, '$')) {
+		pr_debug4("Skipping unsupported SDT argument; %s\n", *desc);
+		return 1;
+	}
+
+	/*
+	 * If the argument addressing mode is indirect, we must check
+	 * a few things...
+	 */
+	tmp = strchr(*desc, '(');
+	if (tmp) {
+		int j;
+
+		/*
+		 * ...if the addressing mode is indirect with a
+		 * positive offset (ex.: "1608(%ax)"), we need to add
+		 * a '+' prefix so as to be compliant with uprobe
+		 * format.
+		 */
+		if ((*desc)[0] != '+' && (*desc)[0] != '-')
+			*prefix = ((*desc)[0] == '(') ? "+0" : "+";
+
+		/*
+		 * ...or if the addressing mode is indirect with a symbol
+		 * as offset, the argument will not be supported by
+		 * the uprobe tracer format; so, let's skip this one.
+		 */
+		for (j = 0; j < tmp - *desc; j++) {
+			if ((*desc)[j] != '+' && (*desc)[j] != '-' &&
+			    !isdigit((*desc)[j])) {
+				pr_debug4("Skipping unsupported SDT argument; "
+					"%s\n", *desc);
+				return 1;
+			}
+		}
+	}
+
+	/*
+	 * The uprobe parser does not support all gas register names;
+	 * so, we have to replace them (ex. for x86_64: %rax -> %ax);
+	 * the loop below looks for the register names (starting with
+	 * a '%' and tries to perform the needed renamings.
+	 */
+	tmp = strchr(*desc, '%');
+	while (tmp) {
+		size_t offset = tmp - *desc;
+
+		ret = sdt_rename_register(desc, *desc + offset);
+		if (ret < 0)
+			return ret;
+
+		/*
+		 * The *desc pointer might have changed; so, let's not
+		 * try to reuse tmp for next lookup
+		 */
+		tmp = strchr(*desc + offset + 1, '%');
+	}
+	return 0;
+}
diff --git a/tools/perf/util/perf_regs.c b/tools/perf/util/perf_regs.c
index a37e593..f2b3d0d 100644
--- a/tools/perf/util/perf_regs.c
+++ b/tools/perf/util/perf_regs.c
@@ -6,8 +6,13 @@ const struct sample_reg __weak sample_reg_masks[] = {
 	SMPL_REG_END
 };
 
-int __weak sdt_rename_register(char **pdesc __maybe_unused,
-			char *old_name __maybe_unused)
+bool __weak arch_sdt_probe_arg_supp(void)
+{
+	return false;
+}
+
+int __weak arch_sdt_probe_parse_op(char **op_ptr __maybe_unused,
+				   const char **prefix __maybe_unused)
 {
 	return 0;
 }
diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
index 7544a15..86a2961 100644
--- a/tools/perf/util/perf_regs.h
+++ b/tools/perf/util/perf_regs.h
@@ -15,11 +15,8 @@ struct sample_reg {
 
 extern const struct sample_reg sample_reg_masks[];
 
-/*
- * The table sdt_reg_renamings is used for adjusting gcc/gas-generated
- * registers before filling the uprobe tracer interface.
- */
-int sdt_rename_register(char **pdesc, char *old_name);
+bool arch_sdt_probe_arg_supp(void);
+int arch_sdt_probe_parse_op(char **op_ptr, const char **prefix);
 
 #ifdef HAVE_PERF_REGS_SUPPORT
 #include <perf_regs.h>
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index d8a169e..38eca3c 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -693,6 +693,25 @@ static const char * const type_to_suffix[] = {
 	"", ":u8", ":u16", "", ":u32", "", "", "", ":u64"
 };
 
+/*
+ * Isolate the string number and convert it into a decimal value;
+ * this will be an index to get suffix of the uprobe name (defining
+ * the type)
+ */
+static int sdt_probe_parse_n(char *n_ptr, const char **suffix)
+{
+	long type_idx;
+
+	type_idx = strtol(n_ptr, NULL, 10);
+	if (type_idx < -8 || type_idx > 8) {
+		pr_debug4("Failed to get a valid sdt type\n");
+		return -1;
+	}
+
+	*suffix = type_to_suffix[type_idx + 8];
+	return 0;
+}
+
 static int synthesize_sdt_probe_arg(struct strbuf *buf, int i, const char *arg)
 {
 	char *tmp, *desc = strdup(arg);
@@ -704,109 +723,29 @@ static int synthesize_sdt_probe_arg(struct strbuf *buf, int i, const char *arg)
 		return ret;
 	}
 
+	/*
+	 * Argument is in N@OP format. N is size of the argument and OP is
+	 * the actual assembly operand. N can be omitted; in that case
+	 * argument is just OP(without @).
+	 */
 	tmp = strchr(desc, '@');
 	if (tmp) {
-		long type_idx;
-		/*
-		 * Isolate the string number and convert it into a
-		 * binary value; this will be an index to get suffix
-		 * of the uprobe name (defining the type)
-		 */
 		tmp[0] = '\0';
-		type_idx = strtol(desc, NULL, 10);
-		/* Check that the conversion went OK */
-		if (type_idx == LONG_MIN || type_idx == LONG_MAX) {
-			pr_debug4("Failed to parse sdt type\n");
-			goto error;
-		}
-		/* Check that the converted value is OK */
-		if (type_idx < -8 || type_idx > 8) {
-			pr_debug4("Failed to get a valid sdt type\n");
-			goto error;
-		}
-		suffix = type_to_suffix[type_idx + 8];
-		/* Get rid of the sdt prefix which is now useless */
 		tmp++;
-		memmove(desc, tmp, strlen(tmp) + 1);
-	}
 
-	/*
-	 * The uprobe tracer format does not support all the
-	 * addressing modes (notably: in x86 the scaled mode); so, we
-	 * detect ',' characters, if there is just one, there is no
-	 * use converting the sdt arg into a uprobe one.
-	 */
-	if (strchr(desc, ',')) {
-		pr_debug4("Skipping unsupported SDT argument; %s\n", desc);
-		goto out;
-	}
-
-	/*
-	 * If the argument addressing mode is indirect, we must check
-	 * a few things...
-	 */
-	tmp = strchr(desc, '(');
-	if (tmp) {
-		int j;
-
-		/*
-		 * ...if the addressing mode is indirect with a
-		 * positive offset (ex.: "1608(%ax)"), we need to add
-		 * a '+' prefix so as to be compliant with uprobe
-		 * format.
-		 */
-		if (desc[0] != '+' && desc[0] != '-')
-			prefix = "+";
-
-		/*
-		 * ...or if the addressing mode is indirect with a symbol
-		 * as offset, the argument will not be supported by
-		 * the uprobe tracer format; so, let's skip this one.
-		 */
-		for (j = 0; j < tmp - desc; j++) {
-			if (desc[j] != '+' && desc[j] != '-' &&
-				!isdigit(desc[j])) {
-				pr_debug4("Skipping unsupported SDT argument; "
-					"%s\n", desc);
-				goto out;
-			}
-		}
-	}
-
-	/*
-	 * The uprobe tracer format does not support constants; if we
-	 * find one in the current argument, let's skip the argument.
-	 */
-	if (strchr(desc, '$')) {
-		pr_debug4("Skipping unsupported SDT argument; %s\n", desc);
-		goto out;
-	}
-
-	/*
-	 * The uprobe parser does not support all gas register names;
-	 * so, we have to replace them (ex. for x86_64: %rax -> %ax);
-	 * the loop below looks for the register names (starting with
-	 * a '%' and tries to perform the needed renamings.
-	 */
-	tmp = strchr(desc, '%');
-	while (tmp) {
-		size_t offset = tmp - desc;
-
-		ret = sdt_rename_register(&desc, desc + offset);
-		if (ret < 0)
+		if (sdt_probe_parse_n(desc, &suffix))
 			goto error;
-
-		/*
-		 * The desc pointer might have changed; so, let's not
-		 * try to reuse tmp for next lookup
-		 */
-		tmp = strchr(desc + offset + 1, '%');
+		memmove(desc, tmp, strlen(tmp) + 1);
 	}
 
-	if (strbuf_addf(buf, " arg%d=%s%s%s", i + 1, prefix, desc, suffix) < 0)
+	ret = arch_sdt_probe_parse_op(&desc, &prefix);
+	if (ret < 0)
+		goto error;
+
+	if (ret == 0 &&
+	    strbuf_addf(buf, " arg%d=%s%s%s", i + 1, prefix, desc, suffix) < 0)
 		goto error;
 
-out:
 	ret = 0;
 error:
 	free(desc);
@@ -829,7 +768,7 @@ static char *synthesize_sdt_probe_command(struct sdt_note *note,
 				sdt_note__get_addr(note)) < 0)
 		goto error;
 
-	if (!note->args)
+	if (!note->args || !arch_sdt_probe_arg_supp())
 		goto out;
 
 	if (note->args) {
-- 
2.9.3