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]

Re: RFC: patch to refresh prev_pc


The following patch solves a problem on the ia64.  The problem
exists because of a generic problem to reset the prev_pc value
after an inferior function call or after a return command.

Because the value is not properly set, the line number used to
initialize the ecs is incorrect.  On the ia64 this causes a problem
because there are extraneous linetable entries generated by the compiler
that are within the line (i.e. they don't change the line number).
When we apply "next" logic which uses the ecs line number, we end up stopping
at the first line table entry past our start position.  This often ends
up being just a few insns farther in the same line.  A specific example
of this problem is the next to 1237 test inside call-ar-st.exp.  An
inferior call is made on line 1236 and upon return we issue a next.

I discussed this topic on the gdb forum and a number of attempts were
made to ensure the prev_pc value was up to date in init_execution_control_state()
in infrun.c.  Those attempts failed because the inferior was not guaranteed to
be stopped and so we weren't guaranteed that a ptrace to fetch the pc would work.

This patch attempts to refresh the prev_pc value just before resuming in proceed().
It works for the ia64 problems cited above and also I have tested it on the x86.

Is this patch ok?

Yes, definitly a better strategy. Two some tweaks:


- That new single line assignment needs some sort of big jucy stand-alone comment that explains the rationale for the change, mention where it was before, and where else was tried (and why both failed). The more details the better, but something based on the above would do the trick.

- the old (now redundant) code in stop_stepping vis:

  if (target_has_execution)
    {
      /* Assuming the inferior still exists, set these up for next
         time, just like we did above if we didn't break out of the
         loop.  */
      prev_pc = read_pc ();
    }

should be deleted (but check that it really does still work).

With those changes made, consider it approved.

Andrew


2003-05-05 Jeff Johnston <jjohnstn@redhat.com>

    * infrun.c (prev_pc): Move declaration ahead of proceed().
    (proceed): Refresh prev_pc value before resuming.



Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.108
diff -u -p -r1.108 infrun.c
--- infrun.c 5 May 2003 00:27:07 -0000 1.108
+++ infrun.c 5 May 2003 20:34:16 -0000
@@ -667,6 +667,12 @@ clear_proceed_status (void)
bpstat_clear (&stop_bpstat);
}
+
+/* Record the pc of the program the last time it stopped. This is
+ just used internally by wait_for_inferior, but need to be preserved
+ over calls to it and cleared when the inferior is started. */
+static CORE_ADDR prev_pc;
+
/* Basic routine for continuing the program in various fashions.
ADDR is the address to resume at, or -1 for resume where stopped.
@@ -772,6 +778,10 @@ proceed (CORE_ADDR addr, enum target_sig
inferior. */
gdb_flush (gdb_stdout);
+ /* Refresh prev_pc value which may have been altered by an inferior
+ function call or a return command. */
+ prev_pc = read_pc ();
+
/* Resume inferior. */
resume (oneproc || step || bpstat_should_step (), stop_signal);
@@ -785,11 +795,6 @@ proceed (CORE_ADDR addr, enum target_sig
normal_stop ();
}
}
-
-/* Record the pc of the program the last time it stopped. This is
- just used internally by wait_for_inferior, but need to be preserved
- over calls to it and cleared when the inferior is started. */
-static CORE_ADDR prev_pc;

/* Start remote-debugging of a machine over a serial link. */



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]