* 4.17.86 on AIX
1999-03-24 1:46 ` 4.17.86 on AIX Andrew Cagney
@ 1999-04-01 0:00 ` Andrew Cagney
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Cagney @ 1999-04-01 0:00 UTC (permalink / raw)
To: rodneybrown; +Cc: gdb-patches
Hello,
(Thanks to Rodney Brown for picking this up)
I've tried the below on an AIX 4.1 system. They are based on the
problems that Rodney reported (and bundled a patch for) with an AIX 4.2
system
Andrew
Wed Mar 24 01:01:27 1999 Andrew Cagney <cagney@sludge.cygnus.com>
* rs6000-tdep.c (rs6000_software_single_step): Change SIGNAL to
unsigned int.
From Rodney Brown <rodneybrown@pmsc.com>
* target.h (enum thread_control_capibilities), breakpoint.h (enum
bptype), breakpoint.c (enum insertion_state_t): Strict ISO-C
doesn't allow trailing comma in enum definition.
*** gdb-4.17.86/gdb/breakpoint.c Sun Feb 28 20:51:53 1999
--- aix-4.18/gdb-4.17.86.cagney/gdb/breakpoint.c Wed Mar 24 01:08:55 1999
***************
*** 139,145 ****
typedef enum {
mark_inserted,
! mark_uninserted,
} insertion_state_t;
static int
--- 139,145 ----
typedef enum {
mark_inserted,
! mark_uninserted
} insertion_state_t;
static int
*** gdb-4.17.86/gdb/breakpoint.h Tue Jan 26 15:15:14 1999
--- aix-4.18/gdb-4.17.86.cagney/gdb/breakpoint.h Wed Mar 24 01:08:53 1999
***************
*** 111,117 ****
/* These are catchpoints to implement "catch catch" and "catch throw"
commands for C++ exception handling. */
bp_catch_catch,
! bp_catch_throw,
};
--- 111,117 ----
/* These are catchpoints to implement "catch catch" and "catch throw"
commands for C++ exception handling. */
bp_catch_catch,
! bp_catch_throw
};
*** gdb-4.17.86/gdb/rs6000-tdep.c Mon Feb 1 13:17:19 1999
--- aix-4.18/gdb-4.17.86.cagney/gdb/rs6000-tdep.c Wed Mar 24 01:10:08 1999
***************
*** 204,210 ****
void
rs6000_software_single_step (signal, insert_breakpoints_p)
! enum target_signal signal;
int insert_breakpoints_p;
{
#define INSNLEN(OPCODE) 4
--- 204,210 ----
void
rs6000_software_single_step (signal, insert_breakpoints_p)
! unsigned int signal;
int insert_breakpoints_p;
{
#define INSNLEN(OPCODE) 4
*** gdb-4.17.86/gdb/target.h Tue Jan 19 09:02:55 1999
--- aix-4.18/gdb-4.17.86.cagney/gdb/target.h Wed Mar 24 01:08:56 1999
***************
*** 54,60 ****
enum thread_control_capabilities {
tc_none = 0, /* Default: can't control thread execution. */
tc_schedlock = 1, /* Can lock the thread scheduler. */
! tc_switch = 2, /* Can switch the running thread on demand. */
};
/* Stuff for target_wait. */
--- 54,60 ----
enum thread_control_capabilities {
tc_none = 0, /* Default: can't control thread execution. */
tc_schedlock = 1, /* Can lock the thread scheduler. */
! tc_switch = 2 /* Can switch the running thread on demand. */
};
/* Stuff for target_wait. */
From ac131313@cygnus.com Thu Apr 01 00:00:00 1999
From: Andrew Cagney <ac131313@cygnus.com>
To: gdb-patches@cygnus.com
Subject: Re: Patch to add breakpoint extension to remote protocol
Date: Thu, 01 Apr 1999 00:00:00 -0000
Message-id: <36C91767.B35C2DD7@cygnus.com>
References: <199812160030.QAA05122.cygnus.patches.gdb@jtc.redbacknetworks.com>
X-SW-Source: 1999-q1/msg00040.html
Content-length: 8742
Hello,
"J.T. Conklin" wrote:
> The enclosed patch adds insert and remove breakpoint commands to the
> GDB remote protocol. The protocol changes are as I described in the
> messages I sent to the gdb list over the last few weeks.
Here, finally (:-( ) is some feedback.
Please take a look at each of the issues raised below (you'll notice that I've only commented
on how remote.c implements the protocol).
> I created a X86 embedded target configuration (embed.mt / tm-embed.h)
> that inherits from the sysv config, but then adds the definitions for
> hardware watchpoints and breakpoints. I believe that the other X86
> embeded targets (i386-coff and i386-elf) should also use the embed
> config, but I have not make that change since I am unable to test it.
Perhaphs separate this out into a second patch once the remote.c changes are in.
> 1998-12-15 J.T. Conklin <jtc@redbacknetworks.com>
>
> * remote.c (stub_supports_B): New variable, set to 1 if stub
> supports the new insert and remove breakpoint commands.
> (remote_insert_breakpoint, remote_remove_breakpoint): If
> stub_supports_B is set, attempt to use use the insert and remove
> breakpoint commands. If we receive a empty response, the stub
> does not support the new commands. In that case, stub_supports_B
> is reset and breakpoints will be inserted and removed by writing
> breakpoint insns as before.
> (remote_insert_watchpoint, remote_remove_watchpoint,
> remote_insert_hw_watchpoint, remote_remove_hw_watchpoint): New
> functions, present if TARGET_HAS_HARDWARE_WATCHPOINTS.
> (remote_open_1): Set stub_supports_B to 1.
Re: int stub_supports_B:
remote.c (for stub_supports_P and now stub_supports_B) assumes that any rejected P packet
should be interpreted as as an indication that the P (B) packet is no longer supported. This
assumption is wrong. A target that accepted an initial P (B) packet should continue to accept
such packets and we really should ensure that this occures. (A change of mind is an error and
should result in an abort and not a fallback to some other mechanism).
Could I encourage you to take the oportunity to investigate this. An excert of a patch I've
been playing with is below (please ignore the stub_supports_P -> stub_supports_P_packet
rename):
*** remote.c 1998/10/02 19:54:44 1.162
--- remote.c 1999/02/16 05:12:23
*************** static int remote_address_size;
*** 362,371 ****
static int remote_register_buf_size = 0;
! /* Should we try the 'P' request? If this is set to one when the stub
! doesn't support 'P', the only consequence is some unnecessary traffic. */
! static int stub_supports_P = 1;
/* These are pointers to hook functions that may be set in order to
modify resume/wait behavior for a particular architecture. */
--- 362,384 ----
static int remote_register_buf_size = 0;
! /* Some stubs do not support all packet types. For such packets, they
! are initially marked as SUPPORT_UNKNOWN. Upon first use, depending
! on the response of the remote stub that is changed to either
! SUPPORTED or UNSUPPORTED. It is important that once used
! (especially for B packets) that once used they packet continues to
! be used. */
+ enum packet_support { PACKET_UNSUPPORTED = 0, PACKET_SUPPORT_UNKNOWN,
+ PACKET_SUPPORTED };
+
+ /* Should we use the 'P' packet? */
+ enum packet_support stub_supports_P_packet;
+
+ /* Should we use the B packet? */
+ enum packet_support stub_supports_B_packet;
+
+
/* These are pointers to hook functions that may be set in order to
modify resume/wait behavior for a particular architecture. */
*************** device is attached to the remote system
*** 633,642 ****
}
push_target (target); /* Switch to using remote target now */
! /* Start out by trying the 'P' request to set registers. We set this each
! time that we open a new target so that if the user switches from one
! stub to another, we can (if the target is closed and reopened) cope. */
! stub_supports_P = 1;
general_thread = -2;
cont_thread = -2;
--- 646,657 ----
}
push_target (target); /* Switch to using remote target now */
! /* For optionally supported packets, start out not assuming
! anything. We set this each time that we open a new target so
! that if the user switches from one stub to another, we can (if
! the target is closed and reopened) cope. */
! stub_supports_P_packet = PACKET_SUPPORT_UNKNOWN;
! stub_supports_B_packet = PACKET_SUPPORT_UNKNOWN;
general_thread = -2;
cont_thread = -2;
*************** remote_store_registers (regno)
*** 1091,1097 ****
set_thread (inferior_pid, 1);
! if (regno >= 0 && stub_supports_P)
{
/* Try storing a single register. */
char *regp;
--- 1106,1112 ----
set_thread (inferior_pid, 1);
! if (regno >= 0 && stub_supports_P_packet != PACKET_UNSUPPORTED)
{
/* Try storing a single register. */
char *regp;
*************** remote_store_registers (regno)
*** 1106,1121 ****
}
*p = '\0';
remote_send (buf);
if (buf[0] != '\0')
{
/* The stub understands the 'P' request. We are done. */
return;
}
-
/* The stub does not support the 'P' request. Use 'G' instead,
and don't try using 'P' in the future (it will just waste our
time). */
! stub_supports_P = 0;
}
buf[0] = 'G';
--- 1121,1139 ----
}
*p = '\0';
remote_send (buf);
+ if (stub_supports_P_packet == PACKET_SUPPORTED)
+ /* should we check buf[0] for a protocol error? */
+ return;
if (buf[0] != '\0')
{
/* The stub understands the 'P' request. We are done. */
+ stub_supports_P_packet = PACKET_SUPPORTED;
return;
}
/* The stub does not support the 'P' request. Use 'G' instead,
and don't try using 'P' in the future (it will just waste our
time). */
! stub_supports_P_packet = PACKET_UNSUPPORTED;
}
buf[0] = 'G';
----
> ***************
> *** 90,95 ****
> --- 90,117 ----
> where only part of the data was
> written).
>
> + insert break Bt,AA..AA[,LLLL]
> + or watchpoint t is type: 0 - software breakpoint,
> + 1 - hardware breakpoint, 2 - write
> + watchpoint, 3 - read watchpoint, 4 -
> + access watchpoint;
> + AA..AA is address;
> + LLLL is number of bytes
> + reply OK for success
> + ENN for an error
> + (not supported by all stubs).
> +
> + remove break bt,AA..AA[,LLLL]
> + or watchpoint t is type: 0 - software breakpoint,
> + 1 - hardware breakpoint, 2 - write
> + watchpoint, 3 - read watchpoint, 4 -
> + access watchpoint;
> + AA..AA is address;
> + LLLL is number of bytes
> + reply OK for success
> + ENN for an error
> + (not supported by all stubs).
> +
> continue cAA..AA AA..AA is address to resume
> If AA..AA is omitted,
> resume at same address.
Several questions:
o Must the insert/remote value (t,AAA[,LLL]) for a given breakpoint match? (I assume yes)
o For software breakpoints, would a memory inspection of the breakpoint address return the
pre-breakpoint value? (I assume yes)
Re: sprintf (buf, "B%x,%lx,%lx", type + 2, (long) addr, len);
Have a look at remote_write_bytes() and remote_address_masked(). It handles addresses of up
to 64 bits.
Re: #ifdef TARGET_HAS_HARDWARE_WATCHPOINTS
Don't worry about #ifdefing all the hardware watchpoint code out. I'd instead just make
certain that stub_supports_B is set to something that stops GDB trying to use it.
Re: general
(I'm probably rambling) Should there be a `set remote*' command to control the use of the B
and P packets? Just in case we find that we really do/don't want GDB using either of these
protocol extensions?
Sorry this has taken so long,
Andrew
^ permalink raw reply [flat|nested] 2+ messages in thread