diff mbox

[v3,6/6] Fix reverse-step and reverse-next over undebuggable solib code

Message ID 1407935535-27978-7-git-send-email-omair.javaid@linaro.org
State New
Headers show

Commit Message

Omair Javaid Aug. 13, 2014, 1:12 p.m. UTC 
This patch fixes failures to reverse-step or reverse-next over solib functions in absence of line information.
The problem is fixed by making sure that in solib functions we keep doing reverse single stepping in absence of line information.

Tested with no regressions on arm, aarch64 and x86_64.

gdb:

2014-08-13  Omair Javaid  <omair.javaid@linaro.org>

	* infrun.c (process_event_stop_test): Updated.

---
 gdb/infrun.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Omair Javaid Aug. 27, 2014, 9:08 a.m. UTC | #1 
On 13 August 2014 18:12, Omair Javaid <omair.javaid@linaro.org> wrote:
> This patch fixes failures to reverse-step or reverse-next over solib functions in absence of line information.
> The problem is fixed by making sure that in solib functions we keep doing reverse single stepping in absence of line information.
>
> Tested with no regressions on arm, aarch64 and x86_64.
>
> gdb:
>
> 2014-08-13  Omair Javaid  <omair.javaid@linaro.org>
>
>         * infrun.c (process_event_stop_test): Updated.
>
> ---
>  gdb/infrun.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index c18267f..db8f15b 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -4905,12 +4905,15 @@ process_event_stop_test (struct execution_control_state *ecs)
>        return;
>      }
>
> +  stop_pc_sal = find_pc_line (stop_pc, 0);
> +
>    /* Reverse stepping through solib trampolines.  */
>
>    if (execution_direction == EXEC_REVERSE
>        && ecs->event_thread->control.step_over_calls != STEP_OVER_NONE)
>      {
>        if (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
> +         || stop_pc_sal.line == 0
>           || (ecs->stop_func_start == 0
>               && in_solib_dynsym_resolve_code (stop_pc)))
>         {
> @@ -4939,8 +4942,6 @@ process_event_stop_test (struct execution_control_state *ecs)
>         }
>      }
>
> -  stop_pc_sal = find_pc_line (stop_pc, 0);
> -
>    /* NOTE: tausq/2004-05-24: This if block used to be done before all
>       the trampoline processing logic, however, there are some trampolines
>       that have no names, so we should do trampoline handling first.  */
> --
> 1.9.1
>

Ping! Kindly provide your feedback and help me approve this patch series.
Pedro Alves Aug. 27, 2014, 10:34 a.m. UTC | #2 
(This patch doesn't look specific to ARM, and thus could/should
be split out from the rest of the series.)

On 08/13/2014 02:12 PM, Omair Javaid wrote:
> This patch fixes failures to reverse-step or reverse-next over solib functions in absence of line information.

Could you describe the failure a little more please?  What happens instead?

> The problem is fixed by making sure that in solib functions we keep doing reverse single stepping in absence of line information.
> 
> Tested with no regressions on arm, aarch64 and x86_64.

Is there a test in the testsuite that fails before this patch?
If not, could you add one?

There's another similarly looking piece of code above
with the same comment: "Reverse stepping through solib trampolines
that I'm wondering whether it needs treatment as well.

Thanks!

> 
> gdb:
> 
> 2014-08-13  Omair Javaid  <omair.javaid@linaro.org>
> 
> 	* infrun.c (process_event_stop_test): Updated.

Same remark as previous patches here.

> 
> ---
>  gdb/infrun.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index c18267f..db8f15b 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -4905,12 +4905,15 @@ process_event_stop_test (struct execution_control_state *ecs)
>        return;
>      }
>  
> +  stop_pc_sal = find_pc_line (stop_pc, 0);
> +
>    /* Reverse stepping through solib trampolines.  */
>  
>    if (execution_direction == EXEC_REVERSE
>        && ecs->event_thread->control.step_over_calls != STEP_OVER_NONE)
>      {
>        if (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
> +	  || stop_pc_sal.line == 0
>  	  || (ecs->stop_func_start == 0
>  	      && in_solib_dynsym_resolve_code (stop_pc)))
>  	{
> @@ -4939,8 +4942,6 @@ process_event_stop_test (struct execution_control_state *ecs)
>  	}
>      }
>  
> -  stop_pc_sal = find_pc_line (stop_pc, 0);
> -
>    /* NOTE: tausq/2004-05-24: This if block used to be done before all
>       the trampoline processing logic, however, there are some trampolines 
>       that have no names, so we should do trampoline handling first.  */
> 

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/infrun.c b/gdb/infrun.c
index c18267f..db8f15b 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4905,12 +4905,15 @@  process_event_stop_test (struct execution_control_state *ecs)
       return;
     }
 
+  stop_pc_sal = find_pc_line (stop_pc, 0);
+
   /* Reverse stepping through solib trampolines.  */
 
   if (execution_direction == EXEC_REVERSE
       && ecs->event_thread->control.step_over_calls != STEP_OVER_NONE)
     {
       if (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
+	  || stop_pc_sal.line == 0
 	  || (ecs->stop_func_start == 0
 	      && in_solib_dynsym_resolve_code (stop_pc)))
 	{
@@ -4939,8 +4942,6 @@  process_event_stop_test (struct execution_control_state *ecs)
 	}
     }
 
-  stop_pc_sal = find_pc_line (stop_pc, 0);
-
   /* NOTE: tausq/2004-05-24: This if block used to be done before all
      the trampoline processing logic, however, there are some trampolines 
      that have no names, so we should do trampoline handling first.  */