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.bango...@linux.vnet.ibm.com>
---
 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

Reply via email to