[SCM] archer-sergiodj-stap: Lots of comments, small fixes.

[SCM] archer-sergiodj-stap: Lots of comments, small fixes.

[sergiodj]

The branch, archer-sergiodj-stap has been updated
       via  d909b1c3af91be2fe436ed45662a8301cbd6c86d (commit)
      from  0d92265d7094189e7d0719f3bec48d6dd7697097 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email.

- Log -----------------------------------------------------------------
commit d909b1c3af91be2fe436ed45662a8301cbd6c86d
Author: Sergio Durigan Junior <sergiodj@redhat.com>
Date:   Thu Mar 17 03:50:21 2011 -0300

    Lots of comments, small fixes.
    
    This commit adds lots of comments to the evaluation functions,
    explaining how they work and why they do what they do.  I also found an
    issue with some specific expressions, and fixed it.

-----------------------------------------------------------------------

Summary of changes:
 gdb/stap-probe.c |  212 +++++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 170 insertions(+), 42 deletions(-)

First 500 lines of diff:
diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
index f3c4f33..dd86b62 100644
--- a/gdb/stap-probe.c
+++ b/gdb/stap-probe.c
@@ -646,11 +646,11 @@ stap_is_operator (char *s)
 	  || op == '|' || op == '&' || op == '%' || op == '=');
 }
 
-/* Given **P, this function fetches the value of the register whose
-   name starts in **P.  It also applies any register displacements
-   (e.g., `-4(%eax)'), and indirects the contents of the register
-   (e.g., `(%eax)').  It returns RET if the operation has succeeded,
-   or NULL otherwise.  */
+/* This function fetches the value of the register whose
+   name starts in the expression buffer.  It also applies any register
+   displacements (e.g., `-4(%eax)'), and indirects the contents of the
+   register (e.g., `(%eax)').  It returns RET if the operation has succeeded,
+   or calls `error' otherwise.  */
 
 static struct value *
 stap_fetch_reg_value (struct stap_evaluation_info *eval_info,
@@ -667,8 +667,16 @@ stap_fetch_reg_value (struct stap_evaluation_info *eval_info,
   struct value *ret;
 
   if (!s || !*s)
-    return NULL;
+    /* The function which called us did not check if the expression
+       buffer was empty.  */
+    internal_error (__FILE__, __LINE__,
+		    _("Invalid call to function."));
+
+  /* Valid register name on x86 platforms are:
+
+     [paren]%{a-z0-9}[paren]
 
+     Let's check for that here.  */
   if (*s == '(')
     {
       ++s;
@@ -677,6 +685,8 @@ stap_fetch_reg_value (struct stap_evaluation_info *eval_info,
 	error (_("Invalid register name on expression `%s'."),
 	       eval_info->saved_expr);
       ++s;
+      /* The presence of parenthesis means that we want to indirect
+	 the register.  */
       indirect_p = 1;
     }
   else if (*s == '%')
@@ -695,6 +705,7 @@ stap_fetch_reg_value (struct stap_evaluation_info *eval_info,
     error (_("Trying to apply displacement without indirecting register "
 	     "on expression `%s'."), eval_info->saved_expr);
 
+  /* Ok, let's calculate the size of the register name.  */
   start = s;
   while (isalnum (*s))
     ++s;
@@ -705,19 +716,31 @@ stap_fetch_reg_value (struct stap_evaluation_info *eval_info,
     ++s;
 
   if (len >= REG_NAME_MAX_SIZE)
-    return NULL;
+    error (_("The register name is greater than the supported size "
+	     "on expression `%s'."), eval_info->saved_expr);
 
   strncpy (regname, start, len);
   regname[len] = '\0';
 
+  /* Translating the register name into the corresponding number.  */
   regnum = user_reg_map_name_to_regnum (gdbarch, regname, len);
 
+  if (regnum == -1)
+    error (_("Invalid register name `%s' on expression `%s'."),
+	   regname, eval_info->saved_expr);
+
   ret = value_of_register (regnum, frame);
 
   if (indirect_p)
     {
       struct type *t = NULL;
 
+      /* If the user has specified that the register must be indirected,
+	 we should know what's the correct type to cast it before making
+	 the indirection.  This type corresponds to the bitness specified
+	 before the `@' sign on the argument string, or it defaults to
+	 `unsigned long' if the `@' were not present.  */
+
       switch (bitness)
 	{
 	case STAP_ARG_BITNESS_UNDEFINED:
@@ -758,13 +781,23 @@ stap_fetch_reg_value (struct stap_evaluation_info *eval_info,
       ret = value_ind (ret);
     }
 
+  /* Updating the expression buffer pointer, because we have made
+     some modifications to it before.  */
   eval_info->exp_buf = s;
 
   return ret;
 }
 
+/* This function tries to evaluate a single operand of the expression.
+
+   Single operands can be:
+
+   - unary operators `-' and `~';
+   - integer constants (beginning with `$');
+   - register access, with/out displacement and indirection.  */
+
 static struct value *
-stap_parse_single_operand (struct stap_evaluation_info *eval_info)
+stap_evaluate_single_operand (struct stap_evaluation_info *eval_info)
 {
   struct gdbarch *gdbarch = eval_info->gdbarch;
   struct frame_info *frame = eval_info->frame;
@@ -777,6 +810,21 @@ stap_parse_single_operand (struct stap_evaluation_info *eval_info)
 	{
 	  char c = *eval_info->exp_buf;
 
+	  /* This is a binary operator (either `-' or `~').
+
+	     If it is followed by a parenthesis, and this parenthesis
+	     is NOT followed by a `%', then we are dealing with an expression
+	     like `-(2 + 3)' or `~(2 + 3)'.  We just have to treat separately
+	     and return the result after applying the operation (`-' or `~').
+
+	     If it is followed by a digit, then we have only one choice:  it
+	     is a displacement argument for a register access, like
+	     `-4(%eax)'.  It also means that the operator can *only* be `-',
+	     and the characters immediately after the number *must* be `(%'.
+
+	     If it is followed by a `$', then it is an integer constant, and
+	     we should apply the correct operation to it.  */
+
 	  ++eval_info->exp_buf;
 	  eval_info->exp_buf = skip_spaces (eval_info->exp_buf);
 	  if (*eval_info->exp_buf
@@ -784,10 +832,12 @@ stap_parse_single_operand (struct stap_evaluation_info *eval_info)
 	      && eval_info->exp_buf[1] != '%')
 	    {
 	      struct value *tmp_res;
-	      /* We're not dealing with a register name, but with an
-		 expression like:
 
-		 `-(2)'  */
+	      /* We're not dealing with a register name, but with an
+		 expression like `-(2 + 3)' or `~(2 + 3)'.  We first have
+		 to evaluate the right side of the expression (i.e., the
+		 parenthesis), and then apply the specified operation
+		 (either `-' or `~') to it.  */
 	      tmp_res = stap_evaluate_probe_argument_2 (eval_info,
 							/*lhs=*/NULL,
 							/*prec=*/0);
@@ -797,38 +847,59 @@ stap_parse_single_operand (struct stap_evaluation_info *eval_info)
 	      else
 		res = value_complement (tmp_res);
 	    }
-	  else
+	  else if (isdigit (*eval_info->exp_buf))
 	    {
-	      if (isdigit (*eval_info->exp_buf))
-		{
-		  int number = strtol (eval_info->exp_buf,
-				       &eval_info->exp_buf, 0);
-
-		  eval_info->exp_buf = skip_spaces (eval_info->exp_buf);
+	      int number;
+
+	      /* This is a number, so it MUST be a register displacement.
+		 The only operator allowed here is `-', it MUST be
+		 followed by a number, and the number MUST be followed by
+		 `(%'.  */
+	      if (c != '-')
+		error (_("Invalid operator `%c' for register displacement "
+			 "on expression `%s'."), c, eval_info->saved_expr);
+
+	      number = strtol (eval_info->exp_buf,
+			       &eval_info->exp_buf, 0) * -1;
+
+	      if (!*eval_info->exp_buf
+		  || *eval_info->exp_buf != '('
+		  || (*eval_info->exp_buf == '('
+		      && eval_info->exp_buf[1] != '%'))
+		error (_("Invalid method of indirecting a register on "
+			 "expression `%s'."), eval_info->saved_expr);
+
+	      res
+		= value_from_longest (builtin_type (gdbarch)->builtin_int,
+				      number);
+
+	      res = stap_fetch_reg_value (eval_info, res);
+	    }
+	  else if (*eval_info->exp_buf == '$')
+	    {
+	      int number;
 
-		  res
-		    = value_from_longest (builtin_type (gdbarch)->builtin_int,
-					  number);
+	      /* Last case.  We are dealing with an integer constant, so
+		 we must read it and then apply the necessary operation,
+		 either `-' or `~'.  */
+	      ++eval_info->exp_buf;
+	      number = strtol (eval_info->exp_buf,
+			       &eval_info->exp_buf, 0);
 
-		  if (c == '-')
-		    res = value_neg (res);
-		  else
-		    res = value_complement (res);
+	      res
+		= value_from_longest (builtin_type (gdbarch)->builtin_int,
+				      number);
 
-		  if (!*eval_info->exp_buf
-		      || stap_is_operator (eval_info->exp_buf))
-		    break;
-		  else if (*eval_info->exp_buf == '('
-			   && eval_info->exp_buf[1] == '%')
-		    {
-		      if (c == '~')
-			error (_("Invalid operator `%c' on expression `%s'."),
-			       c, eval_info->saved_expr);
+	      eval_info->exp_buf = skip_spaces (eval_info->exp_buf);
 
-		      res = stap_fetch_reg_value (eval_info, res);
-		    }
-		}
+	      if (c == '-')
+		res = value_neg (res);
+	      else
+		res = value_complement (res);
 	    }
+	  else
+	    error (_("Invalid operand to unary operator `%c' on "
+		     "expression `%s'."), c, eval_info->saved_expr);
 	}
       break;
 
@@ -837,6 +908,8 @@ stap_parse_single_operand (struct stap_evaluation_info *eval_info)
       {
 	int number = strtol (eval_info->exp_buf, &eval_info->exp_buf, 0);
 
+	/* This is a register displacement with a positive value.  We read
+	   the number, and then check for the mandatory `(%' part.  */
 	if (!*eval_info->exp_buf
 	    || !(*eval_info->exp_buf == '('
 		 && eval_info->exp_buf[1] == '%'))
@@ -854,6 +927,8 @@ stap_parse_single_operand (struct stap_evaluation_info *eval_info)
       {
 	int number;
 
+	/* This is an integer constant.  We just have to read the number
+	   and return it.  */
 	++eval_info->exp_buf;
 	eval_info->exp_buf = skip_spaces (eval_info->exp_buf);
 
@@ -868,6 +943,7 @@ stap_parse_single_operand (struct stap_evaluation_info *eval_info)
 
     case '(': case '%':
       {
+	/* Register access, with or without indirection.  */
 	res = stap_fetch_reg_value (eval_info, /*displacement=*/NULL);
       }
     break;
@@ -882,6 +958,14 @@ stap_parse_single_operand (struct stap_evaluation_info *eval_info)
   return res;
 }
 
+/* This function is responsible for checking the necessary type of evaluation
+   depending on what is the next "thing" in the buffer.  Valid values are:
+
+   - Unary operators;
+   - Integer constants;
+   - Register displacement, indirection, and direct access;
+   - Parenthesized operand.  */
+
 static struct value *
 stap_evaluate_conditionally (struct stap_evaluation_info *eval_info)
 {
@@ -893,9 +977,13 @@ stap_evaluate_conditionally (struct stap_evaluation_info *eval_info)
       || (isdigit (*s) && s[1] == '(' && s[2] == '%') /* Displacement.  */
       || (*s == '(' && s[1] == '%') /* Register indirection.  */
       || (*s == '%' && isalpha (s[1]))) /* Register value.  */
-    ret = stap_parse_single_operand (eval_info);
+    /* This is a single operand, so just evaluate it and return.  */
+    ret = stap_evaluate_single_operand (eval_info);
   else if (*s == '(')
     {
+      /* We are dealing with a parenthesized operand.  It means we
+	 have to evaluate it as it was a separate expression, without
+	 left-side or precedence.  */
       ++eval_info->exp_buf;
       eval_info->exp_buf = skip_spaces (eval_info->exp_buf);
 
@@ -921,16 +1009,32 @@ stap_evaluate_probe_argument_2 (struct stap_evaluation_info *eval_info,
 {
   struct value *rhs = NULL;
 
+  /* This is an operator-precedence parser and evaluator.
+
+     We work with left- and right-sides of expressions, and
+     evaluate them depending on the precedence of the operators
+     we find.  */
+
   eval_info->exp_buf = skip_spaces (eval_info->exp_buf);
 
   if (!lhs)
-    /* We have to evaluate the left side of this expression.  */
+    /* We were called without a left-side, either because this is the
+       first call, or because we were called to evaluate a parenthesized
+       expression.  It doesn't really matter; we have to evaluate the
+       left-side in order to continue the process.  */
     lhs = stap_evaluate_conditionally (eval_info);
 
+  /* Start to evaluate the right-side, and to "join" left and right sides
+     depending on the operation specified.
+
+     This loop shall continue until we run out of characters in the input,
+     or until we find a close-parenthesis, which means that we've reached
+     the end of a sub-expression.  */
   while (eval_info->exp_buf
 	 && *eval_info->exp_buf
 	 && *eval_info->exp_buf != ')')
     {
+      char *tmp_exp_buf;
       enum exp_opcode opcode;
       int cur_prec;
 
@@ -938,38 +1042,61 @@ stap_evaluate_probe_argument_2 (struct stap_evaluation_info *eval_info,
 	error (_("Invalid operator `%c' on expression `%s'."),
 	       *eval_info->exp_buf, eval_info->saved_expr);
 
-      stap_get_opcode (&eval_info->exp_buf, &opcode);
+      /* We have to save the current value of the expression buffer because
+	 the `stap_get_opcode' modifies it in order to get the current
+	 operator.  If this operator's precedence is lower than PREC, we
+	 should return and not advance the expression buffer pointer.  */
+      tmp_exp_buf = eval_info->exp_buf;
+      stap_get_opcode (&tmp_exp_buf, &opcode);
 
       cur_prec = stap_get_operator_prec (opcode);
       if (cur_prec < prec)
+	/* If the precedence of the operator that we are seeing now is
+	   lower than the precedence of the first operator seen before
+	   this evaluation process began, it means we should stop evaluating
+	   and return.  */
 	break;
 
+      eval_info->exp_buf = tmp_exp_buf;
       eval_info->exp_buf = skip_spaces (eval_info->exp_buf);
 
+      /* Evaluate the right-side of the expression.  */
       rhs = stap_evaluate_conditionally (eval_info);
 
+      /* While we still have operators, try to evaluate another
+	 right-side, but using the current right-side as a left-side.  */
       while (*eval_info->exp_buf
 	     && stap_is_operator (eval_info->exp_buf))
 	{
 	  enum exp_opcode lookahead_opcode;
 	  int lookahead_prec;
 
-	  stap_get_opcode (&eval_info->exp_buf, &lookahead_opcode);
+	  /* Saving the current expression buffer position.  The explanation
+	     is the same as above.  */
+	  tmp_exp_buf = eval_info->exp_buf;
+	  stap_get_opcode (&tmp_exp_buf, &lookahead_opcode);
 	  lookahead_prec = stap_get_operator_prec (lookahead_opcode);
 
 	  if (lookahead_prec <= prec)
+	    /* If we are dealing with an operator whose precedence is lower
+	       than the first one, just abandon the attempt.  */
 	    break;
 
 	  rhs = stap_evaluate_probe_argument_2 (eval_info,
 						rhs, lookahead_prec);
 	}
 
+      /* Now, "join" both left and right sides into one left-side, using
+	 the specified operator.  */
       lhs = value_binop (lhs, rhs, opcode);
     }
 
   return lhs;
 }
 
+/* This function fills the necessary arguments for the evaluation function
+   to work.  */
+
 #define TEST 0
 static struct value *
 stap_evaluate_probe_argument_1 (struct objfile *objfile,
@@ -983,12 +1110,13 @@ stap_evaluate_probe_argument_1 (struct objfile *objfile,
   struct value *res, *vs[4];
 #if TEST /*TESTING*/
   const char *args[4] = { "$2",
-      "$2 + $3",
+      "$2 + $3 * $4 + $5",
       "-($2 + $3)",
       "(~($2) * $3) / ($1 + $1)" };
   int i;
 #endif
 
+  /* Filling necessary information for evaluation function.  */
   eval_info.saved_expr = s;
   eval_info.exp_buf = s;
   eval_info.gdbarch = get_objfile_arch (objfile);


hooks/post-receive
--
Repository for Project Archer.