Re: tracepoints implementation: bug in byte code generating.

[Michael Snyder <msnyder@redhat.com>]

Josef Ezra wrote:

> Michael
> 
> 1. Your code is better. I can't resist this modification.

Thanks, I'll check it in.

> 2. In the future, I will try to follow change-log-entry roles,
> sorry for this one.

Thanks, no need to appologize.  It's a learning curve.

> 3. I would like to add commands to the remote protocol, is there a gnu
> routine to standardizes some of those changed?

Ah, you might be on thin ice.
Are your changes confined to the tracepoint protocol?
Those should be LESS controvercial than otherwise, since
they're in "Q" and 'q' packets (special purpose packets
that other stubs can ignore.

Any changes to the remote protocol can be discussed
on this mailing list.

Michael
From msnyder@redhat.com Thu Oct 19 11:15:00 2000
From: Michael Snyder <msnyder@redhat.com>
To: gdb-patches@sourceware.cygnus.com
Subject: Re: [RFA] addresses/pointers vs numbers and expression evaluation
Date: Thu, 19 Oct 2000 11:15:00 -0000
Message-id: <39EF36EF.10B5@redhat.com>
References: <200010121406.KAA21106@texas.cygnus.com>
X-SW-Source: 2000-10/msg00122.html
Content-length: 14519

David, 
The breakpoint and tracepoint changes are approved.
			Michael

David Taylor wrote:
> 
> The following patch concerns bugs brought to light during a port of
> gdb to a target with separate instruction and data.
> 
> The bugs will potentially impact any port which defines
> POINTER_TO_ADDRESS to have any value other than the default value.
> 
> There are undoubtably additional bugs still lurking in the expression
> evaluation code.
> 
> Several places in gdb it calls
> 
>     parse_and_eval_address
> 
> when the expression is *NOT* an address, but rather is just a number!
> For example, it calls parse_and_eval_address when you type:
> 
>     set height 24
> 
> For this processor, treating 24 as a pointer resulted in an address of
> 0x01000018 -- 16 meg plus 24!  Similarly, it does this for the count
> given to the continue and step commands as well as a few other places.
> 
> To fix this, I have create a new function called parse_and_eval_long
> and changed 17 of the calls to parse_and_eval_address.
> 
> I have tested this on solaris native and on solaris x d10v (using the
> simulator).  There were no regressions.
> 
> Here's the ChangeLog entry and the patch:
> 
>         * parse.c (parse_and_eval_long): New function.
>         (value.h): Declare it.
> 
>         * breakpoint.c (breakpoints_info, maintenance_info_breakpoints):
>         Call parse_and_eval_long, not parse_and_eval_address.
>         * command.c (do_setshow_command): Ditto.
>         * infcmd.c (step_1, signal_command, continue_command): Ditto.
>         * infrun.c (signals_info): Ditto.
>         * stack.c (set_backtrace_limit_command, backtrace_command_1,
>         up_silently_base, down_silently_base): Ditto.
>         * tracepoints.c (tracepoints_info, trace_find_command,
>         trace_find_tracepoint_command): Ditto.
>         * valprint.c (set_radix): Ditto.
>         * values (show_values): Ditto.
> 
> Index: breakpoint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/breakpoint.c,v
> retrieving revision 1.17
> diff -c -r1.17 breakpoint.c
> *** breakpoint.c        2000/08/30 00:58:58     1.17
> --- breakpoint.c        2000/10/12 13:57:06
> ***************
> *** 3638,3644 ****
>     int bnum = -1;
> 
>     if (bnum_exp)
> !     bnum = parse_and_eval_address (bnum_exp);
> 
>     breakpoint_1 (bnum, 0);
>   }
> --- 3638,3644 ----
>     int bnum = -1;
> 
>     if (bnum_exp)
> !     bnum = parse_and_eval_long (bnum_exp);
> 
>     breakpoint_1 (bnum, 0);
>   }
> ***************
> *** 3650,3656 ****
>     int bnum = -1;
> 
>     if (bnum_exp)
> !     bnum = parse_and_eval_address (bnum_exp);
> 
>     breakpoint_1 (bnum, 1);
>   }
> --- 3650,3656 ----
>     int bnum = -1;
> 
>     if (bnum_exp)
> !     bnum = parse_and_eval_long (bnum_exp);
> 
>     breakpoint_1 (bnum, 1);
>   }
> Index: command.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/command.c,v
> retrieving revision 1.17
> diff -c -r1.17 command.c
> *** command.c   2000/08/08 00:17:39     1.17
> --- command.c   2000/10/12 13:57:08
> ***************
> *** 1622,1628 ****
>         case var_uinteger:
>           if (arg == NULL)
>             error_no_arg ("integer to set it to.");
> !         *(unsigned int *) c->var = parse_and_eval_address (arg);
>           if (*(unsigned int *) c->var == 0)
>             *(unsigned int *) c->var = UINT_MAX;
>           break;
> --- 1622,1628 ----
>         case var_uinteger:
>           if (arg == NULL)
>             error_no_arg ("integer to set it to.");
> !         *(unsigned int *) c->var = parse_and_eval_long (arg);
>           if (*(unsigned int *) c->var == 0)
>             *(unsigned int *) c->var = UINT_MAX;
>           break;
> ***************
> *** 1631,1637 ****
>             unsigned int val;
>             if (arg == NULL)
>               error_no_arg ("integer to set it to.");
> !           val = parse_and_eval_address (arg);
>             if (val == 0)
>               *(int *) c->var = INT_MAX;
>             else if (val >= INT_MAX)
> --- 1631,1637 ----
>             unsigned int val;
>             if (arg == NULL)
>               error_no_arg ("integer to set it to.");
> !           val = parse_and_eval_long (arg);
>             if (val == 0)
>               *(int *) c->var = INT_MAX;
>             else if (val >= INT_MAX)
> ***************
> *** 1643,1649 ****
>         case var_zinteger:
>           if (arg == NULL)
>             error_no_arg ("integer to set it to.");
> !         *(int *) c->var = parse_and_eval_address (arg);
>           break;
>         case var_enum:
>           {
> --- 1643,1649 ----
>         case var_zinteger:
>           if (arg == NULL)
>             error_no_arg ("integer to set it to.");
> !         *(int *) c->var = parse_and_eval_long (arg);
>           break;
>         case var_enum:
>           {
> Index: eval.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/eval.c,v
> retrieving revision 1.8
> diff -c -r1.8 eval.c
> *** eval.c      2000/09/01 23:50:17     1.8
> --- eval.c      2000/10/12 13:57:09
> ***************
> *** 103,108 ****
> --- 103,123 ----
>     return addr;
>   }
> 
> + /* Like parse_and_eval_address, but treats the value of the expression
> +    as an integer, not an address, returns a LONGEST, not a CORE_ADDR */
> + LONGEST
> + parse_and_eval_long (char *exp)
> + {
> +   struct expression *expr = parse_expression (exp);
> +   register LONGEST retval;
> +   register struct cleanup *old_chain =
> +     make_cleanup (free_current_contents, &expr);
> +
> +   retval = value_as_long (evaluate_expression (expr));
> +   do_cleanups (old_chain);
> +   return (retval);
> + }
> +
>   value_ptr
>   parse_and_eval (char *exp)
>   {
> Index: infcmd.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infcmd.c,v
> retrieving revision 1.11
> diff -c -r1.11 infcmd.c
> *** infcmd.c    2000/09/02 00:07:13     1.11
> --- infcmd.c    2000/10/12 13:57:11
> ***************
> *** 381,387 ****
>         while (num != 0)
>         {
>           set_ignore_count (num,
> !                           parse_and_eval_address (proc_count_exp) - 1,
>                             from_tty);
>           /* set_ignore_count prints a message ending with a period.
>              So print two spaces before "Continuing.".  */
> --- 381,387 ----
>         while (num != 0)
>         {
>           set_ignore_count (num,
> !                           parse_and_eval_long (proc_count_exp) - 1,
>                             from_tty);
>           /* set_ignore_count prints a message ending with a period.
>              So print two spaces before "Continuing.".  */
> ***************
> *** 465,471 ****
>         async_disable_stdin ();
>       }
> 
> !   count = count_string ? parse_and_eval_address (count_string) : 1;
> 
>     if (!single_inst || skip_subroutines)               /* leave si command alone */
>       {
> --- 465,471 ----
>         async_disable_stdin ();
>       }
> 
> !   count = count_string ? parse_and_eval_long (count_string) : 1;
> 
>     if (!single_inst || skip_subroutines)               /* leave si command alone */
>       {
> ***************
> *** 777,783 ****
>     if (oursig == TARGET_SIGNAL_UNKNOWN)
>       {
>         /* No, try numeric.  */
> !       int num = parse_and_eval_address (signum_exp);
> 
>         if (num == 0)
>         oursig = TARGET_SIGNAL_0;
> --- 777,783 ----
>     if (oursig == TARGET_SIGNAL_UNKNOWN)
>       {
>         /* No, try numeric.  */
> !       int num = parse_and_eval_long (signum_exp);
> 
>         if (num == 0)
>         oursig = TARGET_SIGNAL_0;
> Index: infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.18
> diff -c -r1.18 infrun.c
> *** infrun.c    2000/09/02 00:08:05     1.18
> --- infrun.c    2000/10/12 13:57:15
> ***************
> *** 3899,3905 ****
>         {
>           /* No, try numeric.  */
>           oursig =
> !           target_signal_from_command (parse_and_eval_address (signum_exp));
>         }
>         sig_print_info (oursig);
>         return;
> --- 3899,3905 ----
>         {
>           /* No, try numeric.  */
>           oursig =
> !           target_signal_from_command (parse_and_eval_long (signum_exp));
>         }
>         sig_print_info (oursig);
>         return;
> Index: stack.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/stack.c,v
> retrieving revision 1.7
> diff -c -r1.7 stack.c
> *** stack.c     2000/07/30 01:48:27     1.7
> --- stack.c     2000/10/12 13:57:17
> ***************
> *** 1038,1044 ****
>   static void
>   set_backtrace_limit_command (char *count_exp, int from_tty)
>   {
> !   int count = parse_and_eval_address (count_exp);
> 
>     if (count < 0)
>       error ("Negative argument not meaningful as backtrace limit.");
> --- 1038,1044 ----
>   static void
>   set_backtrace_limit_command (char *count_exp, int from_tty)
>   {
> !   int count = parse_and_eval_long (count_exp);
> 
>     if (count < 0)
>       error ("Negative argument not meaningful as backtrace limit.");
> ***************
> *** 1086,1092 ****
>     trailing_level = 0;
>     if (count_exp)
>       {
> !       count = parse_and_eval_address (count_exp);
>         if (count < 0)
>         {
>           struct frame_info *current;
> --- 1086,1092 ----
>     trailing_level = 0;
>     if (count_exp)
>       {
> !       count = parse_and_eval_long (count_exp);
>         if (count < 0)
>         {
>           struct frame_info *current;
> ***************
> *** 1740,1746 ****
>     register struct frame_info *fi;
>     int count = 1, count1;
>     if (count_exp)
> !     count = parse_and_eval_address (count_exp);
>     count1 = count;
> 
>     if (target_has_stack == 0 || selected_frame == 0)
> --- 1740,1746 ----
>     register struct frame_info *fi;
>     int count = 1, count1;
>     if (count_exp)
> !     count = parse_and_eval_long (count_exp);
>     count1 = count;
> 
>     if (target_has_stack == 0 || selected_frame == 0)
> ***************
> *** 1777,1783 ****
>     register struct frame_info *frame;
>     int count = -1, count1;
>     if (count_exp)
> !     count = -parse_and_eval_address (count_exp);
>     count1 = count;
> 
>     if (target_has_stack == 0 || selected_frame == 0)
> --- 1777,1783 ----
>     register struct frame_info *frame;
>     int count = -1, count1;
>     if (count_exp)
> !     count = -parse_and_eval_long (count_exp);
>     count1 = count;
> 
>     if (target_has_stack == 0 || selected_frame == 0)
> Index: tracepoint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/tracepoint.c,v
> retrieving revision 1.10
> diff -c -r1.10 tracepoint.c
> *** tracepoint.c        2000/09/01 00:12:10     1.10
> --- tracepoint.c        2000/10/12 13:57:19
> ***************
> *** 465,471 ****
>     int tpnum = -1;
> 
>     if (tpnum_exp)
> !     tpnum = parse_and_eval_address (tpnum_exp);
> 
>     ALL_TRACEPOINTS (t)
>       if (tpnum == -1 || tpnum == t->number)
> --- 465,471 ----
>     int tpnum = -1;
> 
>     if (tpnum_exp)
> !     tpnum = parse_and_eval_long (tpnum_exp);
> 
>     ALL_TRACEPOINTS (t)
>       if (tpnum == -1 || tpnum == t->number)
> ***************
> *** 1986,1992 ****
>           frameno = traceframe_number - 1;
>         }
>         else
> !       frameno = parse_and_eval_address (args);
> 
>         if (frameno < -1)
>         error ("invalid input (%d is less than zero)", frameno);
> --- 1986,1992 ----
>           frameno = traceframe_number - 1;
>         }
>         else
> !       frameno = parse_and_eval_long (args);
> 
>         if (frameno < -1)
>         error ("invalid input (%d is less than zero)", frameno);
> Index: valprint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/valprint.c,v
> retrieving revision 1.6
> diff -c -r1.6 valprint.c
> *** valprint.c  2000/07/30 01:48:27     1.6
> --- valprint.c  2000/10/12 13:57:20
> ***************
> *** 1416,1422 ****
>   {
>     unsigned radix;
> 
> !   radix = (arg == NULL) ? 10 : parse_and_eval_address (arg);
>     set_output_radix_1 (0, radix);
>     set_input_radix_1 (0, radix);
>     if (from_tty)
> --- 1416,1422 ----
>   {
>     unsigned radix;
> 
> !   radix = (arg == NULL) ? 10 : parse_and_eval_long (arg);
>     set_output_radix_1 (0, radix);
>     set_input_radix_1 (0, radix);
>     if (from_tty)
> Index: value.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/value.h,v
> retrieving revision 1.10
> diff -c -r1.10 value.h
> *** value.h     2000/09/04 08:29:25     1.10
> --- value.h     2000/10/12 13:57:21
> ***************
> *** 410,415 ****
> --- 410,417 ----
> 
>   extern CORE_ADDR parse_and_eval_address_1 (char **expptr);
> 
> + extern LONGEST parse_and_eval_long (char *exp);
> +
>   extern value_ptr access_value_history (int num);
> 
>   extern value_ptr value_of_internalvar (struct internalvar *var);
> Index: values.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/values.c,v
> retrieving revision 1.9
> diff -c -r1.9 values.c
> *** values.c    2000/07/30 01:48:27     1.9
> --- values.c    2000/10/12 13:57:22
> ***************
> *** 351,357 ****
>         /* "info history +" should print from the stored position.
>            "info history <exp>" should print around value number <exp>.  */
>         if (num_exp[0] != '+' || num_exp[1] != '\0')
> !       num = parse_and_eval_address (num_exp) - 5;
>       }
>     else
>       {
> --- 351,357 ----
>         /* "info history +" should print from the stored position.
>            "info history <exp>" should print around value number <exp>.  */
>         if (num_exp[0] != '+' || num_exp[1] != '\0')
> !       num = parse_and_eval_long (num_exp) - 5;
>       }
>     else
>       {
From jezra@emc.com Thu Oct 19 11:23:00 2000
From: "Josef Ezra" <jezra@emc.com>
To: "Michael Snyder" <msnyder@redhat.com>
Cc: <gdb-patches@sources.redhat.com>, "ezra, josef" <ezra_josef@emc.com>, <shagam@emc.com>, <sgordon@emc.com>
Subject: Re: tracepoints implementation: bug in byte code generating.
Date: Thu, 19 Oct 2000 11:23:00 -0000
Message-id: <006101c039f9$df09df60$6c219fa8@lss.emc.com>
References: <00c801c02c9a$fe671c90$6c219fa8@lss.emc.com> <39D8D1CE.3059@redhat.com> <004e01c039d8$774c1d50$6c219fa8@lss.emc.com> <39EF3020.3F6A@redhat.com> <005b01c039f6$f46b1390$6c219fa8@lss.emc.com> <39EF381C.7EE6@redhat.com>
X-SW-Source: 2000-10/msg00123.html
Content-length: 1077

> Josef Ezra wrote:
>
> > Michael
> >
> > 1. Your code is better. I can't resist this modification.
>
> Thanks, I'll check it in.
>
> > 2. In the future, I will try to follow change-log-entry roles,
> > sorry for this one.
>
> Thanks, no need to appologize.  It's a learning curve.
>
> > 3. I would like to add commands to the remote protocol, is there a gnu
> > routine to standardizes some of those changed?
>
> Ah, you might be on thin ice.
> Are your changes confined to the tracepoint protocol?
> Those should be LESS controvercial than otherwise, since
> they're in "Q" and 'q' packets (special purpose packets
> that other stubs can ignore.
>
> Any changes to the remote protocol can be discussed
> on this mailing list.
>
> Michael
>

Actually I had in mind a 'show protocol version' command (for backward
compatibility) and a special read memory command (like "mxxxx,lll") which
can be used at target side for special reads (in my case, it will show
values in different memory scope).

If it will turn out to be very useful, I will suggest it in this forum.

Josef