This is the mail archive of the gdb-patches@sourceware.org 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]

Patch for 'next' fails to set internal bp in non-stop


Hi,

This is in relation to the issue I reported in
http://sourceware.org/ml/gdb/2011-04/msg00060.html
where doing a 'next' would fail to set its internal breakpoint
in non-stop mode.

I debugged the issue further and I have a fix to propose below.
Here is what I think is happening.

During the 'next' operation, GDB tries to set a bunch of breakpoints
including an internal one for the 'next' itself.  This is done
in insert_breakpoint_locations() from breakpoint.c

In GDB 7.0, where the problem does _not_ occur, when setting
all these breakpoints, GDB has its focus on the thread that is being 
stepped, and that thread is of course not running so ptrace can access it.

Starting with GDB 7.1, where multi-exec is supported, when GDB tries
to set breakpoints, it needs to change its focus to a thread of the right
program space for each breakpoint.  When it is time to set the internal
breakpoint for the 'next' operation, GDB selects a thread using
any_live_thread_of_process() from thread.c.  This method looks for
any thread that is in the state THREAD_STOPPED if available.  The problem 
is that because of the 'next' operation, the thread GDB is stepping has 
already changed state to THREAD_RUNNING.  If there are no other thread
STOPPED, then _any_ RUNNING thread is returned, and unless it happens
to be the thread that is being stepped, the ptrace operation will fail with

(gdb) Warning:
Cannot insert breakpoint 0.   <------ Notice #0 indicating internal breakpoint
Error accessing memory address 0x80484ef: Input/output error.

What I propose is that GDB should try to return the thread that is being
stepped since it is not actually executing.  I found that GDB has the
knowledge that a thread is RUNNING but not executing.  The fix below
changes  any_live_thread_of_process() to first prioritize a STOPPED
thread (like before), but then to prioritize a thread that has its executing_ 
flag set to false and only after that to fall back on an executing_ thread.

This change seems pretty safe to me since it returns a thread that the
method could have returned before, it we were lucky; it is just that now,
we make sure it always does.

This fixes the test scenario that I described in
http://sourceware.org/ml/gdb/2011-04/msg00060.html
which showed how to reproduce the problem.

No regressions on HEAD.

I find this problem to be pretty serious for people using non-stop
and if the fix is approved, I would like to put it in HEAD, 7.3 and 7.2
(is 7.2.1 still going to be released?)

OK?

Thanks


2011-04-17  Marc Khouzam  <marc.khouzam@ericsson.com>

       * thread.c (any_live_thread_of_process): Prioritize a thread
       that is not executing above a thread that is RUNNING.


### Eclipse Workspace Patch 1.0
#P src
Index: gdb/thread.c
===================================================================
RCS file: /cvs/src/src/gdb/thread.c,v
retrieving revision 1.138
diff -u -r1.138 thread.c
--- gdb/thread.c        10 Mar 2011 18:33:59 -0000      1.138
+++ gdb/thread.c        17 Apr 2011 12:40:15 -0000
@@ -471,6 +471,7 @@
 {
   struct thread_info *tp;
   struct thread_info *tp_running = NULL;
+  struct thread_info *tp_not_executing = NULL;
 
   for (tp = thread_list; tp; tp = tp->next)
     if (ptid_get_pid (tp->ptid) == pid)
@@ -478,8 +479,15 @@
        if (tp->state_ == THREAD_STOPPED)
          return tp;
        else if (tp->state_ == THREAD_RUNNING)
-         tp_running = tp;
+         {
+           if (tp->executing_)
+             tp_running = tp;
+           else
+             tp_not_executing = tp;
+         }
       }
+  if (tp_not_executing)
+    return tp_not_executing;
 
   return tp_running;
 }


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