* [RFA] enable software single step on alpha-osf
@ 2002-07-18 13:55 Joel Brobecker
2002-07-22 4:19 ` Eli Zaretskii
` (2 more replies)
0 siblings, 3 replies; 24+ messages in thread
From: Joel Brobecker @ 2002-07-18 13:55 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 2926 bytes --]
One of our customers who is using multi-processor alpha-osf machines
has informed us that the next/step operations do not function correctly
on their machine.
Among other things, they saw messages like these:
Program received signal SIGSEGV, Segmentation fault
<__gnat_malloc> (size=8208) at s-memory.adb:92
or
Program received signal SIGTRAP, Trace/breakpoint trap.
0x1201ee89c in mate.types.data....
or sometimes the behavior of the program becomes odd, and the
debugger stops at a bizarre location...
It turned out that procfs does not seem to be handling multi-cpu machines
very well, as far as next/step operations are concerned. We could not
test this ourselves, because we don't have a multi-cpu alpha-osf
machine, but as soon as we enabled the software-single-step capability,
most these problems were gone...
Other problems surfaced, however. The SIGSEGVs and the SIGTRAPs
disappeared, but "next" sometimes stopped at the wrong location.
The following patch enables software single stepping. It also fixes
all the problems I found when comparing the testsuite results before and
after the switch. The test results are now identical before and after my
changes. For the record, here is a summary of the results I get:
# of expected passes 7246
# of unexpected failures 680
# of unexpected successes 5
# of expected failures 149
# of unresolved testcases 59
# of untested testcases 3
# of unsupported tests 2
I also verified on a linux machine, where software
single-stepping is not enabled, that no regression was introduced.
It would be interesting to see how this change influences the results
of alpha-netbsd. It should improve them.
Ok to commit?
2002-07-18 Joel Brobecker <brobecker@gnat.com>
* alpha-osf1-tdep.c (alpha_osf1_init_abi): Unfortunately,
procfs appears to be broken when debugging on multi-processor
machines. So enable software single stepping in order to avoid
using the procfs interface to do next/step operations, using
internal breakpoints instead.
* infrun.c (handle_inferior_event): When receiving a SIGTRAP
signal, check whether we hit a breakpoint before checking for a
single step breakpoint. Otherwise, GDB fails to notice that a
breakpoint has been hit when stepping onto a breakpoint.
Readjust the stop_pc by DECR_PC_AFTER_BREAK when hitting a
single step breakpoint, to make this pc address equal to the
value it would have if the system stepping capability was used.
* breakpoint.c (bpstat_stop_status): Do not adjust the PC
address by DECR_PC_AFTER_BREAK when software single step is
in use for this architecture, as this has already been taken
care of in handle_inferior_event().
--
Joel
[-- Attachment #2: sw_single_step.diff --]
[-- Type: text/plain, Size: 3685 bytes --]
Index: alpha-osf1-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/alpha-osf1-tdep.c,v
retrieving revision 1.5
diff -c -3 -p -r1.5 alpha-osf1-tdep.c
*** alpha-osf1-tdep.c 21 May 2002 15:36:02 -0000 1.5
--- alpha-osf1-tdep.c 18 Jul 2002 20:29:34 -0000
*************** alpha_osf1_init_abi (struct gdbarch_info
*** 58,63 ****
--- 58,67 ----
struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
set_gdbarch_pc_in_sigtramp (gdbarch, alpha_osf1_pc_in_sigtramp);
+ /* The next/step support via procfs on OSF1 is broken when running
+ on multi-processor machines. We need to use software single stepping
+ instead. */
+ set_gdbarch_software_single_step (gdbarch, alpha_software_single_step);
tdep->skip_sigtramp_frame = alpha_osf1_skip_sigtramp_frame;
tdep->sigcontext_addr = alpha_osf1_sigcontext_addr;
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.63
diff -c -3 -p -r1.63 infrun.c
*** infrun.c 18 Jul 2002 17:53:49 -0000 1.63
--- infrun.c 18 Jul 2002 20:29:36 -0000
*************** handle_inferior_event (struct execution_
*** 1826,1835 ****
if (stop_signal == TARGET_SIGNAL_TRAP)
{
! if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
! ecs->random_signal = 0;
! else if (breakpoints_inserted
! && breakpoint_here_p (stop_pc - DECR_PC_AFTER_BREAK))
{
ecs->random_signal = 0;
if (!breakpoint_thread_match (stop_pc - DECR_PC_AFTER_BREAK,
--- 1826,1836 ----
if (stop_signal == TARGET_SIGNAL_TRAP)
{
! /* Check if a regular breakpoint has been hit before checking
! for a potential single step breakpoint. Otherwise, GDB will
! not see this breakpoint hit when stepping onto breakpoints. */
! if (breakpoints_inserted
! && breakpoint_here_p (stop_pc - DECR_PC_AFTER_BREAK))
{
ecs->random_signal = 0;
if (!breakpoint_thread_match (stop_pc - DECR_PC_AFTER_BREAK,
*************** handle_inferior_event (struct execution_
*** 1885,1890 ****
--- 1886,1901 ----
}
}
}
+ else if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+ {
+ /* Readjust the stop_pc as it is off by DECR_PC_AFTER_BREAK
+ compared to the value it would have if the system stepping
+ capability was used. This allows the rest of the code in
+ this function to use this address without having to worry
+ whether software single step is in use or not. */
+ stop_pc -= DECR_PC_AFTER_BREAK;
+ ecs->random_signal = 0;
+ }
}
else
ecs->random_signal = 1;
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.78
diff -c -3 -p -r1.78 breakpoint.c
*** breakpoint.c 26 Jun 2002 05:20:04 -0000 1.78
--- breakpoint.c 18 Jul 2002 20:29:42 -0000
*************** bpstat_stop_status (CORE_ADDR *pc, int n
*** 2429,2436 ****
trap event. For a trace/singlestep trap event, we would
not want to subtract DECR_PC_AFTER_BREAK from the PC. */
! bp_addr = *pc - (not_a_breakpoint && !SOFTWARE_SINGLE_STEP_P () ?
! 0 : DECR_PC_AFTER_BREAK);
ALL_BREAKPOINTS_SAFE (b, temp)
{
--- 2429,2435 ----
trap event. For a trace/singlestep trap event, we would
not want to subtract DECR_PC_AFTER_BREAK from the PC. */
! bp_addr = *pc - (not_a_breakpoint ? 0 : DECR_PC_AFTER_BREAK);
ALL_BREAKPOINTS_SAFE (b, temp)
{
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-07-18 13:55 [RFA] enable software single step on alpha-osf Joel Brobecker
@ 2002-07-22 4:19 ` Eli Zaretskii
2002-07-25 16:38 ` Andrew Cagney
2002-08-04 16:42 ` Andrew Cagney
2 siblings, 0 replies; 24+ messages in thread
From: Eli Zaretskii @ 2002-07-22 4:19 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
On Thu, 18 Jul 2002, Joel Brobecker wrote:
> The following patch enables software single stepping. It also fixes
> all the problems I found when comparing the testsuite results before and
> after the switch.
Thanks.
If this patch is approved, I suggest an entry in NEWS about it.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-07-18 13:55 [RFA] enable software single step on alpha-osf Joel Brobecker
2002-07-22 4:19 ` Eli Zaretskii
@ 2002-07-25 16:38 ` Andrew Cagney
2002-07-26 10:17 ` Jason R Thorpe
2002-08-04 16:42 ` Andrew Cagney
2 siblings, 1 reply; 24+ messages in thread
From: Andrew Cagney @ 2002-07-25 16:38 UTC (permalink / raw)
To: Joel Brobecker, Jason R Thorpe; +Cc: gdb-patches
> 2002-07-18 Joel Brobecker <brobecker@gnat.com>
>
> * alpha-osf1-tdep.c (alpha_osf1_init_abi): Unfortunately,
> procfs appears to be broken when debugging on multi-processor
> machines. So enable software single stepping in order to avoid
> using the procfs interface to do next/step operations, using
> internal breakpoints instead.
Unless Jason Thorpe objects, this part of the change is approved. The
other alternative would be to select this based on the OS.
Software single step needs to be moved to the target vector.
I need to look some more at the other change. !*&(*!@(^#*
DECR_PC_AFTER_BREAK.
Andrew
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-07-25 16:38 ` Andrew Cagney
@ 2002-07-26 10:17 ` Jason R Thorpe
2002-07-31 10:28 ` Joel Brobecker
0 siblings, 1 reply; 24+ messages in thread
From: Jason R Thorpe @ 2002-07-26 10:17 UTC (permalink / raw)
To: Andrew Cagney; +Cc: Joel Brobecker, gdb-patches
On Thu, Jul 25, 2002 at 07:36:06PM -0400, Andrew Cagney wrote:
> > 2002-07-18 Joel Brobecker <brobecker@gnat.com>
> >
> > * alpha-osf1-tdep.c (alpha_osf1_init_abi): Unfortunately,
> > procfs appears to be broken when debugging on multi-processor
> > machines. So enable software single stepping in order to avoid
> > using the procfs interface to do next/step operations, using
> > internal breakpoints instead.
>
> Unless Jason Thorpe objects, this part of the change is approved. The
> other alternative would be to select this based on the OS.
No objections from me.
Note that the alpha-netbsd target already uses software single-step.
--
-- Jason R. Thorpe <thorpej@wasabisystems.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-07-26 10:17 ` Jason R Thorpe
@ 2002-07-31 10:28 ` Joel Brobecker
0 siblings, 0 replies; 24+ messages in thread
From: Joel Brobecker @ 2002-07-31 10:28 UTC (permalink / raw)
To: Jason R Thorpe, Andrew Cagney, gdb-patches
On Fri, Jul 26, 2002 at 06:12:00AM -0700, Jason R Thorpe wrote:
> > Unless Jason Thorpe objects, this part of the change is approved. The
> > other alternative would be to select this based on the OS.
>
> No objections from me.
I forgot to mention: I prefer to wait for the rest of the patch to be
approved before enabling software single step on alpha-osf, because
without the other fixes, although it greatly improves the situation with
multi-processor machines, stepping on mono-processors does not work as
well as previously.
--
Joel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-07-18 13:55 [RFA] enable software single step on alpha-osf Joel Brobecker
2002-07-22 4:19 ` Eli Zaretskii
2002-07-25 16:38 ` Andrew Cagney
@ 2002-08-04 16:42 ` Andrew Cagney
2002-08-05 11:49 ` Joel Brobecker
2 siblings, 1 reply; 24+ messages in thread
From: Andrew Cagney @ 2002-08-04 16:42 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
(Not just a patch involving DECR_PC_AFTER_BREAK but also something that
involves WFI, outch!)
Can you confirm that the code is encountering a situtation where both
breakpoints_inserted and singlestep_breakpoints_inserted_p are true. I
think this occures when doing a single step after stepping off of a
breakpoint. When single stepping off a breakpoint, only
singlestep_breakpoints_inserted_p would be true.
If this is the case then the comments should make mention of it. It
also makes the re-ordered if statement part of the patch correct.
The second part of the change is more tricky:
+ stop_pc -= DECR_PC_AFTER_BREAK;
Is it fixing any failures? Software singlestep can be handled in two
different ways:
- as a breakpoint
- as a hardware single step
and which is prefered decides if/when there should be a decrement.
Anyway, the thing I'm having trouble convincing myself that there can't
be a double decrement -- eg for a hardware watchpoint or similar.
Andrew
PS: A patch to purge the macro SHIFT_INST_REGS is pre-approved :-)
> Other problems surfaced, however. The SIGSEGVs and the SIGTRAPs
> disappeared, but "next" sometimes stopped at the wrong location.
>
> The following patch enables software single stepping. It also fixes
> all the problems I found when comparing the testsuite results before and
> after the switch. The test results are now identical before and after my
> changes. For the record, here is a summary of the results I get:
>
> # of expected passes 7246
> # of unexpected failures 680
> # of unexpected successes 5
> # of expected failures 149
> # of unresolved testcases 59
> # of untested testcases 3
> # of unsupported tests 2
>
> I also verified on a linux machine, where software
> single-stepping is not enabled, that no regression was introduced.
>
> It would be interesting to see how this change influences the results
> of alpha-netbsd. It should improve them.
>
> Ok to commit?
>
> 2002-07-18 Joel Brobecker <brobecker@gnat.com>
>
> * alpha-osf1-tdep.c (alpha_osf1_init_abi): Unfortunately,
> procfs appears to be broken when debugging on multi-processor
> machines. So enable software single stepping in order to avoid
> using the procfs interface to do next/step operations, using
> internal breakpoints instead.
>
> * infrun.c (handle_inferior_event): When receiving a SIGTRAP
> signal, check whether we hit a breakpoint before checking for a
> single step breakpoint. Otherwise, GDB fails to notice that a
> breakpoint has been hit when stepping onto a breakpoint.
> Readjust the stop_pc by DECR_PC_AFTER_BREAK when hitting a
> single step breakpoint, to make this pc address equal to the
> value it would have if the system stepping capability was used.
>
> * breakpoint.c (bpstat_stop_status): Do not adjust the PC
> address by DECR_PC_AFTER_BREAK when software single step is
> in use for this architecture, as this has already been taken
> care of in handle_inferior_event().
>
> -- Joel
>
>
>
> Index: alpha-osf1-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/alpha-osf1-tdep.c,v
> retrieving revision 1.5
> diff -c -3 -p -r1.5 alpha-osf1-tdep.c
> *** alpha-osf1-tdep.c 21 May 2002 15:36:02 -0000 1.5
> --- alpha-osf1-tdep.c 18 Jul 2002 20:29:34 -0000
> *************** alpha_osf1_init_abi (struct gdbarch_info
> *** 58,63 ****
> --- 58,67 ----
> struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>
> set_gdbarch_pc_in_sigtramp (gdbarch, alpha_osf1_pc_in_sigtramp);
> + /* The next/step support via procfs on OSF1 is broken when running
> + on multi-processor machines. We need to use software single stepping
> + instead. */
> + set_gdbarch_software_single_step (gdbarch, alpha_software_single_step);
>
> tdep->skip_sigtramp_frame = alpha_osf1_skip_sigtramp_frame;
> tdep->sigcontext_addr = alpha_osf1_sigcontext_addr;
> Index: infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.63
> diff -c -3 -p -r1.63 infrun.c
> *** infrun.c 18 Jul 2002 17:53:49 -0000 1.63
> --- infrun.c 18 Jul 2002 20:29:36 -0000
> *************** handle_inferior_event (struct execution_
> *** 1826,1835 ****
>
> if (stop_signal == TARGET_SIGNAL_TRAP)
> {
> ! if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
> ! ecs->random_signal = 0;
> ! else if (breakpoints_inserted
> ! && breakpoint_here_p (stop_pc - DECR_PC_AFTER_BREAK))
> {
> ecs->random_signal = 0;
> if (!breakpoint_thread_match (stop_pc - DECR_PC_AFTER_BREAK,
> --- 1826,1836 ----
>
> if (stop_signal == TARGET_SIGNAL_TRAP)
> {
> ! /* Check if a regular breakpoint has been hit before checking
> ! for a potential single step breakpoint. Otherwise, GDB will
> ! not see this breakpoint hit when stepping onto breakpoints. */
> ! if (breakpoints_inserted
> ! && breakpoint_here_p (stop_pc - DECR_PC_AFTER_BREAK))
> {
> ecs->random_signal = 0;
> if (!breakpoint_thread_match (stop_pc - DECR_PC_AFTER_BREAK,
> *************** handle_inferior_event (struct execution_
> *** 1885,1890 ****
> --- 1886,1901 ----
> }
> }
> }
> + else if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
> + {
> + /* Readjust the stop_pc as it is off by DECR_PC_AFTER_BREAK
> + compared to the value it would have if the system stepping
> + capability was used. This allows the rest of the code in
> + this function to use this address without having to worry
> + whether software single step is in use or not. */
> + stop_pc -= DECR_PC_AFTER_BREAK;
> + ecs->random_signal = 0;
> + }
> }
> else
> ecs->random_signal = 1;
> Index: breakpoint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/breakpoint.c,v
> retrieving revision 1.78
> diff -c -3 -p -r1.78 breakpoint.c
> *** breakpoint.c 26 Jun 2002 05:20:04 -0000 1.78
> --- breakpoint.c 18 Jul 2002 20:29:42 -0000
> *************** bpstat_stop_status (CORE_ADDR *pc, int n
> *** 2429,2436 ****
> trap event. For a trace/singlestep trap event, we would
> not want to subtract DECR_PC_AFTER_BREAK from the PC. */
>
> ! bp_addr = *pc - (not_a_breakpoint && !SOFTWARE_SINGLE_STEP_P () ?
> ! 0 : DECR_PC_AFTER_BREAK);
>
> ALL_BREAKPOINTS_SAFE (b, temp)
> {
> --- 2429,2435 ----
> trap event. For a trace/singlestep trap event, we would
> not want to subtract DECR_PC_AFTER_BREAK from the PC. */
>
> ! bp_addr = *pc - (not_a_breakpoint ? 0 : DECR_PC_AFTER_BREAK);
>
> ALL_BREAKPOINTS_SAFE (b, temp)
> {
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-08-04 16:42 ` Andrew Cagney
@ 2002-08-05 11:49 ` Joel Brobecker
2002-08-05 20:01 ` Andrew Cagney
2002-08-16 10:11 ` Andrew Cagney
0 siblings, 2 replies; 24+ messages in thread
From: Joel Brobecker @ 2002-08-05 11:49 UTC (permalink / raw)
To: Andrew Cagney; +Cc: gdb-patches
> Can you confirm that the code is encountering a situtation where both
> breakpoints_inserted and singlestep_breakpoints_inserted_p are true. I
> think this occures when doing a single step after stepping off of a
> breakpoint. When single stepping off a breakpoint, only
> singlestep_breakpoints_inserted_p would be true.
>
> If this is the case then the comments should make mention of it. It
> also makes the re-ordered if statement part of the patch correct.
Yes, I can confirm this, and this should happen fairly often: suppose
you have inserted a regular breakpoint in your program, anywhere, and
then do a single step. Before resuming the inferior, GDB will re-insert
the breakpoints, and set breakpoints_inserted. At the same time, because
we are doing a s/w single step, singlestep_breakpoints_inserted_p will
be set too. Did I miss something?
As for the re-ordering, I made it because I saw some regressions in the
testsuite after switching to s/w single step. Unfortunately, I don't
remember which ones, I would have to rerun the testsuite without this
change to find them again. But the following comment explains in which
cases the re-ordering was necessary:
/* Check if a regular breakpoint has been hit before checking
for a potential single step breakpoint. Otherwise, GDB will
not see this breakpoint hit when stepping onto breakpoints. */
> The second part of the change is more tricky:
> + stop_pc -= DECR_PC_AFTER_BREAK;
> Is it fixing any failures? Software singlestep can be handled in two
> different ways:
> - as a breakpoint
> - as a hardware single step
> and which is prefered decides if/when there should be a decrement.
Yes, this one is actually fixing most of the failures.
I made several attempts at fixing the regressions before coming with
this solution. This part of the code is quite tricky, and it seems to me
that treating single-step breakpoints as hardware single step is the
simplest way to handle them. I like this because the differences in
processing between software and hardware single-step become smaller.
See for instance the change in breakpoint.c which made the use of this
macro disappear from this file.
> Anyway, the thing I'm having trouble convincing myself that there can't
> be a double decrement -- eg for a hardware watchpoint or similar.
I've tried as much as I can to make sure this can not happen, but I am
not familiar enough to have a good level of confidence in my analysis.
All I can say is: this patch fixes all the regressions observed in the
testsuite after switching to software single step. I know this is no
absolute proof, but that gives me a certain level of confidence.
--
Joel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-08-05 11:49 ` Joel Brobecker
@ 2002-08-05 20:01 ` Andrew Cagney
2002-08-16 10:11 ` Andrew Cagney
1 sibling, 0 replies; 24+ messages in thread
From: Andrew Cagney @ 2002-08-05 20:01 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
>> Can you confirm that the code is encountering a situtation where both
>> breakpoints_inserted and singlestep_breakpoints_inserted_p are true. I
>> think this occures when doing a single step after stepping off of a
>> breakpoint. When single stepping off a breakpoint, only
>> singlestep_breakpoints_inserted_p would be true.
>>
>> If this is the case then the comments should make mention of it. It
>> also makes the re-ordered if statement part of the patch correct.
>
>
> Yes, I can confirm this, and this should happen fairly often: suppose
> you have inserted a regular breakpoint in your program, anywhere, and
> then do a single step. Before resuming the inferior, GDB will re-insert
> the breakpoints, and set breakpoints_inserted. At the same time, because
> we are doing a s/w single step, singlestep_breakpoints_inserted_p will
> be set too. Did I miss something?
Thanks. No you didn't miss anything, I just want to be sure WFI is nasty.
The problem won't have been noticed previously as only the older targets
use software single-step and I don't know how often their testsuite is
beaten on.
> As for the re-ordering, I made it because I saw some regressions in the
> testsuite after switching to s/w single step. Unfortunately, I don't
> remember which ones, I would have to rerun the testsuite without this
> change to find them again. But the following comment explains in which
> cases the re-ordering was necessary:
Yes, ok, the part of the patch that re-orders the test is ok.
----
> I've tried as much as I can to make sure this can not happen, but I am
> not familiar enough to have a good level of confidence in my analysis.
> All I can say is: this patch fixes all the regressions observed in the
> testsuite after switching to software single step. I know this is no
> absolute proof, but that gives me a certain level of confidence.
BTW, if someone ever claims to have a ``good level of confidence'' in
that code, assume that they are lieing :-^
I'm still thinking about this bit, trying to find a way of not so much
increasing our confidence but at least putting us in a position where we
are more sure about what to do when we next encounter a problem.
Andrew
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-08-05 11:49 ` Joel Brobecker
2002-08-05 20:01 ` Andrew Cagney
@ 2002-08-16 10:11 ` Andrew Cagney
2002-08-16 11:21 ` Joel Brobecker
1 sibling, 1 reply; 24+ messages in thread
From: Andrew Cagney @ 2002-08-16 10:11 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
Joel,
Did the re-ordering of that if statement get committed?
This is wfi code so the smaller each change is the better.
>> The second part of the change is more tricky:
>> + stop_pc -= DECR_PC_AFTER_BREAK;
>> Is it fixing any failures? Software singlestep can be handled in two
>> different ways:
>> - as a breakpoint
>> - as a hardware single step
>> and which is prefered decides if/when there should be a decrement.
>
>
> Yes, this one is actually fixing most of the failures.
>
> I made several attempts at fixing the regressions before coming with
> this solution. This part of the code is quite tricky, and it seems to me
> that treating single-step breakpoints as hardware single step is the
> simplest way to handle them. I like this because the differences in
> processing between software and hardware single-step become smaller.
> See for instance the change in breakpoint.c which made the use of this
> macro disappear from this file.
>
>
>> Anyway, the thing I'm having trouble convincing myself that there can't
>> be a double decrement -- eg for a hardware watchpoint or similar.
>
>
> I've tried as much as I can to make sure this can not happen, but I am
> not familiar enough to have a good level of confidence in my analysis.
> All I can say is: this patch fixes all the regressions observed in the
> testsuite after switching to software single step. I know this is no
> absolute proof, but that gives me a certain level of confidence.
> + /* Readjust the stop_pc as it is off by DECR_PC_AFTER_BREAK
> + compared to the value it would have if the system stepping
> + capability was used. This allows the rest of the code in
> + this function to use this address without having to worry
> + whether software single step is in use or not. */
> + stop_pc -= DECR_PC_AFTER_BREAK;
> + ecs->random_signal = 0;
Ok, I'm convinced that this is going in the right direction (and was
most likely the original intent of the code :-). The more we can
localize DECR_PC_AFTER_BREAK (ie. extract it from bpstat_stop_status())
the better! :-)
I this will be ok with some tweaks and a litte bit more experimentation.
I _think_ the above should be also writing the decremented stop_pc back
to the target. This is what appears to happen else where. It also
ensures that code that calls read_pc() instead of using stop_pc, gets a
consistent value. However, there could be some sort of dragon lurking
here. Can you please check this out? The other possability is that
something is detecting that the PC wasn't written (status register) and
hence, assuming it should be decremented? It might even be written by
resume().
I think, the above code should also set a flag to indicate a ``software
single step trap'' occured. That indication can then be passed down to
bpstat_stop_status() to so that it knows that this software single-step
trap is not a breakpoint and hence that the decr_pc_after_break doesn't
apply. Off hand, a ``software_singlestep_p'' variable (||ed into
not_a_sw_breakpoint) or (better?) an extra enum trap_type (unknown_trap,
software_singlestep_trap) parameter explictly specifying the type of
trap that was identifed.
> + }
For the below, note that I've changed the variable name to
``not_a_sw_breakpoint'' so you'll get a clash when merging.
> ! bp_addr = *pc - (not_a_breakpoint && !SOFTWARE_SINGLE_STEP_P () ?
> ! 0 : DECR_PC_AFTER_BREAK);
>
> ALL_BREAKPOINTS_SAFE (b, temp)
> {
> --- 2429,2435 ----
> trap event. For a trace/singlestep trap event, we would
> not want to subtract DECR_PC_AFTER_BREAK from the PC. */
>
> ! bp_addr = *pc - (not_a_breakpoint ? 0 : DECR_PC_AFTER_BREAK);
>
> ALL_BREAKPOINTS_SAFE (b, temp)
> {
Andrew
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-08-16 10:11 ` Andrew Cagney
@ 2002-08-16 11:21 ` Joel Brobecker
2002-08-16 12:11 ` Andrew Cagney
2002-08-16 16:05 ` Joel Brobecker
0 siblings, 2 replies; 24+ messages in thread
From: Joel Brobecker @ 2002-08-16 11:21 UTC (permalink / raw)
To: Andrew Cagney; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 3370 bytes --]
> Did the re-ordering of that if statement get committed?
> This is wfi code so the smaller each change is the better.
[muttering something about too many things to do at the same time]
I tested (on Linux) and committed the attached patch. This is almost
the entire infrun.c change, except that I put the following change
on hold:
> >>The second part of the change is more tricky:
> >> + stop_pc -= DECR_PC_AFTER_BREAK;
Concerning this particular change:
> >+ /* Readjust the stop_pc as it is off by DECR_PC_AFTER_BREAK
> >+ compared to the value it would have if the system stepping
> >+ capability was used. This allows the rest of the code in
> >+ this function to use this address without having to worry
> >+ whether software single step is in use or not. */
> >+ stop_pc -= DECR_PC_AFTER_BREAK;
> >+ ecs->random_signal = 0;
>
> Ok, I'm convinced that this is going in the right direction (and was
> most likely the original intent of the code :-). The more we can
> localize DECR_PC_AFTER_BREAK (ie. extract it from bpstat_stop_status())
> the better! :-)
>
> I this will be ok with some tweaks and a litte bit more experimentation.
Cool.
> I _think_ the above should be also writing the decremented stop_pc back
> to the target. This is what appears to happen else where. It also
> ensures that code that calls read_pc() instead of using stop_pc, gets a
> consistent value. However, there could be some sort of dragon lurking
> here. Can you please check this out? The other possability is that
> something is detecting that the PC wasn't written (status register) and
> hence, assuming it should be decremented? It might even be written by
> resume().
It makes sense. I should be able to verify this change sometime soon.
> I think, the above code should also set a flag to indicate a ``software
> single step trap'' occured. That indication can then be passed down to
> bpstat_stop_status() to so that it knows that this software single-step
> trap is not a breakpoint and hence that the decr_pc_after_break doesn't
> apply. Off hand, a ``software_singlestep_p'' variable (||ed into
> not_a_sw_breakpoint) or (better?) an extra enum trap_type (unknown_trap,
> software_singlestep_trap) parameter explictly specifying the type of
> trap that was identifed.
This needs to be investigated. So far, the not_a_sw_breakpoint seems
to be sufficient, but adding this flag would definitely make things
clearer, especially on the caller side.
May I suggest we treat this as a separate patch that would be applied
after all the issues in this RFA are dealt with?
> For the below, note that I've changed the variable name to
> ``not_a_sw_breakpoint'' so you'll get a clash when merging.
Thanks.
So, to summarize:
1 - The change "+ stop_pc -= DECR_PC_AFTER_BREAK;" seems
to be going in the right direction.
2 - However I should hold this change for now because you think I should
write the adjusted PC value back to the target, by adding something
like "write_pc_pid (stop_pc, ecs->ptid)"
I will verify the impact of such a change, and report.
3 - Assuming we get all issues in this RFA resolved, then I will start
looking at the addition of the software_singlestep flag.
--
Joel
[-- Attachment #2: infrun.c.diff --]
[-- Type: text/plain, Size: 1400 bytes --]
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.64
diff -c -r1.64 infrun.c
*** infrun.c 24 Jul 2002 14:38:55 -0000 1.64
--- infrun.c 16 Aug 2002 17:55:05 -0000
***************
*** 1826,1835 ****
if (stop_signal == TARGET_SIGNAL_TRAP)
{
! if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
! ecs->random_signal = 0;
! else if (breakpoints_inserted
! && breakpoint_here_p (stop_pc - DECR_PC_AFTER_BREAK))
{
ecs->random_signal = 0;
if (!breakpoint_thread_match (stop_pc - DECR_PC_AFTER_BREAK,
--- 1826,1836 ----
if (stop_signal == TARGET_SIGNAL_TRAP)
{
! /* Check if a regular breakpoint has been hit before checking
! for a potential single step breakpoint. Otherwise, GDB will
! not see this breakpoint hit when stepping onto breakpoints. */
! if (breakpoints_inserted
! && breakpoint_here_p (stop_pc - DECR_PC_AFTER_BREAK))
{
ecs->random_signal = 0;
if (!breakpoint_thread_match (stop_pc - DECR_PC_AFTER_BREAK,
***************
*** 1885,1890 ****
--- 1886,1895 ----
}
}
}
+ else if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+ {
+ ecs->random_signal = 0;
+ }
}
else
ecs->random_signal = 1;
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-08-16 11:21 ` Joel Brobecker
@ 2002-08-16 12:11 ` Andrew Cagney
2002-08-16 12:26 ` Daniel Jacobowitz
2002-08-16 16:05 ` Joel Brobecker
1 sibling, 1 reply; 24+ messages in thread
From: Andrew Cagney @ 2002-08-16 12:11 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
> So, to summarize:
>
> 1 - The change "+ stop_pc -= DECR_PC_AFTER_BREAK;" seems
> to be going in the right direction.
>
> 2 - However I should hold this change for now because you think I should
> write the adjusted PC value back to the target, by adding something
> like "write_pc_pid (stop_pc, ecs->ptid)"
>
> I will verify the impact of such a change, and report.
>
> 3 - Assuming we get all issues in this RFA resolved, then I will start
> looking at the addition of the software_singlestep flag.
I think the flag should be added as part of the change. That way we're
100% certain that bpstat_stop_status() isn't going to do a
decr_pc_after_break.
Expressions like:
/* Pass TRUE if our reason for stopping is something other
than hitting a breakpoint. We do this by checking that
1) stepping is going on and 2) we didn't hit a breakpoint
in a signal handler without an intervening stop in
sigtramp, which is detected by a new stack pointer value
below any usual function calling stack adjustments. */
(currently_stepping (ecs)
&& prev_pc != stop_pc - DECR_PC_AFTER_BREAK
&& !(step_range_end && INNER_THAN (read_sp (),(step_sp - 16)))));
would be changed to read:
(trap_was_a_software_singlestep
|| ....)
Hmm, looking at the above, on an architecture like the i386, the test
``prev_pc != stop_pc - DECR_PC_AFTER_BREAK'' is probably false when if
the code has just stepped off a single byte instruction :-(
enjoy,
Andrew
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-08-16 12:11 ` Andrew Cagney
@ 2002-08-16 12:26 ` Daniel Jacobowitz
2002-08-16 12:40 ` Kevin Buettner
2002-08-16 12:41 ` Andrew Cagney
0 siblings, 2 replies; 24+ messages in thread
From: Daniel Jacobowitz @ 2002-08-16 12:26 UTC (permalink / raw)
To: Andrew Cagney; +Cc: Joel Brobecker, gdb-patches
On Fri, Aug 16, 2002 at 03:11:02PM -0400, Andrew Cagney wrote:
>
> >So, to summarize:
> >
> > 1 - The change "+ stop_pc -= DECR_PC_AFTER_BREAK;" seems
> > to be going in the right direction.
> >
> > 2 - However I should hold this change for now because you think I should
> > write the adjusted PC value back to the target, by adding something
> > like "write_pc_pid (stop_pc, ecs->ptid)"
> >
> > I will verify the impact of such a change, and report.
> >
> > 3 - Assuming we get all issues in this RFA resolved, then I will start
> > looking at the addition of the software_singlestep flag.
>
> I think the flag should be added as part of the change. That way we're
> 100% certain that bpstat_stop_status() isn't going to do a
> decr_pc_after_break.
>
> Expressions like:
>
> /* Pass TRUE if our reason for stopping is something other
> than hitting a breakpoint. We do this by checking that
> 1) stepping is going on and 2) we didn't hit a breakpoint
> in a signal handler without an intervening stop in
> sigtramp, which is detected by a new stack pointer value
> below any usual function calling stack adjustments. */
> (currently_stepping (ecs)
> && prev_pc != stop_pc - DECR_PC_AFTER_BREAK
> && !(step_range_end && INNER_THAN (read_sp (),(step_sp - 16)))));
Which reminds me - does that use of INNER_THAN make even the slightest
sense on stack-grows-up architectures? I don't think it does.
> would be changed to read:
>
> (trap_was_a_software_singlestep
> || ....)
>
> Hmm, looking at the above, on an architecture like the i386, the test
> ``prev_pc != stop_pc - DECR_PC_AFTER_BREAK'' is probably false when if
> the code has just stepped off a single byte instruction :-(
Is there some way we can do this without growing that condition? It's
awful, and it makes very little sense; it feels like something that
should already have been handled.
Yes, I'm trying to trick Joel into doing some of the
DECR_PC_AFTER_BREAK cleanups that everyone keeps meaning to do. It's
just not being handled somewhere useful right now.
--
Daniel Jacobowitz
MontaVista Software Debian GNU/Linux Developer
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-08-16 12:26 ` Daniel Jacobowitz
@ 2002-08-16 12:40 ` Kevin Buettner
2002-08-16 14:40 ` Peter.Schauer
2002-08-16 12:41 ` Andrew Cagney
1 sibling, 1 reply; 24+ messages in thread
From: Kevin Buettner @ 2002-08-16 12:40 UTC (permalink / raw)
To: Daniel Jacobowitz, Andrew Cagney; +Cc: Joel Brobecker, gdb-patches
On Aug 16, 3:26pm, Daniel Jacobowitz wrote:
> > Expressions like:
> >
> > /* Pass TRUE if our reason for stopping is something other
> > than hitting a breakpoint. We do this by checking that
> > 1) stepping is going on and 2) we didn't hit a breakpoint
> > in a signal handler without an intervening stop in
> > sigtramp, which is detected by a new stack pointer value
> > below any usual function calling stack adjustments. */
> > (currently_stepping (ecs)
> > && prev_pc != stop_pc - DECR_PC_AFTER_BREAK
> > && !(step_range_end && INNER_THAN (read_sp (),(step_sp - 16)))));
>
> Which reminds me - does that use of INNER_THAN make even the slightest
> sense on stack-grows-up architectures? I don't think it does.
I don't think so either.
Also, that magical value of 16 just can't be right for all architectures.
But this is one of those areas where we have to be *very* careful.
We can attempt to make well-meaning changes and then discover many
months later that we've broken something that used work.
For the above, I think we need to figure out the intent behind the
condition
INNER_THAN (read_sp (),(step_sp - 16),
and then write an architecture dependent method for it.
Kevin
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-08-16 12:26 ` Daniel Jacobowitz
2002-08-16 12:40 ` Kevin Buettner
@ 2002-08-16 12:41 ` Andrew Cagney
1 sibling, 0 replies; 24+ messages in thread
From: Andrew Cagney @ 2002-08-16 12:41 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: Joel Brobecker, gdb-patches
> Which reminds me - does that use of INNER_THAN make even the slightest
> sense on stack-grows-up architectures? I don't think it does.
Even the 16 makes no sense! If you pull the code though, solaris (from
memory) gets really bad test results ....
>> would be changed to read:
>>
>> (trap_was_a_software_singlestep
>> || ....)
>>
>> Hmm, looking at the above, on an architecture like the i386, the test
>> ``prev_pc != stop_pc - DECR_PC_AFTER_BREAK'' is probably false when if
>> the code has just stepped off a single byte instruction :-(
>
>
> Is there some way we can do this without growing that condition? It's
> awful, and it makes very little sense; it feels like something that
> should already have been handled.
It will be an ``||'' and not an &&. Hence, while making it longer, it
is at least simple to read and makes reasonable sense (unlike the other
bits). My other suggestion was to add a new parameter ``trap_type'' but
that I think can wait for later.
But yes, eventually zapping that expression would be a good idea.
> Yes, I'm trying to trick Joel into doing some of the
> DECR_PC_AFTER_BREAK cleanups that everyone keeps meaning to do. It's
> just not being handled somewhere useful right now.
Shh, ....
Andrew
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-08-16 12:40 ` Kevin Buettner
@ 2002-08-16 14:40 ` Peter.Schauer
0 siblings, 0 replies; 24+ messages in thread
From: Peter.Schauer @ 2002-08-16 14:40 UTC (permalink / raw)
To: Kevin Buettner; +Cc: drow, ac131313, brobecker, gdb-patches
Sigh.
http://sources.redhat.com/ml/gdb-patches/2001-06/msg00383.html
http://sources.redhat.com/ml/gdb-patches/2001-06/msg00512.html
http://sources.redhat.com/ml/gdb-patches/2001-07/msg00001.html
will help figuring out the intent.
> On Aug 16, 3:26pm, Daniel Jacobowitz wrote:
>
> > > Expressions like:
> > >
> > > /* Pass TRUE if our reason for stopping is something other
> > > than hitting a breakpoint. We do this by checking that
> > > 1) stepping is going on and 2) we didn't hit a breakpoint
> > > in a signal handler without an intervening stop in
> > > sigtramp, which is detected by a new stack pointer value
> > > below any usual function calling stack adjustments. */
> > > (currently_stepping (ecs)
> > > && prev_pc != stop_pc - DECR_PC_AFTER_BREAK
> > > && !(step_range_end && INNER_THAN (read_sp (),(step_sp - 16)))));
> >
> > Which reminds me - does that use of INNER_THAN make even the slightest
> > sense on stack-grows-up architectures? I don't think it does.
>
> I don't think so either.
>
> Also, that magical value of 16 just can't be right for all architectures.
>
> But this is one of those areas where we have to be *very* careful.
> We can attempt to make well-meaning changes and then discover many
> months later that we've broken something that used work.
>
> For the above, I think we need to figure out the intent behind the
> condition
>
> INNER_THAN (read_sp (),(step_sp - 16),
>
> and then write an architecture dependent method for it.
>
> Kevin
>
>
--
Peter Schauer pes@regent.e-technik.tu-muenchen.de
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-08-16 11:21 ` Joel Brobecker
2002-08-16 12:11 ` Andrew Cagney
@ 2002-08-16 16:05 ` Joel Brobecker
2002-08-16 16:45 ` Andrew Cagney
1 sibling, 1 reply; 24+ messages in thread
From: Joel Brobecker @ 2002-08-16 16:05 UTC (permalink / raw)
To: Andrew Cagney; +Cc: gdb-patches
> 2 - However I should hold this change for now because you think I should
> write the adjusted PC value back to the target, by adding something
> like "write_pc_pid (stop_pc, ecs->ptid)"
>
> I will verify the impact of such a change, and report.
Good news :). Adding this does not introduce any regression, so no red
dragon lurking so far...
> 3 - Assuming we get all issues in this RFA resolved, then I will start
> looking at the addition of the software_singlestep flag.
I am starting to work on this one.
I suggest for this RFA to:
- define a new boolean software_singlestep_p
- set it at the same time we adjust the PC when detecting sw single
step.
- Change the value of not_a_sw_breakpoint in the call to
bpstat_stop_status() to prepend a "sw_single_step_p ||".
As a next patch, we would add a new sw_single_step_p parameter to
bpstat_stop_status() to pass this value separately.
--
Joel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-08-16 16:05 ` Joel Brobecker
@ 2002-08-16 16:45 ` Andrew Cagney
2002-08-16 17:58 ` Joel Brobecker
0 siblings, 1 reply; 24+ messages in thread
From: Andrew Cagney @ 2002-08-16 16:45 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
>> 2 - However I should hold this change for now because you think I should
>> write the adjusted PC value back to the target, by adding something
>> like "write_pc_pid (stop_pc, ecs->ptid)"
>>
>> I will verify the impact of such a change, and report.
>
>
> Good news :). Adding this does not introduce any regression, so no red
> dragon lurking so far...
Ya!
>> 3 - Assuming we get all issues in this RFA resolved, then I will start
>> looking at the addition of the software_singlestep flag.
>
>
> I am starting to work on this one.
>
> I suggest for this RFA to:
> - define a new boolean software_singlestep_p
> - set it at the same time we adjust the PC when detecting sw single
> step.
> - Change the value of not_a_sw_breakpoint in the call to
> bpstat_stop_status() to prepend a "sw_single_step_p ||".
Yep,
> As a next patch, we would add a new sw_single_step_p parameter to
> bpstat_stop_status() to pass this value separately.
Somebody elses problem (hmm, perhaps we could trick DanielJ into doing
it ;-)
Andrew
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-08-16 16:45 ` Andrew Cagney
@ 2002-08-16 17:58 ` Joel Brobecker
2002-08-16 18:23 ` Andrew Cagney
0 siblings, 1 reply; 24+ messages in thread
From: Joel Brobecker @ 2002-08-16 17:58 UTC (permalink / raw)
To: Andrew Cagney; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1368 bytes --]
Following Andrew's suggestion, here is a revised patch to provide
software single stepping on alpha-osf1, plus fix all the quirks that
appeared after the switch.
I know the alpha-osf1-tdep.c change is approved, but since I haven't
committed it yet, I am including it in this patch for completeness.
No regression on alpha-osf1 (SW single step enabled) and on x86-linux
(sw single step disabled):
2002-07-18 Joel Brobecker <brobecker@gnat.com>
* alpha-osf1-tdep.c (alpha_osf1_init_abi): Unfortunately,
procfs appears to be broken when debugging on multi-processor
machines. So enable software single stepping in order to avoid
using the procfs interface to do next/step operations, using
internal breakpoints instead.
* infrun.c (handle_inferior_event): Readjust the stop_pc by
DECR_PC_AFTER_BREAK when hitting a single step breakpoint, to
make this pc address equal to the value it would have if the
system stepping capability was used. Also set a new flag used
to ensure that we don't readjust the PC one more time later.
* breakpoint.c (bpstat_stop_status): Do not adjust the PC
address by DECR_PC_AFTER_BREAK when software single step is
in use for this architecture, as this has already been taken
care of in handle_inferior_event().
--
Joel
[-- Attachment #2: sw_single_step.diff --]
[-- Type: text/plain, Size: 4935 bytes --]
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.66
diff -c -3 -p -r1.66 infrun.c
*** infrun.c 17 Aug 2002 00:16:54 -0000 1.66
--- infrun.c 17 Aug 2002 00:49:19 -0000
*************** handle_inferior_event (struct execution_
*** 1408,1413 ****
--- 1408,1414 ----
{
CORE_ADDR tmp;
int stepped_after_stopped_by_watchpoint;
+ int sw_single_step_trap_p = 0;
/* Cache the last pid/waitstatus. */
target_last_wait_ptid = ecs->ptid;
*************** handle_inferior_event (struct execution_
*** 1888,1893 ****
--- 1889,1906 ----
}
else if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
{
+ /* Readjust the stop_pc as it is off by DECR_PC_AFTER_BREAK
+ compared to the value it would have if the system stepping
+ capability was used. This allows the rest of the code in
+ this function to use this address without having to worry
+ whether software single step is in use or not. */
+ if (DECR_PC_AFTER_BREAK)
+ {
+ stop_pc -= DECR_PC_AFTER_BREAK;
+ write_pc_pid (stop_pc, ecs->ptid);
+ }
+
+ sw_single_step_trap_p = 1;
ecs->random_signal = 0;
}
}
*************** handle_inferior_event (struct execution_
*** 2111,2124 ****
(&stop_pc,
/* Pass TRUE if our reason for stopping is something other
than hitting a breakpoint. We do this by checking that
1) stepping is going on and 2) we didn't hit a breakpoint
in a signal handler without an intervening stop in
sigtramp, which is detected by a new stack pointer value
below any usual function calling stack adjustments. */
! (currently_stepping (ecs)
! && prev_pc != stop_pc - DECR_PC_AFTER_BREAK
! && !(step_range_end
! && INNER_THAN (read_sp (), (step_sp - 16)))));
/* Following in case break condition called a
function. */
stop_print_frame = 1;
--- 2124,2139 ----
(&stop_pc,
/* Pass TRUE if our reason for stopping is something other
than hitting a breakpoint. We do this by checking that
+ either we detected earlier a software single step trap or
1) stepping is going on and 2) we didn't hit a breakpoint
in a signal handler without an intervening stop in
sigtramp, which is detected by a new stack pointer value
below any usual function calling stack adjustments. */
! sw_single_step_trap_p
! || (currently_stepping (ecs)
! && prev_pc != stop_pc - DECR_PC_AFTER_BREAK
! && !(step_range_end
! && INNER_THAN (read_sp (), (step_sp - 16)))));
/* Following in case break condition called a
function. */
stop_print_frame = 1;
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.79
diff -c -3 -p -r1.79 breakpoint.c
*** breakpoint.c 16 Aug 2002 15:37:54 -0000 1.79
--- breakpoint.c 17 Aug 2002 00:49:20 -0000
*************** bpstat_stop_status (CORE_ADDR *pc, int n
*** 2429,2436 ****
trace/singlestep trap event, we would not want to subtract
DECR_PC_AFTER_BREAK from the PC. */
! bp_addr = *pc - (not_a_sw_breakpoint && !SOFTWARE_SINGLE_STEP_P () ?
! 0 : DECR_PC_AFTER_BREAK);
ALL_BREAKPOINTS_SAFE (b, temp)
{
--- 2429,2435 ----
trace/singlestep trap event, we would not want to subtract
DECR_PC_AFTER_BREAK from the PC. */
! bp_addr = *pc - (not_a_sw_breakpoint ? 0 : DECR_PC_AFTER_BREAK);
ALL_BREAKPOINTS_SAFE (b, temp)
{
Index: alpha-osf1-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/alpha-osf1-tdep.c,v
retrieving revision 1.5
diff -c -3 -p -r1.5 alpha-osf1-tdep.c
*** alpha-osf1-tdep.c 21 May 2002 15:36:02 -0000 1.5
--- alpha-osf1-tdep.c 17 Aug 2002 00:49:20 -0000
*************** alpha_osf1_init_abi (struct gdbarch_info
*** 58,63 ****
--- 58,67 ----
struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
set_gdbarch_pc_in_sigtramp (gdbarch, alpha_osf1_pc_in_sigtramp);
+ /* The next/step support via procfs on OSF1 is broken when running
+ on multi-processor machines. We need to use software single stepping
+ instead. */
+ set_gdbarch_software_single_step (gdbarch, alpha_software_single_step);
tdep->skip_sigtramp_frame = alpha_osf1_skip_sigtramp_frame;
tdep->sigcontext_addr = alpha_osf1_sigcontext_addr;
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-08-16 17:58 ` Joel Brobecker
@ 2002-08-16 18:23 ` Andrew Cagney
2002-08-16 23:29 ` Joel Brobecker
0 siblings, 1 reply; 24+ messages in thread
From: Andrew Cagney @ 2002-08-16 18:23 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
> Following Andrew's suggestion, here is a revised patch to provide
> software single stepping on alpha-osf1, plus fix all the quirks that
> appeared after the switch.
>
> I know the alpha-osf1-tdep.c change is approved, but since I haven't
> committed it yet, I am including it in this patch for completeness.
>
> No regression on alpha-osf1 (SW single step enabled) and on x86-linux
> (sw single step disabled):
Yep, testing x68 linux is a good idea.
> 2002-07-18 Joel Brobecker <brobecker@gnat.com>
>
> * alpha-osf1-tdep.c (alpha_osf1_init_abi): Unfortunately,
> procfs appears to be broken when debugging on multi-processor
> machines. So enable software single stepping in order to avoid
> using the procfs interface to do next/step operations, using
> internal breakpoints instead.
>
> * infrun.c (handle_inferior_event): Readjust the stop_pc by
> DECR_PC_AFTER_BREAK when hitting a single step breakpoint, to
> make this pc address equal to the value it would have if the
> system stepping capability was used. Also set a new flag used
> to ensure that we don't readjust the PC one more time later.
>
> * breakpoint.c (bpstat_stop_status): Do not adjust the PC
> address by DECR_PC_AFTER_BREAK when software single step is
> in use for this architecture, as this has already been taken
> care of in handle_inferior_event().
>
Yes and thanks. Finally something in wfi that I understand!
Andrew
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-08-16 18:23 ` Andrew Cagney
@ 2002-08-16 23:29 ` Joel Brobecker
2002-08-20 8:55 ` Joel Brobecker
0 siblings, 1 reply; 24+ messages in thread
From: Joel Brobecker @ 2002-08-16 23:29 UTC (permalink / raw)
To: Andrew Cagney; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1537 bytes --]
> >2002-07-18 Joel Brobecker <brobecker@gnat.com>
> >
> > * alpha-osf1-tdep.c (alpha_osf1_init_abi): Unfortunately,
> > procfs appears to be broken when debugging on multi-processor
> > machines. So enable software single stepping in order to avoid
> > using the procfs interface to do next/step operations, using
> > internal breakpoints instead.
> >
> > * infrun.c (handle_inferior_event): Readjust the stop_pc by
> > DECR_PC_AFTER_BREAK when hitting a single step breakpoint, to
> > make this pc address equal to the value it would have if the
> > system stepping capability was used. Also set a new flag used
> > to ensure that we don't readjust the PC one more time later.
> >
> > * breakpoint.c (bpstat_stop_status): Do not adjust the PC
> > address by DECR_PC_AFTER_BREAK when software single step is
> > in use for this architecture, as this has already been taken
> > care of in handle_inferior_event().
> >
>
> Yes and thanks. Finally something in wfi that I understand!
Me too. 10 lines understood, 1500 more to go :). Thanks for the careful
review.
This patch has been checked-in. Here is a patch to update the NEWS file,
as suggested by Eli. I'm not very good at wording, so any suggestion for
improvement is more than welcome.
2002-08-17 Joel Brobecker <brobecker@gnat.com>
* NEWS: Add an entry regarding the improvement of the next/step
operation on Alpha Tru64 multi-processor machines.
--
Joel
[-- Attachment #2: NEWS.diff --]
[-- Type: text/plain, Size: 672 bytes --]
Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.86
diff -c -3 -r1.86 NEWS
*** NEWS 15 Aug 2002 22:51:40 -0000 1.86
--- NEWS 17 Aug 2002 06:22:04 -0000
***************
*** 91,96 ****
--- 91,102 ----
to a bfd-format or binary file (dump and append), and back
from a file into memory (restore).
+ * Improved "next/step" support on multi-processor Alpha Tru64.
+
+ On multi-processor Alpha Tru64, next/step operations sometimes used to
+ cause problems, such as the random apperance of SIGSEGV or SIGTRAP
+ signals for instance.
+
*** Changes in GDB 5.2.1:
* New targets.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-08-16 23:29 ` Joel Brobecker
@ 2002-08-20 8:55 ` Joel Brobecker
2002-08-20 17:29 ` Andrew Cagney
0 siblings, 1 reply; 24+ messages in thread
From: Joel Brobecker @ 2002-08-20 8:55 UTC (permalink / raw)
To: gdb-patches
> Here is a patch to update the NEWS file,
> as suggested by Eli. I'm not very good at wording, so any suggestion for
> improvement is more than welcome.
>
> 2002-08-17 Joel Brobecker <brobecker@gnat.com>
>
> * NEWS: Add an entry regarding the improvement of the next/step
> operation on Alpha Tru64 multi-processor machines.
I am just realizing that I was probably not very clear... I am asking
for approval before I check this change in, in case the wording, or
maybe the way I organized this item in the NEWS file can be improved.
> Index: NEWS
> ===================================================================
> RCS file: /cvs/src/src/gdb/NEWS,v
> retrieving revision 1.86
> diff -c -3 -r1.86 NEWS
> *** NEWS 15 Aug 2002 22:51:40 -0000 1.86
> --- NEWS 17 Aug 2002 06:22:04 -0000
> ***************
> *** 91,96 ****
> --- 91,102 ----
> to a bfd-format or binary file (dump and append), and back
> from a file into memory (restore).
>
> + * Improved "next/step" support on multi-processor Alpha Tru64.
> +
> + On multi-processor Alpha Tru64, next/step operations sometimes used to
> + cause problems, such as the random apperance of SIGSEGV or SIGTRAP
> + signals for instance.
> +
> *** Changes in GDB 5.2.1:
>
> * New targets.
--
Joel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-08-20 8:55 ` Joel Brobecker
@ 2002-08-20 17:29 ` Andrew Cagney
2002-08-20 19:14 ` Daniel Jacobowitz
0 siblings, 1 reply; 24+ messages in thread
From: Andrew Cagney @ 2002-08-20 17:29 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
>> Here is a patch to update the NEWS file,
>> as suggested by Eli. I'm not very good at wording, so any suggestion for
>> improvement is more than welcome.
>>
>> 2002-08-17 Joel Brobecker <brobecker@gnat.com>
>>
>> * NEWS: Add an entry regarding the improvement of the next/step
>> operation on Alpha Tru64 multi-processor machines.
>
>
> I am just realizing that I was probably not very clear... I am asking
> for approval before I check this change in, in case the wording, or
> maybe the way I organized this item in the NEWS file can be improved.
No one maintains the NEWS file. If you get no comments after a few
days, it's likely probably ok.
Andrew
>> Index: NEWS
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/NEWS,v
>> retrieving revision 1.86
>> diff -c -3 -r1.86 NEWS
>> *** NEWS 15 Aug 2002 22:51:40 -0000 1.86
>> --- NEWS 17 Aug 2002 06:22:04 -0000
>> ***************
>> *** 91,96 ****
>> --- 91,102 ----
>> to a bfd-format or binary file (dump and append), and back
>> from a file into memory (restore).
>>
>> + * Improved "next/step" support on multi-processor Alpha Tru64.
>> +
>> + On multi-processor Alpha Tru64, next/step operations sometimes used to
>> + cause problems, such as the random apperance of SIGSEGV or SIGTRAP
>> + signals for instance.
>> +
>> *** Changes in GDB 5.2.1:
>>
>> * New targets.
>
>
> -- Joel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-08-20 17:29 ` Andrew Cagney
@ 2002-08-20 19:14 ` Daniel Jacobowitz
2002-08-21 7:01 ` Joel Brobecker
0 siblings, 1 reply; 24+ messages in thread
From: Daniel Jacobowitz @ 2002-08-20 19:14 UTC (permalink / raw)
To: Andrew Cagney; +Cc: Joel Brobecker, gdb-patches
On Tue, Aug 20, 2002 at 08:29:18PM -0400, Andrew Cagney wrote:
> >>Here is a patch to update the NEWS file,
> >>as suggested by Eli. I'm not very good at wording, so any suggestion for
> >>improvement is more than welcome.
> >>
> >>2002-08-17 Joel Brobecker <brobecker@gnat.com>
> >>
> >> * NEWS: Add an entry regarding the improvement of the next/step
> >> operation on Alpha Tru64 multi-processor machines.
> >
> >
> >I am just realizing that I was probably not very clear... I am asking
> >for approval before I check this change in, in case the wording, or
> >maybe the way I organized this item in the NEWS file can be improved.
>
> No one maintains the NEWS file. If you get no comments after a few
> days, it's likely probably ok.
>
> Andrew
>
>
> >>Index: NEWS
> >>===================================================================
> >>RCS file: /cvs/src/src/gdb/NEWS,v
> >>retrieving revision 1.86
> >>diff -c -3 -r1.86 NEWS
> >>*** NEWS 15 Aug 2002 22:51:40 -0000 1.86
> >>--- NEWS 17 Aug 2002 06:22:04 -0000
> >>***************
> >>*** 91,96 ****
> >>--- 91,102 ----
> >> to a bfd-format or binary file (dump and append), and back
> >> from a file into memory (restore).
> >>
> >>+ * Improved "next/step" support on multi-processor Alpha Tru64.
> >>+
> >>+ On multi-processor Alpha Tru64, next/step operations sometimes used to
> >>+ cause problems, such as the random apperance of SIGSEGV or SIGTRAP
> >>+ signals for instance.
> >>+
If I may suggest...
* Improved "next/step" support on multi-processor Alpha Tru64.
The previous single-step mechanism could cause unpredictable problems,
including the random appearance of SIGSEGV or SIGTRAP signals. The use
of a software single-step mechanism prevents this.
--
Daniel Jacobowitz
MontaVista Software Debian GNU/Linux Developer
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFA] enable software single step on alpha-osf
2002-08-20 19:14 ` Daniel Jacobowitz
@ 2002-08-21 7:01 ` Joel Brobecker
0 siblings, 0 replies; 24+ messages in thread
From: Joel Brobecker @ 2002-08-21 7:01 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 122 bytes --]
> If I may suggest...
Most definitely, thanks for the suggestion. Attached is the patch that I
just committed.
--
Joel
[-- Attachment #2: NEWS.diff --]
[-- Type: text/plain, Size: 703 bytes --]
Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.88
diff -c -r1.88 NEWS
*** NEWS 21 Aug 2002 00:57:42 -0000 1.88
--- NEWS 21 Aug 2002 13:57:39 -0000
***************
*** 97,102 ****
--- 97,108 ----
to a bfd-format or binary file (dump and append), and back
from a file into memory (restore).
+ * Improved "next/step" support on multi-processor Alpha Tru64.
+
+ The previous single-step mechanism could cause unpredictable problems,
+ including the random appearance of SIGSEGV or SIGTRAP signals. The use
+ of a software single-step mechanism prevents this.
+
*** Changes in GDB 5.2.1:
* New targets.
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2002-08-21 14:01 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-07-18 13:55 [RFA] enable software single step on alpha-osf Joel Brobecker
2002-07-22 4:19 ` Eli Zaretskii
2002-07-25 16:38 ` Andrew Cagney
2002-07-26 10:17 ` Jason R Thorpe
2002-07-31 10:28 ` Joel Brobecker
2002-08-04 16:42 ` Andrew Cagney
2002-08-05 11:49 ` Joel Brobecker
2002-08-05 20:01 ` Andrew Cagney
2002-08-16 10:11 ` Andrew Cagney
2002-08-16 11:21 ` Joel Brobecker
2002-08-16 12:11 ` Andrew Cagney
2002-08-16 12:26 ` Daniel Jacobowitz
2002-08-16 12:40 ` Kevin Buettner
2002-08-16 14:40 ` Peter.Schauer
2002-08-16 12:41 ` Andrew Cagney
2002-08-16 16:05 ` Joel Brobecker
2002-08-16 16:45 ` Andrew Cagney
2002-08-16 17:58 ` Joel Brobecker
2002-08-16 18:23 ` Andrew Cagney
2002-08-16 23:29 ` Joel Brobecker
2002-08-20 8:55 ` Joel Brobecker
2002-08-20 17:29 ` Andrew Cagney
2002-08-20 19:14 ` Daniel Jacobowitz
2002-08-21 7:01 ` Joel Brobecker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox