* [PATCH] Split i386_stap_parse_special_token into smaller functions
@ 2013-12-29 5:43 Sergio Durigan Junior
2013-12-30 3:11 ` Joel Brobecker
0 siblings, 1 reply; 7+ messages in thread
From: Sergio Durigan Junior @ 2013-12-29 5:43 UTC (permalink / raw)
To: GDB Patches; +Cc: Joel Brobecker
Hi,
As requested by Joel on:
<https://sourceware.org/ml/gdb-patches/2013-12/msg00977.html>
I am reposting this separate patch whose only purpose is to split
i386_stap_parse_special_token into smaller functions. I haven't
modified anything logical in the functions, i.e., there's still one
latent bug on i386_stap_parse_special_token_triplet now. I will soon
post a patch to fix this, and to also improve the readability of the two
new functions.
I am also posting the output of "git diff -b" here.
-=-=- git diff -b -=-=-
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 4f86f0c..1a435c1 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -3605,40 +3605,20 @@ i386_stap_is_single_operand (struct gdbarch *gdbarch, const char *s)
|| (*s == '%' && isalpha (s[1]))); /* Register access. */
}
-/* Implementation of `gdbarch_stap_parse_special_token', as defined in
- gdbarch.h. */
-
-int
-i386_stap_parse_special_token (struct gdbarch *gdbarch,
- struct stap_parse_info *p)
-{
- /* In order to parse special tokens, we use a state-machine that go
- through every known token and try to get a match. */
- enum
- {
- TRIPLET,
- THREE_ARG_DISPLACEMENT,
- DONE
- } current_state;
-
- current_state = TRIPLET;
+/* Helper function for i386_stap_parse_special_token.
- /* The special tokens to be parsed here are:
-
- - `register base + (register index * size) + offset', as represented
- in `(%rcx,%rax,8)', or `[OFFSET](BASE_REG,INDEX_REG[,SIZE])'.
+ This function parses operands of the form `-8+3+1(%rbp)', which
+ must be interpreted as `*(-8 + 3 - 1 + (void *) $eax)'.
- - Operands of the form `-8+3+1(%rbp)', which must be interpreted as
- `*(-8 + 3 - 1 + (void *) $eax)'. */
+ Return 1 if the operand was parsed successfully, zero
+ otherwise. */
- while (current_state != DONE)
- {
+static int
+i386_stap_parse_special_token_triplet (struct gdbarch *gdbarch,
+ struct stap_parse_info *p)
+{
const char *s = p->arg;
- switch (current_state)
- {
- case TRIPLET:
- {
if (isdigit (*s) || *s == '-' || *s == '+')
{
int got_minus[3];
@@ -3665,7 +3645,7 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
if (*s != '+' && *s != '-')
{
/* We are not dealing with a triplet. */
- break;
+ return 0;
}
got_minus[1] = 0;
@@ -3683,7 +3663,7 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
if (*s != '+' && *s != '-')
{
/* We are not dealing with a triplet. */
- break;
+ return 0;
}
got_minus[2] = 0;
@@ -3699,7 +3679,7 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
s = endp;
if (*s != '(' || s[1] != '%')
- break;
+ return 0;
s += 2;
start = s;
@@ -3708,7 +3688,7 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
++s;
if (*s++ != ')')
- break;
+ return 0;
len = s - start;
regname = alloca (len + 1);
@@ -3716,17 +3696,14 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
strncpy (regname, start, len);
regname[len] = '\0';
- if (user_reg_map_name_to_regnum (gdbarch,
- regname, len) == -1)
- error (_("Invalid register name `%s' "
- "on expression `%s'."),
+ if (user_reg_map_name_to_regnum (gdbarch, regname, len) == -1)
+ error (_("Invalid register name `%s' on expression `%s'."),
regname, p->saved_arg);
for (i = 0; i < 3; i++)
{
write_exp_elt_opcode (OP_LONG);
- write_exp_elt_type
- (builtin_type (gdbarch)->builtin_long);
+ write_exp_elt_type (builtin_type (gdbarch)->builtin_long);
write_exp_elt_longcst (displacements[i]);
write_exp_elt_opcode (OP_LONG);
if (got_minus[i])
@@ -3757,10 +3734,25 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
return 1;
}
- break;
- }
- case THREE_ARG_DISPLACEMENT:
- {
+
+ return 0;
+}
+
+/* Helper function for i386_stap_parse_special_token.
+
+ This function parses operands of the form `register base +
+ (register index * size) + offset', as represented in
+ `(%rcx,%rax,8)', or `[OFFSET](BASE_REG,INDEX_REG[,SIZE])'.
+
+ Return 1 if the operand was parsed successfully, zero
+ otherwise. */
+
+static int
+i386_stap_parse_special_token_three_arg_disp (struct gdbarch *gdbarch,
+ struct stap_parse_info *p)
+{
+ const char *s = p->arg;
+
if (isdigit (*s) || *s == '(' || *s == '-' || *s == '+')
{
int offset_minus = 0;
@@ -3783,7 +3775,7 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
}
if (offset_minus && !isdigit (*s))
- break;
+ return 0;
if (isdigit (*s))
{
@@ -3794,7 +3786,7 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
}
if (*s != '(' || s[1] != '%')
- break;
+ return 0;
s += 2;
start = s;
@@ -3803,17 +3795,15 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
++s;
if (*s != ',' || s[1] != '%')
- break;
+ return 0;
len_base = s - start;
base = alloca (len_base + 1);
strncpy (base, start, len_base);
base[len_base] = '\0';
- if (user_reg_map_name_to_regnum (gdbarch,
- base, len_base) == -1)
- error (_("Invalid register name `%s' "
- "on expression `%s'."),
+ if (user_reg_map_name_to_regnum (gdbarch, base, len_base) == -1)
+ error (_("Invalid register name `%s' on expression `%s'."),
base, p->saved_arg);
s += 2;
@@ -3827,14 +3817,12 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
strncpy (index, start, len_index);
index[len_index] = '\0';
- if (user_reg_map_name_to_regnum (gdbarch,
- index, len_index) == -1)
- error (_("Invalid register name `%s' "
- "on expression `%s'."),
+ if (user_reg_map_name_to_regnum (gdbarch, index, len_index) == -1)
+ error (_("Invalid register name `%s' on expression `%s'."),
index, p->saved_arg);
if (*s != ',' && *s != ')')
- break;
+ return 0;
if (*s == ',')
{
@@ -3853,7 +3841,7 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
s = endp;
if (*s != ')')
- break;
+ return 0;
}
++s;
@@ -3861,8 +3849,7 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
if (offset)
{
write_exp_elt_opcode (OP_LONG);
- write_exp_elt_type
- (builtin_type (gdbarch)->builtin_long);
+ write_exp_elt_type (builtin_type (gdbarch)->builtin_long);
write_exp_elt_longcst (offset);
write_exp_elt_opcode (OP_LONG);
if (offset_minus)
@@ -3887,8 +3874,7 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
if (size)
{
write_exp_elt_opcode (OP_LONG);
- write_exp_elt_type
- (builtin_type (gdbarch)->builtin_long);
+ write_exp_elt_type (builtin_type (gdbarch)->builtin_long);
write_exp_elt_longcst (size);
write_exp_elt_opcode (OP_LONG);
if (size_minus)
@@ -3908,8 +3894,49 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
return 1;
}
+
+ return 0;
+}
+
+/* Implementation of `gdbarch_stap_parse_special_token', as defined in
+ gdbarch.h. */
+
+int
+i386_stap_parse_special_token (struct gdbarch *gdbarch,
+ struct stap_parse_info *p)
+{
+ /* In order to parse special tokens, we use a state-machine that go
+ through every known token and try to get a match. */
+ enum
+ {
+ TRIPLET,
+ THREE_ARG_DISPLACEMENT,
+ DONE
+ } current_state;
+
+ current_state = TRIPLET;
+
+ /* The special tokens to be parsed here are:
+
+ - `register base + (register index * size) + offset', as represented
+ in `(%rcx,%rax,8)', or `[OFFSET](BASE_REG,INDEX_REG[,SIZE])'.
+
+ - Operands of the form `-8+3+1(%rbp)', which must be interpreted as
+ `*(-8 + 3 - 1 + (void *) $eax)'. */
+
+ while (current_state != DONE)
+ {
+ switch (current_state)
+ {
+ case TRIPLET:
+ if (i386_stap_parse_special_token_triplet (gdbarch, p))
+ return 1;
+ break;
+
+ case THREE_ARG_DISPLACEMENT:
+ if (i386_stap_parse_special_token_three_arg_disp (gdbarch, p))
+ return 1;
break;
- }
}
/* Advancing to the next state. */
-=-=- git diff -b -=-=-
I tested this on Fedora 18 x86_64, everything is OK.
OK to apply?
--
Sergio
2013-12-29 Sergio Durigan Junior <sergiodj@redhat.com>
* i386-tdep.c (i386_stap_parse_special_token_triplet): New
function, with code from i386_stap_parse_special_token.
(i386_stap_parse_special_token_three_arg_disp): Likewise.
(i386_stap_parse_special_token): Move code to the two functions
above; simplify it.
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 4f86f0c..1a435c1 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -3605,311 +3605,338 @@ i386_stap_is_single_operand (struct gdbarch *gdbarch, const char *s)
|| (*s == '%' && isalpha (s[1]))); /* Register access. */
}
-/* Implementation of `gdbarch_stap_parse_special_token', as defined in
- gdbarch.h. */
+/* Helper function for i386_stap_parse_special_token.
-int
-i386_stap_parse_special_token (struct gdbarch *gdbarch,
- struct stap_parse_info *p)
+ This function parses operands of the form `-8+3+1(%rbp)', which
+ must be interpreted as `*(-8 + 3 - 1 + (void *) $eax)'.
+
+ Return 1 if the operand was parsed successfully, zero
+ otherwise. */
+
+static int
+i386_stap_parse_special_token_triplet (struct gdbarch *gdbarch,
+ struct stap_parse_info *p)
{
- /* In order to parse special tokens, we use a state-machine that go
- through every known token and try to get a match. */
- enum
+ const char *s = p->arg;
+
+ if (isdigit (*s) || *s == '-' || *s == '+')
{
- TRIPLET,
- THREE_ARG_DISPLACEMENT,
- DONE
- } current_state;
+ int got_minus[3];
+ int i;
+ long displacements[3];
+ const char *start;
+ char *regname;
+ int len;
+ struct stoken str;
+ char *endp;
+
+ got_minus[0] = 0;
+ if (*s == '+')
+ ++s;
+ else if (*s == '-')
+ {
+ ++s;
+ got_minus[0] = 1;
+ }
- current_state = TRIPLET;
+ displacements[0] = strtol (s, &endp, 10);
+ s = endp;
- /* The special tokens to be parsed here are:
+ if (*s != '+' && *s != '-')
+ {
+ /* We are not dealing with a triplet. */
+ return 0;
+ }
- - `register base + (register index * size) + offset', as represented
- in `(%rcx,%rax,8)', or `[OFFSET](BASE_REG,INDEX_REG[,SIZE])'.
+ got_minus[1] = 0;
+ if (*s == '+')
+ ++s;
+ else
+ {
+ ++s;
+ got_minus[1] = 1;
+ }
- - Operands of the form `-8+3+1(%rbp)', which must be interpreted as
- `*(-8 + 3 - 1 + (void *) $eax)'. */
+ displacements[1] = strtol (s, &endp, 10);
+ s = endp;
- while (current_state != DONE)
- {
- const char *s = p->arg;
+ if (*s != '+' && *s != '-')
+ {
+ /* We are not dealing with a triplet. */
+ return 0;
+ }
- switch (current_state)
+ got_minus[2] = 0;
+ if (*s == '+')
+ ++s;
+ else
{
- case TRIPLET:
- {
- if (isdigit (*s) || *s == '-' || *s == '+')
- {
- int got_minus[3];
- int i;
- long displacements[3];
- const char *start;
- char *regname;
- int len;
- struct stoken str;
- char *endp;
-
- got_minus[0] = 0;
- if (*s == '+')
- ++s;
- else if (*s == '-')
- {
- ++s;
- got_minus[0] = 1;
- }
+ ++s;
+ got_minus[2] = 1;
+ }
- displacements[0] = strtol (s, &endp, 10);
- s = endp;
+ displacements[2] = strtol (s, &endp, 10);
+ s = endp;
- if (*s != '+' && *s != '-')
- {
- /* We are not dealing with a triplet. */
- break;
- }
+ if (*s != '(' || s[1] != '%')
+ return 0;
- got_minus[1] = 0;
- if (*s == '+')
- ++s;
- else
- {
- ++s;
- got_minus[1] = 1;
- }
+ s += 2;
+ start = s;
- displacements[1] = strtol (s, &endp, 10);
- s = endp;
+ while (isalnum (*s))
+ ++s;
- if (*s != '+' && *s != '-')
- {
- /* We are not dealing with a triplet. */
- break;
- }
+ if (*s++ != ')')
+ return 0;
- got_minus[2] = 0;
- if (*s == '+')
- ++s;
- else
- {
- ++s;
- got_minus[2] = 1;
- }
+ len = s - start;
+ regname = alloca (len + 1);
- displacements[2] = strtol (s, &endp, 10);
- s = endp;
+ strncpy (regname, start, len);
+ regname[len] = '\0';
- if (*s != '(' || s[1] != '%')
- break;
+ if (user_reg_map_name_to_regnum (gdbarch, regname, len) == -1)
+ error (_("Invalid register name `%s' on expression `%s'."),
+ regname, p->saved_arg);
- s += 2;
- start = s;
+ for (i = 0; i < 3; i++)
+ {
+ write_exp_elt_opcode (OP_LONG);
+ write_exp_elt_type (builtin_type (gdbarch)->builtin_long);
+ write_exp_elt_longcst (displacements[i]);
+ write_exp_elt_opcode (OP_LONG);
+ if (got_minus[i])
+ write_exp_elt_opcode (UNOP_NEG);
+ }
- while (isalnum (*s))
- ++s;
+ write_exp_elt_opcode (OP_REGISTER);
+ str.ptr = regname;
+ str.length = len;
+ write_exp_string (str);
+ write_exp_elt_opcode (OP_REGISTER);
- if (*s++ != ')')
- break;
+ write_exp_elt_opcode (UNOP_CAST);
+ write_exp_elt_type (builtin_type (gdbarch)->builtin_data_ptr);
+ write_exp_elt_opcode (UNOP_CAST);
- len = s - start;
- regname = alloca (len + 1);
+ write_exp_elt_opcode (BINOP_ADD);
+ write_exp_elt_opcode (BINOP_ADD);
+ write_exp_elt_opcode (BINOP_ADD);
- strncpy (regname, start, len);
- regname[len] = '\0';
+ write_exp_elt_opcode (UNOP_CAST);
+ write_exp_elt_type (lookup_pointer_type (p->arg_type));
+ write_exp_elt_opcode (UNOP_CAST);
- if (user_reg_map_name_to_regnum (gdbarch,
- regname, len) == -1)
- error (_("Invalid register name `%s' "
- "on expression `%s'."),
- regname, p->saved_arg);
+ write_exp_elt_opcode (UNOP_IND);
- for (i = 0; i < 3; i++)
- {
- write_exp_elt_opcode (OP_LONG);
- write_exp_elt_type
- (builtin_type (gdbarch)->builtin_long);
- write_exp_elt_longcst (displacements[i]);
- write_exp_elt_opcode (OP_LONG);
- if (got_minus[i])
- write_exp_elt_opcode (UNOP_NEG);
- }
+ p->arg = s;
- write_exp_elt_opcode (OP_REGISTER);
- str.ptr = regname;
- str.length = len;
- write_exp_string (str);
- write_exp_elt_opcode (OP_REGISTER);
+ return 1;
+ }
- write_exp_elt_opcode (UNOP_CAST);
- write_exp_elt_type (builtin_type (gdbarch)->builtin_data_ptr);
- write_exp_elt_opcode (UNOP_CAST);
+ return 0;
+}
- write_exp_elt_opcode (BINOP_ADD);
- write_exp_elt_opcode (BINOP_ADD);
- write_exp_elt_opcode (BINOP_ADD);
+/* Helper function for i386_stap_parse_special_token.
- write_exp_elt_opcode (UNOP_CAST);
- write_exp_elt_type (lookup_pointer_type (p->arg_type));
- write_exp_elt_opcode (UNOP_CAST);
+ This function parses operands of the form `register base +
+ (register index * size) + offset', as represented in
+ `(%rcx,%rax,8)', or `[OFFSET](BASE_REG,INDEX_REG[,SIZE])'.
- write_exp_elt_opcode (UNOP_IND);
+ Return 1 if the operand was parsed successfully, zero
+ otherwise. */
- p->arg = s;
+static int
+i386_stap_parse_special_token_three_arg_disp (struct gdbarch *gdbarch,
+ struct stap_parse_info *p)
+{
+ const char *s = p->arg;
- return 1;
- }
- break;
- }
- case THREE_ARG_DISPLACEMENT:
- {
- if (isdigit (*s) || *s == '(' || *s == '-' || *s == '+')
- {
- int offset_minus = 0;
- long offset = 0;
- int size_minus = 0;
- long size = 0;
- const char *start;
- char *base;
- int len_base;
- char *index;
- int len_index;
- struct stoken base_token, index_token;
-
- if (*s == '+')
- ++s;
- else if (*s == '-')
- {
- ++s;
- offset_minus = 1;
- }
+ if (isdigit (*s) || *s == '(' || *s == '-' || *s == '+')
+ {
+ int offset_minus = 0;
+ long offset = 0;
+ int size_minus = 0;
+ long size = 0;
+ const char *start;
+ char *base;
+ int len_base;
+ char *index;
+ int len_index;
+ struct stoken base_token, index_token;
+
+ if (*s == '+')
+ ++s;
+ else if (*s == '-')
+ {
+ ++s;
+ offset_minus = 1;
+ }
- if (offset_minus && !isdigit (*s))
- break;
+ if (offset_minus && !isdigit (*s))
+ return 0;
- if (isdigit (*s))
- {
- char *endp;
+ if (isdigit (*s))
+ {
+ char *endp;
- offset = strtol (s, &endp, 10);
- s = endp;
- }
+ offset = strtol (s, &endp, 10);
+ s = endp;
+ }
- if (*s != '(' || s[1] != '%')
- break;
+ if (*s != '(' || s[1] != '%')
+ return 0;
- s += 2;
- start = s;
+ s += 2;
+ start = s;
- while (isalnum (*s))
- ++s;
+ while (isalnum (*s))
+ ++s;
- if (*s != ',' || s[1] != '%')
- break;
+ if (*s != ',' || s[1] != '%')
+ return 0;
- len_base = s - start;
- base = alloca (len_base + 1);
- strncpy (base, start, len_base);
- base[len_base] = '\0';
+ len_base = s - start;
+ base = alloca (len_base + 1);
+ strncpy (base, start, len_base);
+ base[len_base] = '\0';
- if (user_reg_map_name_to_regnum (gdbarch,
- base, len_base) == -1)
- error (_("Invalid register name `%s' "
- "on expression `%s'."),
- base, p->saved_arg);
+ if (user_reg_map_name_to_regnum (gdbarch, base, len_base) == -1)
+ error (_("Invalid register name `%s' on expression `%s'."),
+ base, p->saved_arg);
- s += 2;
- start = s;
+ s += 2;
+ start = s;
- while (isalnum (*s))
- ++s;
+ while (isalnum (*s))
+ ++s;
- len_index = s - start;
- index = alloca (len_index + 1);
- strncpy (index, start, len_index);
- index[len_index] = '\0';
+ len_index = s - start;
+ index = alloca (len_index + 1);
+ strncpy (index, start, len_index);
+ index[len_index] = '\0';
- if (user_reg_map_name_to_regnum (gdbarch,
- index, len_index) == -1)
- error (_("Invalid register name `%s' "
- "on expression `%s'."),
- index, p->saved_arg);
+ if (user_reg_map_name_to_regnum (gdbarch, index, len_index) == -1)
+ error (_("Invalid register name `%s' on expression `%s'."),
+ index, p->saved_arg);
- if (*s != ',' && *s != ')')
- break;
+ if (*s != ',' && *s != ')')
+ return 0;
- if (*s == ',')
- {
- char *endp;
+ if (*s == ',')
+ {
+ char *endp;
- ++s;
- if (*s == '+')
- ++s;
- else if (*s == '-')
- {
- ++s;
- size_minus = 1;
- }
+ ++s;
+ if (*s == '+')
+ ++s;
+ else if (*s == '-')
+ {
+ ++s;
+ size_minus = 1;
+ }
- size = strtol (s, &endp, 10);
- s = endp;
+ size = strtol (s, &endp, 10);
+ s = endp;
- if (*s != ')')
- break;
- }
+ if (*s != ')')
+ return 0;
+ }
- ++s;
+ ++s;
- if (offset)
- {
- write_exp_elt_opcode (OP_LONG);
- write_exp_elt_type
- (builtin_type (gdbarch)->builtin_long);
- write_exp_elt_longcst (offset);
- write_exp_elt_opcode (OP_LONG);
- if (offset_minus)
- write_exp_elt_opcode (UNOP_NEG);
- }
+ if (offset)
+ {
+ write_exp_elt_opcode (OP_LONG);
+ write_exp_elt_type (builtin_type (gdbarch)->builtin_long);
+ write_exp_elt_longcst (offset);
+ write_exp_elt_opcode (OP_LONG);
+ if (offset_minus)
+ write_exp_elt_opcode (UNOP_NEG);
+ }
- write_exp_elt_opcode (OP_REGISTER);
- base_token.ptr = base;
- base_token.length = len_base;
- write_exp_string (base_token);
- write_exp_elt_opcode (OP_REGISTER);
+ write_exp_elt_opcode (OP_REGISTER);
+ base_token.ptr = base;
+ base_token.length = len_base;
+ write_exp_string (base_token);
+ write_exp_elt_opcode (OP_REGISTER);
- if (offset)
- write_exp_elt_opcode (BINOP_ADD);
+ if (offset)
+ write_exp_elt_opcode (BINOP_ADD);
- write_exp_elt_opcode (OP_REGISTER);
- index_token.ptr = index;
- index_token.length = len_index;
- write_exp_string (index_token);
- write_exp_elt_opcode (OP_REGISTER);
+ write_exp_elt_opcode (OP_REGISTER);
+ index_token.ptr = index;
+ index_token.length = len_index;
+ write_exp_string (index_token);
+ write_exp_elt_opcode (OP_REGISTER);
- if (size)
- {
- write_exp_elt_opcode (OP_LONG);
- write_exp_elt_type
- (builtin_type (gdbarch)->builtin_long);
- write_exp_elt_longcst (size);
- write_exp_elt_opcode (OP_LONG);
- if (size_minus)
- write_exp_elt_opcode (UNOP_NEG);
- write_exp_elt_opcode (BINOP_MUL);
- }
+ if (size)
+ {
+ write_exp_elt_opcode (OP_LONG);
+ write_exp_elt_type (builtin_type (gdbarch)->builtin_long);
+ write_exp_elt_longcst (size);
+ write_exp_elt_opcode (OP_LONG);
+ if (size_minus)
+ write_exp_elt_opcode (UNOP_NEG);
+ write_exp_elt_opcode (BINOP_MUL);
+ }
- write_exp_elt_opcode (BINOP_ADD);
+ write_exp_elt_opcode (BINOP_ADD);
- write_exp_elt_opcode (UNOP_CAST);
- write_exp_elt_type (lookup_pointer_type (p->arg_type));
- write_exp_elt_opcode (UNOP_CAST);
+ write_exp_elt_opcode (UNOP_CAST);
+ write_exp_elt_type (lookup_pointer_type (p->arg_type));
+ write_exp_elt_opcode (UNOP_CAST);
- write_exp_elt_opcode (UNOP_IND);
+ write_exp_elt_opcode (UNOP_IND);
- p->arg = s;
+ p->arg = s;
- return 1;
- }
- break;
- }
+ return 1;
+ }
+
+ return 0;
+}
+
+/* Implementation of `gdbarch_stap_parse_special_token', as defined in
+ gdbarch.h. */
+
+int
+i386_stap_parse_special_token (struct gdbarch *gdbarch,
+ struct stap_parse_info *p)
+{
+ /* In order to parse special tokens, we use a state-machine that go
+ through every known token and try to get a match. */
+ enum
+ {
+ TRIPLET,
+ THREE_ARG_DISPLACEMENT,
+ DONE
+ } current_state;
+
+ current_state = TRIPLET;
+
+ /* The special tokens to be parsed here are:
+
+ - `register base + (register index * size) + offset', as represented
+ in `(%rcx,%rax,8)', or `[OFFSET](BASE_REG,INDEX_REG[,SIZE])'.
+
+ - Operands of the form `-8+3+1(%rbp)', which must be interpreted as
+ `*(-8 + 3 - 1 + (void *) $eax)'. */
+
+ while (current_state != DONE)
+ {
+ switch (current_state)
+ {
+ case TRIPLET:
+ if (i386_stap_parse_special_token_triplet (gdbarch, p))
+ return 1;
+ break;
+
+ case THREE_ARG_DISPLACEMENT:
+ if (i386_stap_parse_special_token_three_arg_disp (gdbarch, p))
+ return 1;
+ break;
}
/* Advancing to the next state. */
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Split i386_stap_parse_special_token into smaller functions
2013-12-29 5:43 [PATCH] Split i386_stap_parse_special_token into smaller functions Sergio Durigan Junior
@ 2013-12-30 3:11 ` Joel Brobecker
2013-12-30 5:38 ` Sergio Durigan Junior
2014-01-06 15:34 ` Sergio Durigan Junior
0 siblings, 2 replies; 7+ messages in thread
From: Joel Brobecker @ 2013-12-30 3:11 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: GDB Patches, Mark Kettenis
> As requested by Joel on:
>
> <https://sourceware.org/ml/gdb-patches/2013-12/msg00977.html>
>
> I am reposting this separate patch whose only purpose is to split
> i386_stap_parse_special_token into smaller functions. I haven't
> modified anything logical in the functions, i.e., there's still one
> latent bug on i386_stap_parse_special_token_triplet now. I will soon
> post a patch to fix this, and to also improve the readability of the two
> new functions.
>
> I am also posting the output of "git diff -b" here.
Thank you, Sergio. This patch is missing a ChangeLog :).
FWIW, this patch looks good to me, and it is IMO a nice improvement
over the current state. But i386-tdep.c is usually under Mark's
responsibility, so let's give him a little bit of time to reply
as well.
>
> -=-=- git diff -b -=-=-
>
> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
> index 4f86f0c..1a435c1 100644
> --- a/gdb/i386-tdep.c
> +++ b/gdb/i386-tdep.c
> @@ -3605,40 +3605,20 @@ i386_stap_is_single_operand (struct gdbarch *gdbarch, const char *s)
> || (*s == '%' && isalpha (s[1]))); /* Register access. */
> }
>
> -/* Implementation of `gdbarch_stap_parse_special_token', as defined in
> - gdbarch.h. */
> -
> -int
> -i386_stap_parse_special_token (struct gdbarch *gdbarch,
> - struct stap_parse_info *p)
> -{
> - /* In order to parse special tokens, we use a state-machine that go
> - through every known token and try to get a match. */
> - enum
> - {
> - TRIPLET,
> - THREE_ARG_DISPLACEMENT,
> - DONE
> - } current_state;
> -
> - current_state = TRIPLET;
> +/* Helper function for i386_stap_parse_special_token.
>
> - /* The special tokens to be parsed here are:
> -
> - - `register base + (register index * size) + offset', as represented
> - in `(%rcx,%rax,8)', or `[OFFSET](BASE_REG,INDEX_REG[,SIZE])'.
> + This function parses operands of the form `-8+3+1(%rbp)', which
> + must be interpreted as `*(-8 + 3 - 1 + (void *) $eax)'.
>
> - - Operands of the form `-8+3+1(%rbp)', which must be interpreted as
> - `*(-8 + 3 - 1 + (void *) $eax)'. */
> + Return 1 if the operand was parsed successfully, zero
> + otherwise. */
>
> - while (current_state != DONE)
> - {
> +static int
> +i386_stap_parse_special_token_triplet (struct gdbarch *gdbarch,
> + struct stap_parse_info *p)
> +{
> const char *s = p->arg;
>
> - switch (current_state)
> - {
> - case TRIPLET:
> - {
> if (isdigit (*s) || *s == '-' || *s == '+')
> {
> int got_minus[3];
> @@ -3665,7 +3645,7 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
> if (*s != '+' && *s != '-')
> {
> /* We are not dealing with a triplet. */
> - break;
> + return 0;
> }
>
> got_minus[1] = 0;
> @@ -3683,7 +3663,7 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
> if (*s != '+' && *s != '-')
> {
> /* We are not dealing with a triplet. */
> - break;
> + return 0;
> }
>
> got_minus[2] = 0;
> @@ -3699,7 +3679,7 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
> s = endp;
>
> if (*s != '(' || s[1] != '%')
> - break;
> + return 0;
>
> s += 2;
> start = s;
> @@ -3708,7 +3688,7 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
> ++s;
>
> if (*s++ != ')')
> - break;
> + return 0;
>
> len = s - start;
> regname = alloca (len + 1);
> @@ -3716,17 +3696,14 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
> strncpy (regname, start, len);
> regname[len] = '\0';
>
> - if (user_reg_map_name_to_regnum (gdbarch,
> - regname, len) == -1)
> - error (_("Invalid register name `%s' "
> - "on expression `%s'."),
> + if (user_reg_map_name_to_regnum (gdbarch, regname, len) == -1)
> + error (_("Invalid register name `%s' on expression `%s'."),
> regname, p->saved_arg);
>
> for (i = 0; i < 3; i++)
> {
> write_exp_elt_opcode (OP_LONG);
> - write_exp_elt_type
> - (builtin_type (gdbarch)->builtin_long);
> + write_exp_elt_type (builtin_type (gdbarch)->builtin_long);
> write_exp_elt_longcst (displacements[i]);
> write_exp_elt_opcode (OP_LONG);
> if (got_minus[i])
> @@ -3757,10 +3734,25 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
>
> return 1;
> }
> - break;
> - }
> - case THREE_ARG_DISPLACEMENT:
> - {
> +
> + return 0;
> +}
> +
> +/* Helper function for i386_stap_parse_special_token.
> +
> + This function parses operands of the form `register base +
> + (register index * size) + offset', as represented in
> + `(%rcx,%rax,8)', or `[OFFSET](BASE_REG,INDEX_REG[,SIZE])'.
> +
> + Return 1 if the operand was parsed successfully, zero
> + otherwise. */
> +
> +static int
> +i386_stap_parse_special_token_three_arg_disp (struct gdbarch *gdbarch,
> + struct stap_parse_info *p)
> +{
> + const char *s = p->arg;
> +
> if (isdigit (*s) || *s == '(' || *s == '-' || *s == '+')
> {
> int offset_minus = 0;
> @@ -3783,7 +3775,7 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
> }
>
> if (offset_minus && !isdigit (*s))
> - break;
> + return 0;
>
> if (isdigit (*s))
> {
> @@ -3794,7 +3786,7 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
> }
>
> if (*s != '(' || s[1] != '%')
> - break;
> + return 0;
>
> s += 2;
> start = s;
> @@ -3803,17 +3795,15 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
> ++s;
>
> if (*s != ',' || s[1] != '%')
> - break;
> + return 0;
>
> len_base = s - start;
> base = alloca (len_base + 1);
> strncpy (base, start, len_base);
> base[len_base] = '\0';
>
> - if (user_reg_map_name_to_regnum (gdbarch,
> - base, len_base) == -1)
> - error (_("Invalid register name `%s' "
> - "on expression `%s'."),
> + if (user_reg_map_name_to_regnum (gdbarch, base, len_base) == -1)
> + error (_("Invalid register name `%s' on expression `%s'."),
> base, p->saved_arg);
>
> s += 2;
> @@ -3827,14 +3817,12 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
> strncpy (index, start, len_index);
> index[len_index] = '\0';
>
> - if (user_reg_map_name_to_regnum (gdbarch,
> - index, len_index) == -1)
> - error (_("Invalid register name `%s' "
> - "on expression `%s'."),
> + if (user_reg_map_name_to_regnum (gdbarch, index, len_index) == -1)
> + error (_("Invalid register name `%s' on expression `%s'."),
> index, p->saved_arg);
>
> if (*s != ',' && *s != ')')
> - break;
> + return 0;
>
> if (*s == ',')
> {
> @@ -3853,7 +3841,7 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
> s = endp;
>
> if (*s != ')')
> - break;
> + return 0;
> }
>
> ++s;
> @@ -3861,8 +3849,7 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
> if (offset)
> {
> write_exp_elt_opcode (OP_LONG);
> - write_exp_elt_type
> - (builtin_type (gdbarch)->builtin_long);
> + write_exp_elt_type (builtin_type (gdbarch)->builtin_long);
> write_exp_elt_longcst (offset);
> write_exp_elt_opcode (OP_LONG);
> if (offset_minus)
> @@ -3887,8 +3874,7 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
> if (size)
> {
> write_exp_elt_opcode (OP_LONG);
> - write_exp_elt_type
> - (builtin_type (gdbarch)->builtin_long);
> + write_exp_elt_type (builtin_type (gdbarch)->builtin_long);
> write_exp_elt_longcst (size);
> write_exp_elt_opcode (OP_LONG);
> if (size_minus)
> @@ -3908,8 +3894,49 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
>
> return 1;
> }
> +
> + return 0;
> +}
> +
> +/* Implementation of `gdbarch_stap_parse_special_token', as defined in
> + gdbarch.h. */
> +
> +int
> +i386_stap_parse_special_token (struct gdbarch *gdbarch,
> + struct stap_parse_info *p)
> +{
> + /* In order to parse special tokens, we use a state-machine that go
> + through every known token and try to get a match. */
> + enum
> + {
> + TRIPLET,
> + THREE_ARG_DISPLACEMENT,
> + DONE
> + } current_state;
> +
> + current_state = TRIPLET;
> +
> + /* The special tokens to be parsed here are:
> +
> + - `register base + (register index * size) + offset', as represented
> + in `(%rcx,%rax,8)', or `[OFFSET](BASE_REG,INDEX_REG[,SIZE])'.
> +
> + - Operands of the form `-8+3+1(%rbp)', which must be interpreted as
> + `*(-8 + 3 - 1 + (void *) $eax)'. */
> +
> + while (current_state != DONE)
> + {
> + switch (current_state)
> + {
> + case TRIPLET:
> + if (i386_stap_parse_special_token_triplet (gdbarch, p))
> + return 1;
> + break;
> +
> + case THREE_ARG_DISPLACEMENT:
> + if (i386_stap_parse_special_token_three_arg_disp (gdbarch, p))
> + return 1;
> break;
> - }
> }
>
> /* Advancing to the next state. */
>
> -=-=- git diff -b -=-=-
>
> I tested this on Fedora 18 x86_64, everything is OK.
>
> OK to apply?
>
> --
> Sergio
>
> 2013-12-29 Sergio Durigan Junior <sergiodj@redhat.com>
>
> * i386-tdep.c (i386_stap_parse_special_token_triplet): New
> function, with code from i386_stap_parse_special_token.
> (i386_stap_parse_special_token_three_arg_disp): Likewise.
> (i386_stap_parse_special_token): Move code to the two functions
> above; simplify it.
>
> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
> index 4f86f0c..1a435c1 100644
> --- a/gdb/i386-tdep.c
> +++ b/gdb/i386-tdep.c
> @@ -3605,311 +3605,338 @@ i386_stap_is_single_operand (struct gdbarch *gdbarch, const char *s)
> || (*s == '%' && isalpha (s[1]))); /* Register access. */
> }
>
> -/* Implementation of `gdbarch_stap_parse_special_token', as defined in
> - gdbarch.h. */
> +/* Helper function for i386_stap_parse_special_token.
>
> -int
> -i386_stap_parse_special_token (struct gdbarch *gdbarch,
> - struct stap_parse_info *p)
> + This function parses operands of the form `-8+3+1(%rbp)', which
> + must be interpreted as `*(-8 + 3 - 1 + (void *) $eax)'.
> +
> + Return 1 if the operand was parsed successfully, zero
> + otherwise. */
> +
> +static int
> +i386_stap_parse_special_token_triplet (struct gdbarch *gdbarch,
> + struct stap_parse_info *p)
> {
> - /* In order to parse special tokens, we use a state-machine that go
> - through every known token and try to get a match. */
> - enum
> + const char *s = p->arg;
> +
> + if (isdigit (*s) || *s == '-' || *s == '+')
> {
> - TRIPLET,
> - THREE_ARG_DISPLACEMENT,
> - DONE
> - } current_state;
> + int got_minus[3];
> + int i;
> + long displacements[3];
> + const char *start;
> + char *regname;
> + int len;
> + struct stoken str;
> + char *endp;
> +
> + got_minus[0] = 0;
> + if (*s == '+')
> + ++s;
> + else if (*s == '-')
> + {
> + ++s;
> + got_minus[0] = 1;
> + }
>
> - current_state = TRIPLET;
> + displacements[0] = strtol (s, &endp, 10);
> + s = endp;
>
> - /* The special tokens to be parsed here are:
> + if (*s != '+' && *s != '-')
> + {
> + /* We are not dealing with a triplet. */
> + return 0;
> + }
>
> - - `register base + (register index * size) + offset', as represented
> - in `(%rcx,%rax,8)', or `[OFFSET](BASE_REG,INDEX_REG[,SIZE])'.
> + got_minus[1] = 0;
> + if (*s == '+')
> + ++s;
> + else
> + {
> + ++s;
> + got_minus[1] = 1;
> + }
>
> - - Operands of the form `-8+3+1(%rbp)', which must be interpreted as
> - `*(-8 + 3 - 1 + (void *) $eax)'. */
> + displacements[1] = strtol (s, &endp, 10);
> + s = endp;
>
> - while (current_state != DONE)
> - {
> - const char *s = p->arg;
> + if (*s != '+' && *s != '-')
> + {
> + /* We are not dealing with a triplet. */
> + return 0;
> + }
>
> - switch (current_state)
> + got_minus[2] = 0;
> + if (*s == '+')
> + ++s;
> + else
> {
> - case TRIPLET:
> - {
> - if (isdigit (*s) || *s == '-' || *s == '+')
> - {
> - int got_minus[3];
> - int i;
> - long displacements[3];
> - const char *start;
> - char *regname;
> - int len;
> - struct stoken str;
> - char *endp;
> -
> - got_minus[0] = 0;
> - if (*s == '+')
> - ++s;
> - else if (*s == '-')
> - {
> - ++s;
> - got_minus[0] = 1;
> - }
> + ++s;
> + got_minus[2] = 1;
> + }
>
> - displacements[0] = strtol (s, &endp, 10);
> - s = endp;
> + displacements[2] = strtol (s, &endp, 10);
> + s = endp;
>
> - if (*s != '+' && *s != '-')
> - {
> - /* We are not dealing with a triplet. */
> - break;
> - }
> + if (*s != '(' || s[1] != '%')
> + return 0;
>
> - got_minus[1] = 0;
> - if (*s == '+')
> - ++s;
> - else
> - {
> - ++s;
> - got_minus[1] = 1;
> - }
> + s += 2;
> + start = s;
>
> - displacements[1] = strtol (s, &endp, 10);
> - s = endp;
> + while (isalnum (*s))
> + ++s;
>
> - if (*s != '+' && *s != '-')
> - {
> - /* We are not dealing with a triplet. */
> - break;
> - }
> + if (*s++ != ')')
> + return 0;
>
> - got_minus[2] = 0;
> - if (*s == '+')
> - ++s;
> - else
> - {
> - ++s;
> - got_minus[2] = 1;
> - }
> + len = s - start;
> + regname = alloca (len + 1);
>
> - displacements[2] = strtol (s, &endp, 10);
> - s = endp;
> + strncpy (regname, start, len);
> + regname[len] = '\0';
>
> - if (*s != '(' || s[1] != '%')
> - break;
> + if (user_reg_map_name_to_regnum (gdbarch, regname, len) == -1)
> + error (_("Invalid register name `%s' on expression `%s'."),
> + regname, p->saved_arg);
>
> - s += 2;
> - start = s;
> + for (i = 0; i < 3; i++)
> + {
> + write_exp_elt_opcode (OP_LONG);
> + write_exp_elt_type (builtin_type (gdbarch)->builtin_long);
> + write_exp_elt_longcst (displacements[i]);
> + write_exp_elt_opcode (OP_LONG);
> + if (got_minus[i])
> + write_exp_elt_opcode (UNOP_NEG);
> + }
>
> - while (isalnum (*s))
> - ++s;
> + write_exp_elt_opcode (OP_REGISTER);
> + str.ptr = regname;
> + str.length = len;
> + write_exp_string (str);
> + write_exp_elt_opcode (OP_REGISTER);
>
> - if (*s++ != ')')
> - break;
> + write_exp_elt_opcode (UNOP_CAST);
> + write_exp_elt_type (builtin_type (gdbarch)->builtin_data_ptr);
> + write_exp_elt_opcode (UNOP_CAST);
>
> - len = s - start;
> - regname = alloca (len + 1);
> + write_exp_elt_opcode (BINOP_ADD);
> + write_exp_elt_opcode (BINOP_ADD);
> + write_exp_elt_opcode (BINOP_ADD);
>
> - strncpy (regname, start, len);
> - regname[len] = '\0';
> + write_exp_elt_opcode (UNOP_CAST);
> + write_exp_elt_type (lookup_pointer_type (p->arg_type));
> + write_exp_elt_opcode (UNOP_CAST);
>
> - if (user_reg_map_name_to_regnum (gdbarch,
> - regname, len) == -1)
> - error (_("Invalid register name `%s' "
> - "on expression `%s'."),
> - regname, p->saved_arg);
> + write_exp_elt_opcode (UNOP_IND);
>
> - for (i = 0; i < 3; i++)
> - {
> - write_exp_elt_opcode (OP_LONG);
> - write_exp_elt_type
> - (builtin_type (gdbarch)->builtin_long);
> - write_exp_elt_longcst (displacements[i]);
> - write_exp_elt_opcode (OP_LONG);
> - if (got_minus[i])
> - write_exp_elt_opcode (UNOP_NEG);
> - }
> + p->arg = s;
>
> - write_exp_elt_opcode (OP_REGISTER);
> - str.ptr = regname;
> - str.length = len;
> - write_exp_string (str);
> - write_exp_elt_opcode (OP_REGISTER);
> + return 1;
> + }
>
> - write_exp_elt_opcode (UNOP_CAST);
> - write_exp_elt_type (builtin_type (gdbarch)->builtin_data_ptr);
> - write_exp_elt_opcode (UNOP_CAST);
> + return 0;
> +}
>
> - write_exp_elt_opcode (BINOP_ADD);
> - write_exp_elt_opcode (BINOP_ADD);
> - write_exp_elt_opcode (BINOP_ADD);
> +/* Helper function for i386_stap_parse_special_token.
>
> - write_exp_elt_opcode (UNOP_CAST);
> - write_exp_elt_type (lookup_pointer_type (p->arg_type));
> - write_exp_elt_opcode (UNOP_CAST);
> + This function parses operands of the form `register base +
> + (register index * size) + offset', as represented in
> + `(%rcx,%rax,8)', or `[OFFSET](BASE_REG,INDEX_REG[,SIZE])'.
>
> - write_exp_elt_opcode (UNOP_IND);
> + Return 1 if the operand was parsed successfully, zero
> + otherwise. */
>
> - p->arg = s;
> +static int
> +i386_stap_parse_special_token_three_arg_disp (struct gdbarch *gdbarch,
> + struct stap_parse_info *p)
> +{
> + const char *s = p->arg;
>
> - return 1;
> - }
> - break;
> - }
> - case THREE_ARG_DISPLACEMENT:
> - {
> - if (isdigit (*s) || *s == '(' || *s == '-' || *s == '+')
> - {
> - int offset_minus = 0;
> - long offset = 0;
> - int size_minus = 0;
> - long size = 0;
> - const char *start;
> - char *base;
> - int len_base;
> - char *index;
> - int len_index;
> - struct stoken base_token, index_token;
> -
> - if (*s == '+')
> - ++s;
> - else if (*s == '-')
> - {
> - ++s;
> - offset_minus = 1;
> - }
> + if (isdigit (*s) || *s == '(' || *s == '-' || *s == '+')
> + {
> + int offset_minus = 0;
> + long offset = 0;
> + int size_minus = 0;
> + long size = 0;
> + const char *start;
> + char *base;
> + int len_base;
> + char *index;
> + int len_index;
> + struct stoken base_token, index_token;
> +
> + if (*s == '+')
> + ++s;
> + else if (*s == '-')
> + {
> + ++s;
> + offset_minus = 1;
> + }
>
> - if (offset_minus && !isdigit (*s))
> - break;
> + if (offset_minus && !isdigit (*s))
> + return 0;
>
> - if (isdigit (*s))
> - {
> - char *endp;
> + if (isdigit (*s))
> + {
> + char *endp;
>
> - offset = strtol (s, &endp, 10);
> - s = endp;
> - }
> + offset = strtol (s, &endp, 10);
> + s = endp;
> + }
>
> - if (*s != '(' || s[1] != '%')
> - break;
> + if (*s != '(' || s[1] != '%')
> + return 0;
>
> - s += 2;
> - start = s;
> + s += 2;
> + start = s;
>
> - while (isalnum (*s))
> - ++s;
> + while (isalnum (*s))
> + ++s;
>
> - if (*s != ',' || s[1] != '%')
> - break;
> + if (*s != ',' || s[1] != '%')
> + return 0;
>
> - len_base = s - start;
> - base = alloca (len_base + 1);
> - strncpy (base, start, len_base);
> - base[len_base] = '\0';
> + len_base = s - start;
> + base = alloca (len_base + 1);
> + strncpy (base, start, len_base);
> + base[len_base] = '\0';
>
> - if (user_reg_map_name_to_regnum (gdbarch,
> - base, len_base) == -1)
> - error (_("Invalid register name `%s' "
> - "on expression `%s'."),
> - base, p->saved_arg);
> + if (user_reg_map_name_to_regnum (gdbarch, base, len_base) == -1)
> + error (_("Invalid register name `%s' on expression `%s'."),
> + base, p->saved_arg);
>
> - s += 2;
> - start = s;
> + s += 2;
> + start = s;
>
> - while (isalnum (*s))
> - ++s;
> + while (isalnum (*s))
> + ++s;
>
> - len_index = s - start;
> - index = alloca (len_index + 1);
> - strncpy (index, start, len_index);
> - index[len_index] = '\0';
> + len_index = s - start;
> + index = alloca (len_index + 1);
> + strncpy (index, start, len_index);
> + index[len_index] = '\0';
>
> - if (user_reg_map_name_to_regnum (gdbarch,
> - index, len_index) == -1)
> - error (_("Invalid register name `%s' "
> - "on expression `%s'."),
> - index, p->saved_arg);
> + if (user_reg_map_name_to_regnum (gdbarch, index, len_index) == -1)
> + error (_("Invalid register name `%s' on expression `%s'."),
> + index, p->saved_arg);
>
> - if (*s != ',' && *s != ')')
> - break;
> + if (*s != ',' && *s != ')')
> + return 0;
>
> - if (*s == ',')
> - {
> - char *endp;
> + if (*s == ',')
> + {
> + char *endp;
>
> - ++s;
> - if (*s == '+')
> - ++s;
> - else if (*s == '-')
> - {
> - ++s;
> - size_minus = 1;
> - }
> + ++s;
> + if (*s == '+')
> + ++s;
> + else if (*s == '-')
> + {
> + ++s;
> + size_minus = 1;
> + }
>
> - size = strtol (s, &endp, 10);
> - s = endp;
> + size = strtol (s, &endp, 10);
> + s = endp;
>
> - if (*s != ')')
> - break;
> - }
> + if (*s != ')')
> + return 0;
> + }
>
> - ++s;
> + ++s;
>
> - if (offset)
> - {
> - write_exp_elt_opcode (OP_LONG);
> - write_exp_elt_type
> - (builtin_type (gdbarch)->builtin_long);
> - write_exp_elt_longcst (offset);
> - write_exp_elt_opcode (OP_LONG);
> - if (offset_minus)
> - write_exp_elt_opcode (UNOP_NEG);
> - }
> + if (offset)
> + {
> + write_exp_elt_opcode (OP_LONG);
> + write_exp_elt_type (builtin_type (gdbarch)->builtin_long);
> + write_exp_elt_longcst (offset);
> + write_exp_elt_opcode (OP_LONG);
> + if (offset_minus)
> + write_exp_elt_opcode (UNOP_NEG);
> + }
>
> - write_exp_elt_opcode (OP_REGISTER);
> - base_token.ptr = base;
> - base_token.length = len_base;
> - write_exp_string (base_token);
> - write_exp_elt_opcode (OP_REGISTER);
> + write_exp_elt_opcode (OP_REGISTER);
> + base_token.ptr = base;
> + base_token.length = len_base;
> + write_exp_string (base_token);
> + write_exp_elt_opcode (OP_REGISTER);
>
> - if (offset)
> - write_exp_elt_opcode (BINOP_ADD);
> + if (offset)
> + write_exp_elt_opcode (BINOP_ADD);
>
> - write_exp_elt_opcode (OP_REGISTER);
> - index_token.ptr = index;
> - index_token.length = len_index;
> - write_exp_string (index_token);
> - write_exp_elt_opcode (OP_REGISTER);
> + write_exp_elt_opcode (OP_REGISTER);
> + index_token.ptr = index;
> + index_token.length = len_index;
> + write_exp_string (index_token);
> + write_exp_elt_opcode (OP_REGISTER);
>
> - if (size)
> - {
> - write_exp_elt_opcode (OP_LONG);
> - write_exp_elt_type
> - (builtin_type (gdbarch)->builtin_long);
> - write_exp_elt_longcst (size);
> - write_exp_elt_opcode (OP_LONG);
> - if (size_minus)
> - write_exp_elt_opcode (UNOP_NEG);
> - write_exp_elt_opcode (BINOP_MUL);
> - }
> + if (size)
> + {
> + write_exp_elt_opcode (OP_LONG);
> + write_exp_elt_type (builtin_type (gdbarch)->builtin_long);
> + write_exp_elt_longcst (size);
> + write_exp_elt_opcode (OP_LONG);
> + if (size_minus)
> + write_exp_elt_opcode (UNOP_NEG);
> + write_exp_elt_opcode (BINOP_MUL);
> + }
>
> - write_exp_elt_opcode (BINOP_ADD);
> + write_exp_elt_opcode (BINOP_ADD);
>
> - write_exp_elt_opcode (UNOP_CAST);
> - write_exp_elt_type (lookup_pointer_type (p->arg_type));
> - write_exp_elt_opcode (UNOP_CAST);
> + write_exp_elt_opcode (UNOP_CAST);
> + write_exp_elt_type (lookup_pointer_type (p->arg_type));
> + write_exp_elt_opcode (UNOP_CAST);
>
> - write_exp_elt_opcode (UNOP_IND);
> + write_exp_elt_opcode (UNOP_IND);
>
> - p->arg = s;
> + p->arg = s;
>
> - return 1;
> - }
> - break;
> - }
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +/* Implementation of `gdbarch_stap_parse_special_token', as defined in
> + gdbarch.h. */
> +
> +int
> +i386_stap_parse_special_token (struct gdbarch *gdbarch,
> + struct stap_parse_info *p)
> +{
> + /* In order to parse special tokens, we use a state-machine that go
> + through every known token and try to get a match. */
> + enum
> + {
> + TRIPLET,
> + THREE_ARG_DISPLACEMENT,
> + DONE
> + } current_state;
> +
> + current_state = TRIPLET;
> +
> + /* The special tokens to be parsed here are:
> +
> + - `register base + (register index * size) + offset', as represented
> + in `(%rcx,%rax,8)', or `[OFFSET](BASE_REG,INDEX_REG[,SIZE])'.
> +
> + - Operands of the form `-8+3+1(%rbp)', which must be interpreted as
> + `*(-8 + 3 - 1 + (void *) $eax)'. */
> +
> + while (current_state != DONE)
> + {
> + switch (current_state)
> + {
> + case TRIPLET:
> + if (i386_stap_parse_special_token_triplet (gdbarch, p))
> + return 1;
> + break;
> +
> + case THREE_ARG_DISPLACEMENT:
> + if (i386_stap_parse_special_token_three_arg_disp (gdbarch, p))
> + return 1;
> + break;
> }
>
> /* Advancing to the next state. */
--
Joel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Split i386_stap_parse_special_token into smaller functions
2013-12-30 3:11 ` Joel Brobecker
@ 2013-12-30 5:38 ` Sergio Durigan Junior
2014-01-06 15:34 ` Sergio Durigan Junior
1 sibling, 0 replies; 7+ messages in thread
From: Sergio Durigan Junior @ 2013-12-30 5:38 UTC (permalink / raw)
To: Joel Brobecker; +Cc: GDB Patches, Mark Kettenis
On Monday, December 30 2014, Joel Brobecker wrote:
> Thank you, Sergio. This patch is missing a ChangeLog :).
Ops, forgot to post it, but it's here, I promise :-).
> FWIW, this patch looks good to me, and it is IMO a nice improvement
> over the current state. But i386-tdep.c is usually under Mark's
> responsibility, so let's give him a little bit of time to reply
> as well.
Fair enough.
Thanks!
--
Sergio
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Split i386_stap_parse_special_token into smaller functions
2013-12-30 3:11 ` Joel Brobecker
2013-12-30 5:38 ` Sergio Durigan Junior
@ 2014-01-06 15:34 ` Sergio Durigan Junior
2014-01-11 1:52 ` Sergio Durigan Junior
1 sibling, 1 reply; 7+ messages in thread
From: Sergio Durigan Junior @ 2014-01-06 15:34 UTC (permalink / raw)
To: Joel Brobecker; +Cc: GDB Patches, Mark Kettenis
On Monday, December 30 2014, Joel Brobecker wrote:
>> As requested by Joel on:
>>
>> <https://sourceware.org/ml/gdb-patches/2013-12/msg00977.html>
>>
>> I am reposting this separate patch whose only purpose is to split
>> i386_stap_parse_special_token into smaller functions. I haven't
>> modified anything logical in the functions, i.e., there's still one
>> latent bug on i386_stap_parse_special_token_triplet now. I will soon
>> post a patch to fix this, and to also improve the readability of the two
>> new functions.
>>
>> I am also posting the output of "git diff -b" here.
>
> Thank you, Sergio. This patch is missing a ChangeLog :).
>
> FWIW, this patch looks good to me, and it is IMO a nice improvement
> over the current state. But i386-tdep.c is usually under Mark's
> responsibility, so let's give him a little bit of time to reply
> as well.
Ping.
--
Sergio
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Split i386_stap_parse_special_token into smaller functions
2014-01-06 15:34 ` Sergio Durigan Junior
@ 2014-01-11 1:52 ` Sergio Durigan Junior
2014-01-11 3:02 ` Joel Brobecker
0 siblings, 1 reply; 7+ messages in thread
From: Sergio Durigan Junior @ 2014-01-11 1:52 UTC (permalink / raw)
To: Joel Brobecker; +Cc: GDB Patches, Mark Kettenis
On Monday, January 06 2014, I wrote:
> On Monday, December 30 2014, Joel Brobecker wrote:
>
>>> As requested by Joel on:
>>>
>>> <https://sourceware.org/ml/gdb-patches/2013-12/msg00977.html>
>>>
>>> I am reposting this separate patch whose only purpose is to split
>>> i386_stap_parse_special_token into smaller functions. I haven't
>>> modified anything logical in the functions, i.e., there's still one
>>> latent bug on i386_stap_parse_special_token_triplet now. I will soon
>>> post a patch to fix this, and to also improve the readability of the two
>>> new functions.
>>>
>>> I am also posting the output of "git diff -b" here.
>>
>> Thank you, Sergio. This patch is missing a ChangeLog :).
>>
>> FWIW, this patch looks good to me, and it is IMO a nice improvement
>> over the current state. But i386-tdep.c is usually under Mark's
>> responsibility, so let's give him a little bit of time to reply
>> as well.
>
> Ping.
Ping^2.
This patch is pretty simple IMO (just code movement), so maybe it could
be reviewed by some other maintainer and maybe approved? It's holding a
fix for a bug :-).
Thanks,
--
Sergio
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Split i386_stap_parse_special_token into smaller functions
2014-01-11 1:52 ` Sergio Durigan Junior
@ 2014-01-11 3:02 ` Joel Brobecker
2014-01-12 3:36 ` Sergio Durigan Junior
0 siblings, 1 reply; 7+ messages in thread
From: Joel Brobecker @ 2014-01-11 3:02 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: GDB Patches, Mark Kettenis
> >> FWIW, this patch looks good to me, and it is IMO a nice improvement
> >> over the current state. But i386-tdep.c is usually under Mark's
> >> responsibility, so let's give him a little bit of time to reply
> >> as well.
> >
> > Ping.
>
> Ping^2.
>
> This patch is pretty simple IMO (just code movement), so maybe it could
> be reviewed by some other maintainer and maybe approved? It's holding a
> fix for a bug :-).
Go ahead and push. I reviewed the patch a second time, and it still
looks good to me.
Thank you, Sergio.
--
Joel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Split i386_stap_parse_special_token into smaller functions
2014-01-11 3:02 ` Joel Brobecker
@ 2014-01-12 3:36 ` Sergio Durigan Junior
0 siblings, 0 replies; 7+ messages in thread
From: Sergio Durigan Junior @ 2014-01-12 3:36 UTC (permalink / raw)
To: Joel Brobecker; +Cc: GDB Patches, Mark Kettenis
On Saturday, January 11 2014, Joel Brobecker wrote:
> Go ahead and push. I reviewed the patch a second time, and it still
> looks good to me.
Thanks, Joel. Checked in:
https://sourceware.org/ml/gdb-cvs/2014-01/msg00045.html
--
Sergio
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-01-12 3:36 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-29 5:43 [PATCH] Split i386_stap_parse_special_token into smaller functions Sergio Durigan Junior
2013-12-30 3:11 ` Joel Brobecker
2013-12-30 5:38 ` Sergio Durigan Junior
2014-01-06 15:34 ` Sergio Durigan Junior
2014-01-11 1:52 ` Sergio Durigan Junior
2014-01-11 3:02 ` Joel Brobecker
2014-01-12 3:36 ` Sergio Durigan Junior
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 read-only IMAP folder(s) and NNTP newsgroup(s).