[RFA] improved handling of shared lib calls (reverse 2/2)

[RFA] improved handling of shared lib calls (reverse 2/2)

[Michael Snyder]

[-- Attachment #1: Type: text/plain, Size: 397 bytes --]

This change depends on the previous one (epilogue unwinder for i386,
http://sourceware.org/ml/gdb-patches/2009-07/msg00099.html).

With the frame unwinder working properly, we can remove the
special case code in the test for "check for subroutine call"
in handle_inferior_event.

Of course that reveals some hidden problems with shared library
calls, so this patch also addresses those.

Michael


[-- Attachment #2: solib.txt --]
[-- Type: text/plain, Size: 2737 bytes --]

2009-07-03  Michael Snyder  <msnyder@vmware.com>

	* infrun.c (handle_inferior_event): Remove an execution_direction
	check in the "check for subroutine calls" test, and add some 
	additional handling for reverse-stepping through shared libraries.

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.400
diff -u -p -r1.400 infrun.c
--- infrun.c	2 Jul 2009 17:21:06 -0000	1.400
+++ infrun.c	4 Jul 2009 00:27:53 -0000
@@ -3776,9 +3776,8 @@ infrun: not switching back to stepped th
      previous frame must have valid frame IDs.  */
   if (!frame_id_eq (get_stack_frame_id (frame),
 		    ecs->event_thread->step_stack_frame_id)
-      && (frame_id_eq (frame_unwind_caller_id (frame),
-		       ecs->event_thread->step_stack_frame_id)
-	  || execution_direction == EXEC_REVERSE))
+      && frame_id_eq (frame_unwind_caller_id (frame),
+		      ecs->event_thread->step_stack_frame_id))
     {
       CORE_ADDR real_stop_pc;
 
@@ -3806,6 +3805,7 @@ infrun: not switching back to stepped th
       /* Reverse stepping through solib trampolines.  */
 
       if (execution_direction == EXEC_REVERSE
+	  && ecs->event_thread->step_over_calls != STEP_OVER_NONE
 	  && (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
 	      || (ecs->stop_func_start == 0
 		  && in_solib_dynsym_resolve_code (stop_pc))))
@@ -3923,6 +3923,38 @@ infrun: not switching back to stepped th
       return;
     }
 
+  /* Reverse stepping through solib trampolines.  */
+
+  if (execution_direction == EXEC_REVERSE
+      && ecs->event_thread->step_over_calls != STEP_OVER_NONE)
+    {
+      if (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
+	  || (ecs->stop_func_start == 0
+	      && in_solib_dynsym_resolve_code (stop_pc)))
+	{
+	  /* Any solib trampoline code can be handled in reverse
+	     by simply continuing to single-step.  We have already
+	     executed the solib function (backwards), and a few 
+	     steps will take us back through the trampoline to the
+	     caller.  */
+	  keep_going (ecs);
+	  return;
+	}
+      else if (in_solib_dynsym_resolve_code (stop_pc))
+	{
+	  /* Stepped backward into the solib dynsym resolver.
+	     Set a breakpoint at its start and continue, then
+	     one more step will take us out.  */
+	  struct symtab_and_line sr_sal;
+	  init_sal (&sr_sal);
+	  sr_sal.pc = ecs->stop_func_start;
+	  insert_step_resume_breakpoint_at_sal (gdbarch, 
+						sr_sal, null_frame_id);
+	  keep_going (ecs);
+	  return;
+	}
+    }
+
   /* If we're in the return path from a shared library trampoline,
      we want to proceed through the trampoline when stepping.  */
   if (gdbarch_in_solib_return_trampoline (gdbarch,

Re: [RFA] improved handling of shared lib calls (reverse 2/2)

[Hui Zhu]

I tried this patch with ubuntu i386.  Everything is OK.

Thanks,
Hui

On Sat, Jul 4, 2009 at 08:32, Michael Snyder<msnyder@vmware.com> wrote:
> This change depends on the previous one (epilogue unwinder for i386,
> http://sourceware.org/ml/gdb-patches/2009-07/msg00099.html).
>
> With the frame unwinder working properly, we can remove the
> special case code in the test for "check for subroutine call"
> in handle_inferior_event.
>
> Of course that reveals some hidden problems with shared library
> calls, so this patch also addresses those.
>
> Michael
>
>
> 2009-07-03  Michael Snyder  <msnyder@vmware.com>
>
>        * infrun.c (handle_inferior_event): Remove an execution_direction
>        check in the "check for subroutine calls" test, and add some
>        additional handling for reverse-stepping through shared libraries.
>
> Index: infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.400
> diff -u -p -r1.400 infrun.c
> --- infrun.c    2 Jul 2009 17:21:06 -0000       1.400
> +++ infrun.c    4 Jul 2009 00:27:53 -0000
> @@ -3776,9 +3776,8 @@ infrun: not switching back to stepped th
>      previous frame must have valid frame IDs.  */
>   if (!frame_id_eq (get_stack_frame_id (frame),
>                    ecs->event_thread->step_stack_frame_id)
> -      && (frame_id_eq (frame_unwind_caller_id (frame),
> -                      ecs->event_thread->step_stack_frame_id)
> -         || execution_direction == EXEC_REVERSE))
> +      && frame_id_eq (frame_unwind_caller_id (frame),
> +                     ecs->event_thread->step_stack_frame_id))
>     {
>       CORE_ADDR real_stop_pc;
>
> @@ -3806,6 +3805,7 @@ infrun: not switching back to stepped th
>       /* Reverse stepping through solib trampolines.  */
>
>       if (execution_direction == EXEC_REVERSE
> +         && ecs->event_thread->step_over_calls != STEP_OVER_NONE
>          && (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
>              || (ecs->stop_func_start == 0
>                  && in_solib_dynsym_resolve_code (stop_pc))))
> @@ -3923,6 +3923,38 @@ infrun: not switching back to stepped th
>       return;
>     }
>
> +  /* Reverse stepping through solib trampolines.  */
> +
> +  if (execution_direction == EXEC_REVERSE
> +      && ecs->event_thread->step_over_calls != STEP_OVER_NONE)
> +    {
> +      if (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
> +         || (ecs->stop_func_start == 0
> +             && in_solib_dynsym_resolve_code (stop_pc)))
> +       {
> +         /* Any solib trampoline code can be handled in reverse
> +            by simply continuing to single-step.  We have already
> +            executed the solib function (backwards), and a few
> +            steps will take us back through the trampoline to the
> +            caller.  */
> +         keep_going (ecs);
> +         return;
> +       }
> +      else if (in_solib_dynsym_resolve_code (stop_pc))
> +       {
> +         /* Stepped backward into the solib dynsym resolver.
> +            Set a breakpoint at its start and continue, then
> +            one more step will take us out.  */
> +         struct symtab_and_line sr_sal;
> +         init_sal (&sr_sal);
> +         sr_sal.pc = ecs->stop_func_start;
> +         insert_step_resume_breakpoint_at_sal (gdbarch,
> +                                               sr_sal, null_frame_id);
> +         keep_going (ecs);
> +         return;
> +       }
> +    }
> +
>   /* If we're in the return path from a shared library trampoline,
>      we want to proceed through the trampoline when stepping.  */
>   if (gdbarch_in_solib_return_trampoline (gdbarch,
>
>

Re: [RFA] improved handling of shared lib calls (reverse 2/2)

[Michael Snyder]

Now how about this one?

Michael Snyder wrote:
> This change depends on the previous one (epilogue unwinder for i386,
> http://sourceware.org/ml/gdb-patches/2009-07/msg00099.html).
> 
> With the frame unwinder working properly, we can remove the
> special case code in the test for "check for subroutine call"
> in handle_inferior_event.
> 
> Of course that reveals some hidden problems with shared library
> calls, so this patch also addresses those.
> 
> Michael
> 
> 
> 
> ------------------------------------------------------------------------
> 
> 2009-07-03  Michael Snyder  <msnyder@vmware.com>
> 
> 	* infrun.c (handle_inferior_event): Remove an execution_direction
> 	check in the "check for subroutine calls" test, and add some 
> 	additional handling for reverse-stepping through shared libraries.
> 
> Index: infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.400
> diff -u -p -r1.400 infrun.c
> --- infrun.c	2 Jul 2009 17:21:06 -0000	1.400
> +++ infrun.c	4 Jul 2009 00:27:53 -0000
> @@ -3776,9 +3776,8 @@ infrun: not switching back to stepped th
>       previous frame must have valid frame IDs.  */
>    if (!frame_id_eq (get_stack_frame_id (frame),
>  		    ecs->event_thread->step_stack_frame_id)
> -      && (frame_id_eq (frame_unwind_caller_id (frame),
> -		       ecs->event_thread->step_stack_frame_id)
> -	  || execution_direction == EXEC_REVERSE))
> +      && frame_id_eq (frame_unwind_caller_id (frame),
> +		      ecs->event_thread->step_stack_frame_id))
>      {
>        CORE_ADDR real_stop_pc;
>  
> @@ -3806,6 +3805,7 @@ infrun: not switching back to stepped th
>        /* Reverse stepping through solib trampolines.  */
>  
>        if (execution_direction == EXEC_REVERSE
> +	  && ecs->event_thread->step_over_calls != STEP_OVER_NONE
>  	  && (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
>  	      || (ecs->stop_func_start == 0
>  		  && in_solib_dynsym_resolve_code (stop_pc))))
> @@ -3923,6 +3923,38 @@ infrun: not switching back to stepped th
>        return;
>      }
>  
> +  /* Reverse stepping through solib trampolines.  */
> +
> +  if (execution_direction == EXEC_REVERSE
> +      && ecs->event_thread->step_over_calls != STEP_OVER_NONE)
> +    {
> +      if (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
> +	  || (ecs->stop_func_start == 0
> +	      && in_solib_dynsym_resolve_code (stop_pc)))
> +	{
> +	  /* Any solib trampoline code can be handled in reverse
> +	     by simply continuing to single-step.  We have already
> +	     executed the solib function (backwards), and a few 
> +	     steps will take us back through the trampoline to the
> +	     caller.  */
> +	  keep_going (ecs);
> +	  return;
> +	}
> +      else if (in_solib_dynsym_resolve_code (stop_pc))
> +	{
> +	  /* Stepped backward into the solib dynsym resolver.
> +	     Set a breakpoint at its start and continue, then
> +	     one more step will take us out.  */
> +	  struct symtab_and_line sr_sal;
> +	  init_sal (&sr_sal);
> +	  sr_sal.pc = ecs->stop_func_start;
> +	  insert_step_resume_breakpoint_at_sal (gdbarch, 
> +						sr_sal, null_frame_id);
> +	  keep_going (ecs);
> +	  return;
> +	}
> +    }
> +
>    /* If we're in the return path from a shared library trampoline,
>       we want to proceed through the trampoline when stepping.  */
>    if (gdbarch_in_solib_return_trampoline (gdbarch,

Re: [RFA] improved handling of shared lib calls (reverse 2/2)

[Michael Snyder]

And now this one?

Michael Snyder wrote:
> Now how about this one?
> 
> Michael Snyder wrote:
>> This change depends on the previous one (epilogue unwinder for i386,
>> http://sourceware.org/ml/gdb-patches/2009-07/msg00099.html).
>>
>> With the frame unwinder working properly, we can remove the
>> special case code in the test for "check for subroutine call"
>> in handle_inferior_event.
>>
>> Of course that reveals some hidden problems with shared library
>> calls, so this patch also addresses those.
>>
>> Michael
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>> 2009-07-03  Michael Snyder  <msnyder@vmware.com>
>>
>> 	* infrun.c (handle_inferior_event): Remove an execution_direction
>> 	check in the "check for subroutine calls" test, and add some 
>> 	additional handling for reverse-stepping through shared libraries.
>>
>> Index: infrun.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/infrun.c,v
>> retrieving revision 1.400
>> diff -u -p -r1.400 infrun.c
>> --- infrun.c	2 Jul 2009 17:21:06 -0000	1.400
>> +++ infrun.c	4 Jul 2009 00:27:53 -0000
>> @@ -3776,9 +3776,8 @@ infrun: not switching back to stepped th
>>       previous frame must have valid frame IDs.  */
>>    if (!frame_id_eq (get_stack_frame_id (frame),
>>  		    ecs->event_thread->step_stack_frame_id)
>> -      && (frame_id_eq (frame_unwind_caller_id (frame),
>> -		       ecs->event_thread->step_stack_frame_id)
>> -	  || execution_direction == EXEC_REVERSE))
>> +      && frame_id_eq (frame_unwind_caller_id (frame),
>> +		      ecs->event_thread->step_stack_frame_id))
>>      {
>>        CORE_ADDR real_stop_pc;
>>  
>> @@ -3806,6 +3805,7 @@ infrun: not switching back to stepped th
>>        /* Reverse stepping through solib trampolines.  */
>>  
>>        if (execution_direction == EXEC_REVERSE
>> +	  && ecs->event_thread->step_over_calls != STEP_OVER_NONE
>>  	  && (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
>>  	      || (ecs->stop_func_start == 0
>>  		  && in_solib_dynsym_resolve_code (stop_pc))))
>> @@ -3923,6 +3923,38 @@ infrun: not switching back to stepped th
>>        return;
>>      }
>>  
>> +  /* Reverse stepping through solib trampolines.  */
>> +
>> +  if (execution_direction == EXEC_REVERSE
>> +      && ecs->event_thread->step_over_calls != STEP_OVER_NONE)
>> +    {
>> +      if (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
>> +	  || (ecs->stop_func_start == 0
>> +	      && in_solib_dynsym_resolve_code (stop_pc)))
>> +	{
>> +	  /* Any solib trampoline code can be handled in reverse
>> +	     by simply continuing to single-step.  We have already
>> +	     executed the solib function (backwards), and a few 
>> +	     steps will take us back through the trampoline to the
>> +	     caller.  */
>> +	  keep_going (ecs);
>> +	  return;
>> +	}
>> +      else if (in_solib_dynsym_resolve_code (stop_pc))
>> +	{
>> +	  /* Stepped backward into the solib dynsym resolver.
>> +	     Set a breakpoint at its start and continue, then
>> +	     one more step will take us out.  */
>> +	  struct symtab_and_line sr_sal;
>> +	  init_sal (&sr_sal);
>> +	  sr_sal.pc = ecs->stop_func_start;
>> +	  insert_step_resume_breakpoint_at_sal (gdbarch, 
>> +						sr_sal, null_frame_id);
>> +	  keep_going (ecs);
>> +	  return;
>> +	}
>> +    }
>> +
>>    /* If we're in the return path from a shared library trampoline,
>>       we want to proceed through the trampoline when stepping.  */
>>    if (gdbarch_in_solib_return_trampoline (gdbarch,
> 
> 

Re: [RFA] improved handling of shared lib calls (reverse 2/2)

[Hui Zhu]

I try this patch with cvs-head and program:
void foo() {
    return;
}

int main() {

	foo(); // 1
	foo(); // 2
	foo(); // 3

	return 0;
}
in ubuntu i386.
And test with reverse testsuite.
Everything is OK.

Thanks,
Hui


On Wed, Jul 15, 2009 at 09:08, Michael Snyder<msnyder@vmware.com> wrote:
> And now this one?
>
> Michael Snyder wrote:
>>
>> Now how about this one?
>>
>> Michael Snyder wrote:
>>>
>>> This change depends on the previous one (epilogue unwinder for i386,
>>> http://sourceware.org/ml/gdb-patches/2009-07/msg00099.html).
>>>
>>> With the frame unwinder working properly, we can remove the
>>> special case code in the test for "check for subroutine call"
>>> in handle_inferior_event.
>>>
>>> Of course that reveals some hidden problems with shared library
>>> calls, so this patch also addresses those.
>>>
>>> Michael
>>>
>>>
>>>
>>> ------------------------------------------------------------------------
>>>
>>> 2009-07-03  Michael Snyder  <msnyder@vmware.com>
>>>
>>>        * infrun.c (handle_inferior_event): Remove an execution_direction
>>>        check in the "check for subroutine calls" test, and add some
>>>  additional handling for reverse-stepping through shared libraries.
>>>
>>> Index: infrun.c
>>> ===================================================================
>>> RCS file: /cvs/src/src/gdb/infrun.c,v
>>> retrieving revision 1.400
>>> diff -u -p -r1.400 infrun.c
>>> --- infrun.c    2 Jul 2009 17:21:06 -0000       1.400
>>> +++ infrun.c    4 Jul 2009 00:27:53 -0000
>>> @@ -3776,9 +3776,8 @@ infrun: not switching back to stepped th
>>>      previous frame must have valid frame IDs.  */
>>>   if (!frame_id_eq (get_stack_frame_id (frame),
>>>                    ecs->event_thread->step_stack_frame_id)
>>> -      && (frame_id_eq (frame_unwind_caller_id (frame),
>>> -                      ecs->event_thread->step_stack_frame_id)
>>> -         || execution_direction == EXEC_REVERSE))
>>> +      && frame_id_eq (frame_unwind_caller_id (frame),
>>> +                     ecs->event_thread->step_stack_frame_id))
>>>     {
>>>       CORE_ADDR real_stop_pc;
>>>  @@ -3806,6 +3805,7 @@ infrun: not switching back to stepped th
>>>       /* Reverse stepping through solib trampolines.  */
>>>         if (execution_direction == EXEC_REVERSE
>>> +         && ecs->event_thread->step_over_calls != STEP_OVER_NONE
>>>          && (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
>>>              || (ecs->stop_func_start == 0
>>>                  && in_solib_dynsym_resolve_code (stop_pc))))
>>> @@ -3923,6 +3923,38 @@ infrun: not switching back to stepped th
>>>       return;
>>>     }
>>>  +  /* Reverse stepping through solib trampolines.  */
>>> +
>>> +  if (execution_direction == EXEC_REVERSE
>>> +      && ecs->event_thread->step_over_calls != STEP_OVER_NONE)
>>> +    {
>>> +      if (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
>>> +         || (ecs->stop_func_start == 0
>>> +             && in_solib_dynsym_resolve_code (stop_pc)))
>>> +       {
>>> +         /* Any solib trampoline code can be handled in reverse
>>> +            by simply continuing to single-step.  We have already
>>> +            executed the solib function (backwards), and a few +
>>>    steps will take us back through the trampoline to the
>>> +            caller.  */
>>> +         keep_going (ecs);
>>> +         return;
>>> +       }
>>> +      else if (in_solib_dynsym_resolve_code (stop_pc))
>>> +       {
>>> +         /* Stepped backward into the solib dynsym resolver.
>>> +            Set a breakpoint at its start and continue, then
>>> +            one more step will take us out.  */
>>> +         struct symtab_and_line sr_sal;
>>> +         init_sal (&sr_sal);
>>> +         sr_sal.pc = ecs->stop_func_start;
>>> +         insert_step_resume_breakpoint_at_sal (gdbarch, +
>>>                                 sr_sal, null_frame_id);
>>> +         keep_going (ecs);
>>> +         return;
>>> +       }
>>> +    }
>>> +
>>>   /* If we're in the return path from a shared library trampoline,
>>>      we want to proceed through the trampoline when stepping.  */
>>>   if (gdbarch_in_solib_return_trampoline (gdbarch,
>>
>>
>
>

Re: [RFA] improved handling of shared lib calls (reverse 2/2)

[Hui Zhu]

[-- Attachment #1: Type: text/plain, Size: 10317 bytes --]

On Wed, Jul 15, 2009 at 16:33, Hui Zhu<teawater@gmail.com> wrote:
> I try this patch with cvs-head and program:
> void foo() {
>    return;
> }
>
> int main() {
>
>        foo(); // 1
>        foo(); // 2
>        foo(); // 3
>
>        return 0;
> }
> in ubuntu i386.
> And test with reverse testsuite.
> Everything is OK.
>
> Thanks,
> Hui
>
>
> On Wed, Jul 15, 2009 at 09:08, Michael Snyder<msnyder@vmware.com> wrote:
>> And now this one?
>>
>> Michael Snyder wrote:
>>>
>>> Now how about this one?
>>>
>>> Michael Snyder wrote:
>>>>
>>>> This change depends on the previous one (epilogue unwinder for i386,
>>>> http://sourceware.org/ml/gdb-patches/2009-07/msg00099.html).
>>>>
>>>> With the frame unwinder working properly, we can remove the
>>>> special case code in the test for "check for subroutine call"
>>>> in handle_inferior_event.
>>>>
>>>> Of course that reveals some hidden problems with shared library
>>>> calls, so this patch also addresses those.
>>>>
>>>> Michael
>>>>
>>>>
>>>>
>>>> ------------------------------------------------------------------------
>>>>
>>>> 2009-07-03  Michael Snyder  <msnyder@vmware.com>
>>>>
>>>>        * infrun.c (handle_inferior_event): Remove an execution_direction
>>>>        check in the "check for subroutine calls" test, and add some
>>>>  additional handling for reverse-stepping through shared libraries.
>>>>
>>>> Index: infrun.c
>>>> ===================================================================
>>>> RCS file: /cvs/src/src/gdb/infrun.c,v
>>>> retrieving revision 1.400
>>>> diff -u -p -r1.400 infrun.c
>>>> --- infrun.c    2 Jul 2009 17:21:06 -0000       1.400
>>>> +++ infrun.c    4 Jul 2009 00:27:53 -0000
>>>> @@ -3776,9 +3776,8 @@ infrun: not switching back to stepped th
>>>>      previous frame must have valid frame IDs.  */
>>>>   if (!frame_id_eq (get_stack_frame_id (frame),
>>>>                    ecs->event_thread->step_stack_frame_id)
>>>> -      && (frame_id_eq (frame_unwind_caller_id (frame),
>>>> -                      ecs->event_thread->step_stack_frame_id)
>>>> -         || execution_direction == EXEC_REVERSE))
>>>> +      && frame_id_eq (frame_unwind_caller_id (frame),
>>>> +                     ecs->event_thread->step_stack_frame_id))
>>>>     {
>>>>       CORE_ADDR real_stop_pc;
>>>>  @@ -3806,6 +3805,7 @@ infrun: not switching back to stepped th
>>>>       /* Reverse stepping through solib trampolines.  */
>>>>         if (execution_direction == EXEC_REVERSE
>>>> +         && ecs->event_thread->step_over_calls != STEP_OVER_NONE
>>>>          && (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
>>>>              || (ecs->stop_func_start == 0
>>>>                  && in_solib_dynsym_resolve_code (stop_pc))))
>>>> @@ -3923,6 +3923,38 @@ infrun: not switching back to stepped th
>>>>       return;
>>>>     }
>>>>  +  /* Reverse stepping through solib trampolines.  */
>>>> +
>>>> +  if (execution_direction == EXEC_REVERSE
>>>> +      && ecs->event_thread->step_over_calls != STEP_OVER_NONE)
>>>> +    {
>>>> +      if (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
>>>> +         || (ecs->stop_func_start == 0
>>>> +             && in_solib_dynsym_resolve_code (stop_pc)))
>>>> +       {
>>>> +         /* Any solib trampoline code can be handled in reverse
>>>> +            by simply continuing to single-step.  We have already
>>>> +            executed the solib function (backwards), and a few +
>>>>    steps will take us back through the trampoline to the
>>>> +            caller.  */
>>>> +         keep_going (ecs);
>>>> +         return;
>>>> +       }
>>>> +      else if (in_solib_dynsym_resolve_code (stop_pc))
>>>> +       {
>>>> +         /* Stepped backward into the solib dynsym resolver.
>>>> +            Set a breakpoint at its start and continue, then
>>>> +            one more step will take us out.  */
>>>> +         struct symtab_and_line sr_sal;
>>>> +         init_sal (&sr_sal);
>>>> +         sr_sal.pc = ecs->stop_func_start;
>>>> +         insert_step_resume_breakpoint_at_sal (gdbarch, +
>>>>                                 sr_sal, null_frame_id);
>>>> +         keep_going (ecs);
>>>> +         return;
>>>> +       }
>>>> +    }
>>>> +
>>>>   /* If we're in the return path from a shared library trampoline,
>>>>      we want to proceed through the trampoline when stepping.  */
>>>>   if (gdbarch_in_solib_return_trampoline (gdbarch,
>>>
>>>
>>
>>
>

In AMD64,  This patch cannot pass the "solib-reverse.exp" even if with
the "set_gdbarch_skip_trampoline_code (gdbarch,
find_solib_trampoline_target);" patch.
And cannot pass the:
void foo() {
   return;
}

int main() {

       foo(); // 1
       foo(); // 2
       foo(); // 3

       return 0;
}

I told that we can make " Add step_stack_prev_frame_id" patch together
with this patch.
I make update the patch.  It can handle the solib-reverse.exp and foo.
 I think it let us don't worry about the arch code.

What do you think about it?

Thanks,
Hui

2009-07-16  Michael Snyder  <msnyder@vmware.com>
            Hui Zhu  <teawater@gmail.com>

	* gdbthread.h (thread_info): Add step_stack_prev_frame_id to
	save the prev_frame_id when we can get the prev_frame.
	* infrun.c (inferior_status): Ditto.
	(save_inferior_status): Ditto.
	(restore_inferior_status): Ditto.
	(set_step_info): Get the step_stack_prev_frame_id.
	(handle_inferior_event): Use the step_stack_prev_frame_id make
	sure inferior stepped into subroutine or not.  Add some
	additional handling for reverse-stepping through shared
	libraries.

---
 gdbthread.h |    3 +++
 infrun.c    |   49 +++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 50 insertions(+), 2 deletions(-)

--- a/gdbthread.h
+++ b/gdbthread.h
@@ -88,6 +88,9 @@ struct thread_info
      any inlined frames).  */
   struct frame_id step_stack_frame_id;

+  /* The prev frame id of step_stack_frame_id.  */
+  struct frame_id step_stack_prev_frame_id;
+
   int current_line;
   struct symtab *current_symtab;

--- a/infrun.c
+++ b/infrun.c
@@ -2153,10 +2153,17 @@ void
 set_step_info (struct frame_info *frame, struct symtab_and_line sal)
 {
   struct thread_info *tp = inferior_thread ();
+  struct frame_info *prev_frame;

   tp->step_frame_id = get_frame_id (frame);
   tp->step_stack_frame_id = get_stack_frame_id (frame);

+  prev_frame = get_prev_frame (frame);
+  if (prev_frame)
+    tp->step_stack_prev_frame_id = get_stack_frame_id (prev_frame);
+  else
+    tp->step_stack_prev_frame_id = null_frame_id;
+
   tp->current_symtab = sal.symtab;
   tp->current_line = sal.line;
 }
@@ -3777,8 +3784,10 @@ infrun: not switching back to stepped th
   if (!frame_id_eq (get_stack_frame_id (frame),
 		    ecs->event_thread->step_stack_frame_id)
       && (frame_id_eq (frame_unwind_caller_id (frame),
-		       ecs->event_thread->step_stack_frame_id)
-	  || execution_direction == EXEC_REVERSE))
+		      ecs->event_thread->step_stack_frame_id)
+	  || (execution_direction == EXEC_REVERSE
+               && !frame_id_eq (get_stack_frame_id (frame),
+                                ecs->event_thread->step_stack_prev_frame_id))))
     {
       CORE_ADDR real_stop_pc;

@@ -3806,6 +3815,7 @@ infrun: not switching back to stepped th
       /* Reverse stepping through solib trampolines.  */

       if (execution_direction == EXEC_REVERSE
+	  && ecs->event_thread->step_over_calls != STEP_OVER_NONE
 	  && (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
 	      || (ecs->stop_func_start == 0
 		  && in_solib_dynsym_resolve_code (stop_pc))))
@@ -3923,6 +3933,38 @@ infrun: not switching back to stepped th
       return;
     }

+  /* Reverse stepping through solib trampolines.  */
+
+  if (execution_direction == EXEC_REVERSE
+      && ecs->event_thread->step_over_calls != STEP_OVER_NONE)
+    {
+      if (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
+	  || (ecs->stop_func_start == 0
+	      && in_solib_dynsym_resolve_code (stop_pc)))
+	{
+	  /* Any solib trampoline code can be handled in reverse
+	     by simply continuing to single-step.  We have already
+	     executed the solib function (backwards), and a few
+	     steps will take us back through the trampoline to the
+	     caller.  */
+	  keep_going (ecs);
+	  return;
+	}
+      else if (in_solib_dynsym_resolve_code (stop_pc))
+	{
+	  /* Stepped backward into the solib dynsym resolver.
+	     Set a breakpoint at its start and continue, then
+	     one more step will take us out.  */
+	  struct symtab_and_line sr_sal;
+	  init_sal (&sr_sal);
+	  sr_sal.pc = ecs->stop_func_start;
+	  insert_step_resume_breakpoint_at_sal (gdbarch,
+						sr_sal, null_frame_id);
+	  keep_going (ecs);
+	  return;
+	}
+    }
+
   /* If we're in the return path from a shared library trampoline,
      we want to proceed through the trampoline when stepping.  */
   if (gdbarch_in_solib_return_trampoline (gdbarch,
@@ -5388,6 +5430,7 @@ struct inferior_status
   CORE_ADDR step_range_end;
   struct frame_id step_frame_id;
   struct frame_id step_stack_frame_id;
+  struct frame_id step_stack_prev_frame_id;
   enum step_over_calls_kind step_over_calls;
   CORE_ADDR step_resume_break_address;
   int stop_after_trap;
@@ -5418,6 +5461,7 @@ save_inferior_status (void)
   inf_status->step_range_end = tp->step_range_end;
   inf_status->step_frame_id = tp->step_frame_id;
   inf_status->step_stack_frame_id = tp->step_stack_frame_id;
+  inf_status->step_stack_prev_frame_id = tp->step_stack_prev_frame_id;
   inf_status->step_over_calls = tp->step_over_calls;
   inf_status->stop_after_trap = stop_after_trap;
   inf_status->stop_soon = inf->stop_soon;
@@ -5472,6 +5516,7 @@ restore_inferior_status (struct inferior
   tp->step_range_end = inf_status->step_range_end;
   tp->step_frame_id = inf_status->step_frame_id;
   tp->step_stack_frame_id = inf_status->step_stack_frame_id;
+  tp->step_stack_prev_frame_id = inf_status->step_stack_prev_frame_id;
   tp->step_over_calls = inf_status->step_over_calls;
   stop_after_trap = inf_status->stop_after_trap;
   inf->stop_soon = inf_status->stop_soon;

[-- Attachment #2: solib.txt --]
[-- Type: text/plain, Size: 4519 bytes --]

---
 gdbthread.h |    3 +++
 infrun.c    |   49 +++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 50 insertions(+), 2 deletions(-)

--- a/gdbthread.h
+++ b/gdbthread.h
@@ -88,6 +88,9 @@ struct thread_info
      any inlined frames).  */
   struct frame_id step_stack_frame_id;
 
+  /* The prev frame id of step_stack_frame_id.  */
+  struct frame_id step_stack_prev_frame_id;
+
   int current_line;
   struct symtab *current_symtab;
 
--- a/infrun.c
+++ b/infrun.c
@@ -2153,10 +2153,17 @@ void
 set_step_info (struct frame_info *frame, struct symtab_and_line sal)
 {
   struct thread_info *tp = inferior_thread ();
+  struct frame_info *prev_frame;
 
   tp->step_frame_id = get_frame_id (frame);
   tp->step_stack_frame_id = get_stack_frame_id (frame);
 
+  prev_frame = get_prev_frame (frame);
+  if (prev_frame)
+    tp->step_stack_prev_frame_id = get_stack_frame_id (prev_frame);
+  else
+    tp->step_stack_prev_frame_id = null_frame_id;
+
   tp->current_symtab = sal.symtab;
   tp->current_line = sal.line;
 }
@@ -3777,8 +3784,10 @@ infrun: not switching back to stepped th
   if (!frame_id_eq (get_stack_frame_id (frame),
 		    ecs->event_thread->step_stack_frame_id)
       && (frame_id_eq (frame_unwind_caller_id (frame),
-		       ecs->event_thread->step_stack_frame_id)
-	  || execution_direction == EXEC_REVERSE))
+		      ecs->event_thread->step_stack_frame_id)
+	  || (execution_direction == EXEC_REVERSE
+               && !frame_id_eq (get_stack_frame_id (frame),
+                                ecs->event_thread->step_stack_prev_frame_id))))
     {
       CORE_ADDR real_stop_pc;
 
@@ -3806,6 +3815,7 @@ infrun: not switching back to stepped th
       /* Reverse stepping through solib trampolines.  */
 
       if (execution_direction == EXEC_REVERSE
+	  && ecs->event_thread->step_over_calls != STEP_OVER_NONE
 	  && (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
 	      || (ecs->stop_func_start == 0
 		  && in_solib_dynsym_resolve_code (stop_pc))))
@@ -3923,6 +3933,38 @@ infrun: not switching back to stepped th
       return;
     }
 
+  /* Reverse stepping through solib trampolines.  */
+
+  if (execution_direction == EXEC_REVERSE
+      && ecs->event_thread->step_over_calls != STEP_OVER_NONE)
+    {
+      if (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
+	  || (ecs->stop_func_start == 0
+	      && in_solib_dynsym_resolve_code (stop_pc)))
+	{
+	  /* Any solib trampoline code can be handled in reverse
+	     by simply continuing to single-step.  We have already
+	     executed the solib function (backwards), and a few
+	     steps will take us back through the trampoline to the
+	     caller.  */
+	  keep_going (ecs);
+	  return;
+	}
+      else if (in_solib_dynsym_resolve_code (stop_pc))
+	{
+	  /* Stepped backward into the solib dynsym resolver.
+	     Set a breakpoint at its start and continue, then
+	     one more step will take us out.  */
+	  struct symtab_and_line sr_sal;
+	  init_sal (&sr_sal);
+	  sr_sal.pc = ecs->stop_func_start;
+	  insert_step_resume_breakpoint_at_sal (gdbarch,
+						sr_sal, null_frame_id);
+	  keep_going (ecs);
+	  return;
+	}
+    }
+
   /* If we're in the return path from a shared library trampoline,
      we want to proceed through the trampoline when stepping.  */
   if (gdbarch_in_solib_return_trampoline (gdbarch,
@@ -5388,6 +5430,7 @@ struct inferior_status
   CORE_ADDR step_range_end;
   struct frame_id step_frame_id;
   struct frame_id step_stack_frame_id;
+  struct frame_id step_stack_prev_frame_id;
   enum step_over_calls_kind step_over_calls;
   CORE_ADDR step_resume_break_address;
   int stop_after_trap;
@@ -5418,6 +5461,7 @@ save_inferior_status (void)
   inf_status->step_range_end = tp->step_range_end;
   inf_status->step_frame_id = tp->step_frame_id;
   inf_status->step_stack_frame_id = tp->step_stack_frame_id;
+  inf_status->step_stack_prev_frame_id = tp->step_stack_prev_frame_id;
   inf_status->step_over_calls = tp->step_over_calls;
   inf_status->stop_after_trap = stop_after_trap;
   inf_status->stop_soon = inf->stop_soon;
@@ -5472,6 +5516,7 @@ restore_inferior_status (struct inferior
   tp->step_range_end = inf_status->step_range_end;
   tp->step_frame_id = inf_status->step_frame_id;
   tp->step_stack_frame_id = inf_status->step_stack_frame_id;
+  tp->step_stack_prev_frame_id = inf_status->step_stack_prev_frame_id;
   tp->step_over_calls = inf_status->step_over_calls;
   stop_after_trap = inf_status->stop_after_trap;
   inf->stop_soon = inf_status->stop_soon;

Re: [RFA] improved handling of shared lib calls (reverse 2/2)

[Michael Snyder]

Committing this change.

Michael Snyder wrote:
> This change depends on the previous one (epilogue unwinder for i386,
> http://sourceware.org/ml/gdb-patches/2009-07/msg00099.html).
> 
> With the frame unwinder working properly, we can remove the
> special case code in the test for "check for subroutine call"
> in handle_inferior_event.
> 
> Of course that reveals some hidden problems with shared library
> calls, so this patch also addresses those.
> 
> Michael
> 
> 
> 
> ------------------------------------------------------------------------
> 
> 2009-07-03  Michael Snyder  <msnyder@vmware.com>
> 
> 	* infrun.c (handle_inferior_event): Remove an execution_direction
> 	check in the "check for subroutine calls" test, and add some 
> 	additional handling for reverse-stepping through shared libraries.
> 
> Index: infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.400
> diff -u -p -r1.400 infrun.c
> --- infrun.c	2 Jul 2009 17:21:06 -0000	1.400
> +++ infrun.c	4 Jul 2009 00:27:53 -0000
> @@ -3776,9 +3776,8 @@ infrun: not switching back to stepped th
>       previous frame must have valid frame IDs.  */
>    if (!frame_id_eq (get_stack_frame_id (frame),
>  		    ecs->event_thread->step_stack_frame_id)
> -      && (frame_id_eq (frame_unwind_caller_id (frame),
> -		       ecs->event_thread->step_stack_frame_id)
> -	  || execution_direction == EXEC_REVERSE))
> +      && frame_id_eq (frame_unwind_caller_id (frame),
> +		      ecs->event_thread->step_stack_frame_id))
>      {
>        CORE_ADDR real_stop_pc;
>  
> @@ -3806,6 +3805,7 @@ infrun: not switching back to stepped th
>        /* Reverse stepping through solib trampolines.  */
>  
>        if (execution_direction == EXEC_REVERSE
> +	  && ecs->event_thread->step_over_calls != STEP_OVER_NONE
>  	  && (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
>  	      || (ecs->stop_func_start == 0
>  		  && in_solib_dynsym_resolve_code (stop_pc))))
> @@ -3923,6 +3923,38 @@ infrun: not switching back to stepped th
>        return;
>      }
>  
> +  /* Reverse stepping through solib trampolines.  */
> +
> +  if (execution_direction == EXEC_REVERSE
> +      && ecs->event_thread->step_over_calls != STEP_OVER_NONE)
> +    {
> +      if (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
> +	  || (ecs->stop_func_start == 0
> +	      && in_solib_dynsym_resolve_code (stop_pc)))
> +	{
> +	  /* Any solib trampoline code can be handled in reverse
> +	     by simply continuing to single-step.  We have already
> +	     executed the solib function (backwards), and a few 
> +	     steps will take us back through the trampoline to the
> +	     caller.  */
> +	  keep_going (ecs);
> +	  return;
> +	}
> +      else if (in_solib_dynsym_resolve_code (stop_pc))
> +	{
> +	  /* Stepped backward into the solib dynsym resolver.
> +	     Set a breakpoint at its start and continue, then
> +	     one more step will take us out.  */
> +	  struct symtab_and_line sr_sal;
> +	  init_sal (&sr_sal);
> +	  sr_sal.pc = ecs->stop_func_start;
> +	  insert_step_resume_breakpoint_at_sal (gdbarch, 
> +						sr_sal, null_frame_id);
> +	  keep_going (ecs);
> +	  return;
> +	}
> +    }
> +
>    /* If we're in the return path from a shared library trampoline,
>       we want to proceed through the trampoline when stepping.  */
>    if (gdbarch_in_solib_return_trampoline (gdbarch,