[patch/rfc] How to handle stepping into an unresolved plt entry?

[patch/rfc] How to handle stepping into an unresolved plt entry?

[Randolph Chung]

On hppa, I noticed that when I do a "step" over an indirect function
call, gdb does not stop inside the called function.  I've traced the
problem down to the following:

An indirect function call on hppa goes through a stub ($$dyncall).
$$dyncall loads a PLABEL from the PLT and jumps to it. In the case that
the PLT is not yet resolved, we need to do the normal PLT fixup () in 
the loader before calling the function.

What seems to be happening now is that when a "step" happens at the
indirect function call, we step into the $$dyncall stub, and in
infrun.c, we do a SKIP_TRAMPOLINE_CODE. This resolves into a pc that
points at the fixup routine. Next, in handle_inferior_event (), we try
to lookup the file/line information for this target pc; it's not found,
and so gdb just goes ahead and insert a breakpoint at the return address
of the stub (which is in the caller) and then calls keep_going ().
If we do a step on a second invocation of the indirect function call, it
works correctly; SKIP_TRAMPOLINE_CODE resolves to an address that points
to the target function, so the file/line lookup works and we insert a
breakpoint in the right place.

What is actually supposed to happen in this case? The documentation says:
@findex SKIP_TRAMPOLINE_CODE
If the target machine has trampoline code that sits between callers and
the functions being called, then define this macro to return a new PC
that is at the start of the real function.

But how do you do this if your target address is not yet resolved? The
attached patch gives a workaround for this problem (also needs a small
fix to the target code), but it seems a bit clumsy to me.... it's not
like handle_inferior_event () needs any more special cases... :-(

Comments? Suggestions?

thanks
randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/


2004-05-20  Randolph Chung  <tausq@debian.org>

	* infrun.c (handle_inferior_event): If skipping a trampoline results
	in an address in the resolver, stop there before continuing.
	* hppa-linux-tdep.c (hppa_linux_skip_trampoline_code): Handle the
	case when the plabel points to a PLT stub.

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.163
diff -u -p -r1.163 infrun.c
--- infrun.c	14 May 2004 18:45:42 -0000	1.163
+++ infrun.c	21 May 2004 06:36:36 -0000
@@ -2435,6 +2443,24 @@ process_event_stop_test:
 	  }
       }
 

+    /* If skipping a trampoline results in an address in the resolver,
+       stop there before continuing.  */
+      if (IN_SOLIB_DYNSYM_RESOLVE_CODE (ecs->stop_func_start))
+	{
+	  struct symtab_and_line sr_sal;
+	  init_sal (&sr_sal);
+	  sr_sal.pc = ecs->stop_func_start;
+
+	  check_for_old_step_resume_breakpoint ();
+	  step_resume_breakpoint =
+	    set_momentary_breakpoint (sr_sal, null_frame_id, bp_step_resume);
+	  if (breakpoints_inserted)
+	    insert_breakpoints ();
+
+          keep_going (ecs);
+          return;
+	}
+
       /* If we have no line number and the step-stop-if-no-debug is
 	 set, we stop the step so that the user has a chance to switch
 	 in assembly mode.  */
Index: hppa-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-linux-tdep.c,v
retrieving revision 1.5
diff -u -p -r1.5 hppa-linux-tdep.c
--- hppa-linux-tdep.c	8 May 2004 03:59:34 -0000	1.5
+++ hppa-linux-tdep.c	21 May 2004 06:36:36 -0000
@@ -219,6 +211,24 @@ hppa_linux_skip_trampoline_code (CORE_AD
       if (pc & 0x2)
 	pc = (CORE_ADDR) read_memory_integer (pc & ~0x3, TARGET_PTR_BIT / 8);
 
+      /* if the plt slot has not yet been resolved, the target will
+         be the plt stub */
+      if (in_plt_section (pc, NULL))
+	{
+	  /* Sanity check: are we pointing to the plt stub? */
+  	  if (insns_match_pattern (pc, hppa_plt_stub, insn))
+	    {
+	      /* this should point to the fixup routine */
+      	      pc = (CORE_ADDR) read_memory_integer (pc + 8, TARGET_PTR_BIT / 8);
+	    }
+	  else
+	    {
+	      error ("Cannot resolve plt stub at 0x%s\n",
+		     paddr_nz (pc));
+	      pc = 0;
+	    }
+	}
+
       return pc;
     }
 

Re: [patch/rfc] How to handle stepping into an unresolved plt entry?

[Andrew Cagney]

> On hppa, I noticed that when I do a "step" over an indirect function
> call, gdb does not stop inside the called function.  I've traced the
> problem down to the following:
> 
> An indirect function call on hppa goes through a stub ($$dyncall).
> $$dyncall loads a PLABEL from the PLT and jumps to it. In the case that
> the PLT is not yet resolved, we need to do the normal PLT fixup () in 
> the loader before calling the function.
> 
> What seems to be happening now is that when a "step" happens at the
> indirect function call, we step into the $$dyncall stub, and in
> infrun.c, we do a SKIP_TRAMPOLINE_CODE. This resolves into a pc that
> points at the fixup routine. Next, in handle_inferior_event (), we try
> to lookup the file/line information for this target pc; it's not found,
> and so gdb just goes ahead and insert a breakpoint at the return address
> of the stub (which is in the caller) and then calls keep_going ().
> If we do a step on a second invocation of the indirect function call, it
> works correctly; SKIP_TRAMPOLINE_CODE resolves to an address that points
> to the target function, so the file/line lookup works and we insert a
> breakpoint in the right place.

Hmm, should gdb put a greater reliance on SKIP_TRAMPOLINE_CODE. 
Something like a new separate clause:

   if (we've stepped into a function
       && we're not stopping in this sort of code
       && skip trampoline returns something)
     run to skip trampoline breakpoint, possibly doing a step into function


> What is actually supposed to happen in this case? The documentation says:
> @findex SKIP_TRAMPOLINE_CODE

I think these should be generic attributes of the frame, as you note, 
there are already too many special cases.

Andrew

> If the target machine has trampoline code that sits between callers and
> the functions being called, then define this macro to return a new PC
> that is at the start of the real function.
> 
> But how do you do this if your target address is not yet resolved? The
> attached patch gives a workaround for this problem (also needs a small
> fix to the target code), but it seems a bit clumsy to me.... it's not
> like handle_inferior_event () needs any more special cases... :-(
> 
> Comments? Suggestions?

Re: [patch/rfc] How to handle stepping into an unresolved plt entry?

[Randolph Chung]

> Hmm, should gdb put a greater reliance on SKIP_TRAMPOLINE_CODE. 
> Something like a new separate clause:
> 
>   if (we've stepped into a function
>       && we're not stopping in this sort of code
>       && skip trampoline returns something)
>     run to skip trampoline breakpoint, possibly doing a step into function

sounds ok; i'm curious about what are the other trampoline cases that we
need to deal with. i found another case on hppa where what needs to
happen doesn't really match what infrun.c is doing (i'll describe it
below), but it'll be good if somehow we can handle these in a sort of
standard way so that h_i_e is actually understandable :)

the other problem i've seen is when stepping into a shared library call
-- again, this goes through a stub. gdb steps into the stub, but before
it has a chance to skip over the stub, it hits this (since the stub has
no corresponding symbol):

  if (step_over_calls == STEP_OVER_UNDEBUGGABLE
      && ecs->stop_func_name == NULL)
    {
      /* The inferior just stepped into, or returned to, an
         undebuggable function (where there is no symbol, not even a
         minimal symbol, corresponding to the address where the
         inferior stopped).  Since we want to skip this kind of code,
         we keep going until the inferior returns from this
         function.  */
[...]
          /* Set a breakpoint at callee's return address (the address
             at which the caller will resume).  */
          insert_step_resume_breakpoint (get_prev_frame (get_current_frame ()),
                                         ecs);
          keep_going (ecs);
          return;
    }

if i move this block after the next block (that checks for stub frames
and does SKIP_TRAMPOLINE_CODE () processing) then it works...

I'm still trying to understand why the checks in h_i_e happen in the
current order.... so i don't know whether this is a problem with h_i_e
or somewhere else...

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/rfc] How to handle stepping into an unresolved plt entry?

[Andrew Cagney]

>>Hmm, should gdb put a greater reliance on SKIP_TRAMPOLINE_CODE. 
>>> Something like a new separate clause:
>>> 
>>>   if (we've stepped into a function
>>>       && we're not stopping in this sort of code
>>>       && skip trampoline returns something)
>>>     run to skip trampoline breakpoint, possibly doing a step into function
> 
> 
> sounds ok; i'm curious about what are the other trampoline cases that we
> need to deal with. i found another case on hppa where what needs to
> happen doesn't really match what infrun.c is doing (i'll describe it
> below), but it'll be good if somehow we can handle these in a sort of
> standard way so that h_i_e is actually understandable :)
> 
> the other problem i've seen is when stepping into a shared library call
> -- again, this goes through a stub. gdb steps into the stub, but before
> it has a chance to skip over the stub, it hits this (since the stub has
> no corresponding symbol):

That case should be handled by the same skip-trampoline logic (it is 
just another trampoline).

>   if (step_over_calls == STEP_OVER_UNDEBUGGABLE
>       && ecs->stop_func_name == NULL)
>     {
>       /* The inferior just stepped into, or returned to, an
>          undebuggable function (where there is no symbol, not even a
>          minimal symbol, corresponding to the address where the
>          inferior stopped).  Since we want to skip this kind of code,
>          we keep going until the inferior returns from this
>          function.  */
> [...]
>           /* Set a breakpoint at callee's return address (the address
>              at which the caller will resume).  */
>           insert_step_resume_breakpoint (get_prev_frame (get_current_frame ()),
>                                          ecs);
>           keep_going (ecs);
>           return;
>     }
> 
> if i move this block after the next block (that checks for stub frames
> and does SKIP_TRAMPOLINE_CODE () processing) then it works...
> 
> I'm still trying to understand why the checks in h_i_e happen in the
> current order.... so i don't know whether this is a problem with h_i_e
> or somewhere else...

The rationale (if we think it can be called that :-) is lost in the 
depths of time.   Perhaps some unwinders barfed when presented with a 
function with no function name so it was thought that doing the check 
first was prudent?

Anyway, can the IN_SOLIB_RETURN_TRAMPOLINE be moved up as well (with an 
additional ecs->stop_func_name != NULL check?

Andrew

Re: [patch/rfc] How to handle stepping into an unresolved plt entry?

[Randolph Chung]

> Anyway, can the IN_SOLIB_RETURN_TRAMPOLINE be moved up as well (with an 
> additional ecs->stop_func_name != NULL check?

I don't think checking the stop_func_name is correct; some trampolines
can be named. For example on hpux the export stubs are named. On both
hppa-linux and hpux there is a $$dyncall stub which is named.

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/rfc] How to handle stepping into an unresolved plt entry?

[Randolph Chung]

> The rationale (if we think it can be called that :-) is lost in the 
> depths of time.   Perhaps some unwinders barfed when presented with a 
> function with no function name so it was thought that doing the check 
> first was prudent?
> 
> Anyway, can the IN_SOLIB_RETURN_TRAMPOLINE be moved up as well (with an 
> additional ecs->stop_func_name != NULL check?

The attached patch contains the new arrangement of tests that works
better on hppa. Regression tested on i386-linux. I'd be interested to
see test results for some of the ppc variants, since they seem to be
also have interesting trampoline code.

Also, some comments from looking at the 2nd half of
handle_inferior_event (stepping out of stepping range).... the new 
order of cases being handled is:

1. Runtime loader 
2. Signal trampoline 
3. current frame == step frame
   3a. stepi/nexti-in-prologue 
   3b. next/IGNORE_HELPER_CALL check
   3c. skip trampolines
   3d. Runtime loader check
   3e. If line number available at target, step into function
       else put breakpoint in previous frame and continue 
4. Return trampoline
5. Stopped at function with no name
6. stepi/nexti
7. No line number info
8. Stepped to next line
9. Stepped out of current function
10. Update current frame and continue

Several questions:
- At step 3:
  if (frame_id_eq (frame_unwind_id (get_current_frame ()),
                   step_frame_id))
    {
      /* It's a subroutine call.  */

Why is this "a subroutine call"? It seems that in the general case this
is not a subroutine call.....

- At step 3b, IGNORE_HELPER_CALL () is only defined for mips. The
  comments say:
/* On MIPS16, a function that returns a floating point value may call
   a library helper function to copy the return value to a floating point
   register.  The IGNORE_HELPER_CALL macro returns non-zero if we
   should ignore (i.e. step over) this function call.  */

Shouldn't this be handled by the trampoline logic? I guess this might
be historical...

At step 5: we declare the function as "undebuggable" if it has no name,
but elsewhere in this function it seems to equate "undebuggable" with
"no line number" in a sal lookup.... are both tests needed, or is 5/7
redundant?

Other comments?

thanks,
randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/


2004-05-24  Randolph Chung  <tausq@debian.org>

	* infrun.c (handle_inferior_event): Handle the case when a trampoline
	ends up in the runtime resolver, and if the trampoline has no name.
	Rearrange the code so that all the trampoline processing happens before
	other step-out-of-range handling.

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.163
diff -u -p -r1.163 infrun.c
--- infrun.c	14 May 2004 18:45:42 -0000	1.163
+++ infrun.c	25 May 2004 06:33:51 -0000
@@ -2344,36 +2344,6 @@ process_event_stop_test:
       return;
     }
 
-  if (step_over_calls == STEP_OVER_UNDEBUGGABLE
-      && ecs->stop_func_name == NULL)
-    {
-      /* The inferior just stepped into, or returned to, an
-         undebuggable function (where there is no symbol, not even a
-         minimal symbol, corresponding to the address where the
-         inferior stopped).  Since we want to skip this kind of code,
-         we keep going until the inferior returns from this
-         function.  */
-      if (step_stop_if_no_debug)
-	{
-	  /* If we have no line number and the step-stop-if-no-debug
-	     is set, we stop the step so that the user has a chance to
-	     switch in assembly mode.  */
-	  stop_step = 1;
-	  print_stop_reason (END_STEPPING_RANGE, 0);
-	  stop_stepping (ecs);
-	  return;
-	}
-      else
-	{
-	  /* Set a breakpoint at callee's return address (the address
-	     at which the caller will resume).  */
-	  insert_step_resume_breakpoint (get_prev_frame (get_current_frame ()),
-					 ecs);
-	  keep_going (ecs);
-	  return;
-	}
-    }
-
   if (frame_id_eq (frame_unwind_id (get_current_frame ()),
                    step_frame_id))
     {
@@ -2418,6 +2388,22 @@ process_event_stop_test:
       if (real_stop_pc != 0)
 	ecs->stop_func_start = real_stop_pc;
       
+      if (IN_SOLIB_DYNSYM_RESOLVE_CODE (ecs->stop_func_start))
+	{
+	  struct symtab_and_line sr_sal;
+	  init_sal (&sr_sal);
+	  sr_sal.pc = ecs->stop_func_start;
+
+	  check_for_old_step_resume_breakpoint ();
+	  step_resume_breakpoint =
+	    set_momentary_breakpoint (sr_sal, null_frame_id, bp_step_resume);
+	  if (breakpoints_inserted)
+	    insert_breakpoints ();
+
+          keep_going (ecs);
+          return;
+	}
+
       /* If we have line number information for the function we are
 	 thinking of stepping into, step into it.
 
@@ -2453,20 +2439,6 @@ process_event_stop_test:
       return;
     }
 
-  /* We've wandered out of the step range.  */
-
-  ecs->sal = find_pc_line (stop_pc, 0);
-
-  if (step_range_end == 1)
-    {
-      /* It is stepi or nexti.  We always want to stop stepping after
-         one instruction.  */
-      stop_step = 1;
-      print_stop_reason (END_STEPPING_RANGE, 0);
-      stop_stepping (ecs);
-      return;
-    }
-
   /* If we're in the return path from a shared library trampoline,
      we want to proceed through the trampoline when stepping.  */
   if (IN_SOLIB_RETURN_TRAMPOLINE (stop_pc, ecs->stop_func_name))
@@ -2498,6 +2470,51 @@ process_event_stop_test:
 	  return;
 	}
     }
+
+  /* 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.  */
+  if (step_over_calls == STEP_OVER_UNDEBUGGABLE
+      && ecs->stop_func_name == NULL)
+    {
+      /* The inferior just stepped into, or returned to, an
+         undebuggable function (where there is no symbol, not even a
+         minimal symbol, corresponding to the address where the
+         inferior stopped).  Since we want to skip this kind of code,
+         we keep going until the inferior returns from this
+         function.  */
+      if (step_stop_if_no_debug)
+	{
+	  /* If we have no line number and the step-stop-if-no-debug
+	     is set, we stop the step so that the user has a chance to
+	     switch in assembly mode.  */
+	  stop_step = 1;
+	  print_stop_reason (END_STEPPING_RANGE, 0);
+	  stop_stepping (ecs);
+	  return;
+	}
+      else
+	{
+	  /* Set a breakpoint at callee's return address (the address
+	     at which the caller will resume).  */
+	  insert_step_resume_breakpoint (get_prev_frame (get_current_frame ()),
+					 ecs);
+	  keep_going (ecs);
+	  return;
+	}
+    }
+
+  if (step_range_end == 1)
+    {
+      /* It is stepi or nexti.  We always want to stop stepping after
+         one instruction.  */
+      stop_step = 1;
+      print_stop_reason (END_STEPPING_RANGE, 0);
+      stop_stepping (ecs);
+      return;
+    }
+
+  ecs->sal = find_pc_line (stop_pc, 0);
 
   if (ecs->sal.line == 0)
     {