This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
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.
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! :-)+ /* 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;
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
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |