[master/7.12] Throw error when ptrace fail in regsets_fetch_inferior_registers

Message ID 1470230379-6790-1-git-send-email-yao.qi@linaro.org
State New, archived
Headers

Commit Message

Yao Qi Aug. 3, 2016, 1:19 p.m. UTC
  When I run process-dies-while-detaching.exp with GDBserver, I see many
warnings printed by GDBserver,

ptrace(regsets_fetch_inferior_registers) PID=26183: No such process
ptrace(regsets_fetch_inferior_registers) PID=26183: No such process
ptrace(regsets_fetch_inferior_registers) PID=26184: No such process
ptrace(regsets_fetch_inferior_registers) PID=26184: No such process

regsets_fetch_inferior_registers is called when GDBserver resumes each
lwp.

 #2  0x0000000000428260 in regsets_fetch_inferior_registers (regsets_info=0x4690d0 <aarch64_regsets_info>, regcache=0x31832020)
    at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-low.c:5412
 #3  0x00000000004070e8 in get_thread_regcache (thread=0x31832940, fetch=fetch@entry=1) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/regcache.c:58
 #4  0x0000000000429c40 in linux_resume_one_lwp_throw (info=<optimized out>, signal=0, step=0, lwp=0x31832830)
    at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-low.c:4463
 #5  linux_resume_one_lwp (lwp=0x31832830, step=<optimized out>, signal=<optimized out>, info=<optimized out>)
    at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-low.c:4573

The is the case that threads are disappeared when GDB/GDBserver resumes
them.  Our fix nowadays is to throw exception, caller
linux_resume_one_lwp catches the exception, and swallow it if the lwp
is gone.  See linux-low.c:linux_resume_one_lwp.  So this patch fixes
the problem by throwing exception in regsets_fetch_inferior_registers.
Another caller of regsets_fetch_inferior_registers, linux_fetch_registers,
needs to catch the exception the same way as linux_resume_one_lwp does.

gdb/gdbserver:

2016-08-02  Yao Qi  <yao.qi@linaro.org>

	* linux-low.c (regsets_fetch_inferior_registers): Throw
	exception on ptrace fail.
	(linux_fetch_registers): Rename to ...
	(linux_fetch_registers_throw): ... it.
	(linux_fetch_registers): Call linux_fetch_registers_throw
	and catch the exception.  If check_ptrace_stopped_lwp_gone
	is false, throw it again.
---
 gdb/gdbserver/linux-low.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)
  

Comments

Pedro Alves Aug. 3, 2016, 2:21 p.m. UTC | #1
On 08/03/2016 02:19 PM, Yao Qi wrote:
> When I run process-dies-while-detaching.exp with GDBserver, I see many
> warnings printed by GDBserver,
> 
> ptrace(regsets_fetch_inferior_registers) PID=26183: No such process
> ptrace(regsets_fetch_inferior_registers) PID=26183: No such process
> ptrace(regsets_fetch_inferior_registers) PID=26184: No such process
> ptrace(regsets_fetch_inferior_registers) PID=26184: No such process
> 
> regsets_fetch_inferior_registers is called when GDBserver resumes each
> lwp.
> 
>  #2  0x0000000000428260 in regsets_fetch_inferior_registers (regsets_info=0x4690d0 <aarch64_regsets_info>, regcache=0x31832020)
>     at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-low.c:5412
>  #3  0x00000000004070e8 in get_thread_regcache (thread=0x31832940, fetch=fetch@entry=1) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/regcache.c:58
>  #4  0x0000000000429c40 in linux_resume_one_lwp_throw (info=<optimized out>, signal=0, step=0, lwp=0x31832830)
>     at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-low.c:4463
>  #5  linux_resume_one_lwp (lwp=0x31832830, step=<optimized out>, signal=<optimized out>, info=<optimized out>)
>     at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-low.c:4573
> 
> The is the case that threads are disappeared when GDB/GDBserver resumes
> them.  Our fix nowadays is to throw exception, caller
> linux_resume_one_lwp catches the exception, and swallow it if the lwp
> is gone.  See linux-low.c:linux_resume_one_lwp.  So this patch fixes
> the problem by throwing exception in regsets_fetch_inferior_registers.
> Another caller of regsets_fetch_inferior_registers, linux_fetch_registers,
> needs to catch the exception the same way as linux_resume_one_lwp does.
> 

Throwing is useful when the exception needs to cross several layers.
Originally that was added because the exceptions had to cross through
multiple layers, including inf-ptrace.c and the regcache.

In cases where we have the ptrace errno code at hand, we can simply
check for ESRCH.  That is, in fact what regsets_store_inferior_registers
does.

	  else if (errno == ESRCH)
	    {
	      /* At this point, ESRCH should mean the process is
		 already gone, in which case we simply ignore attempts
		 to change its registers.  See also the related
		 comment in linux_resume_one_lwp.  */
	      free (buf);
	      return 0;
	    }


(store_register checks ESRCH too instead of throwing, as well
as linux_detach_one_lwp and attach_proc_task_lwp_callback.)

However, I don't think swallowing a register/write error is always the
best to do.

If we're resuming the program, then yes, we should swallow the error,
because now that the thread is resumed, we'll collect the exit
status shortly.

But if the thread is _not_ resumed, the user has the now-dead thread
selected, and does "info registers" or "p $somereg = 0xf00" -- then I
think we should report back the error all the way back to the user,
not swallow it and display random garbage for register contents, or
pretend the write succeeded.

So if we go the throw route, I think the catch should be somewhere
higher level, in the code that is reading/writing registers because
it is resuming the thread.  See how linux-nat.c:resume_stopped_resumed_lwps's
TRY block encloses more than one call that might throw.

It may be that in this case the register read is coming from gdb
directly (really no idea), which would mean that it's gdb that
would have to ignore the error, which would complicate things.

But until that is gone, I see no reason to prefer a throw/catch
instead of simply checking for ESRCH, mirroring
regsets_store_inferior_registers?

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index e251ac4..c72b78c 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -5410,7 +5410,7 @@  regsets_fetch_inferior_registers (struct regsets_info *regsets_info,
 	      char s[256];
 	      sprintf (s, "ptrace(regsets_fetch_inferior_registers) PID=%d",
 		       pid);
-	      perror (s);
+	      perror_with_name (_(s));
 	    }
 	}
       else
@@ -5702,7 +5702,7 @@  usr_store_inferior_registers (const struct regs_info *regs_info,
 
 
 static void
-linux_fetch_registers (struct regcache *regcache, int regno)
+linux_fetch_registers_throw (struct regcache *regcache, int regno)
 {
   int use_regsets;
   int all = 0;
@@ -5735,6 +5735,23 @@  linux_fetch_registers (struct regcache *regcache, int regno)
 }
 
 static void
+linux_fetch_registers (struct regcache *regcache, int regno)
+{
+  TRY
+    {
+      linux_fetch_registers_throw (regcache, regno);
+    }
+  CATCH (ex, RETURN_MASK_ERROR)
+    {
+      struct lwp_info *lwp = get_thread_lwp (current_thread);
+
+      if (!check_ptrace_stopped_lwp_gone (lwp))
+	throw_exception (ex);
+    }
+  END_CATCH
+}
+
+static void
 linux_store_registers (struct regcache *regcache, int regno)
 {
   int use_regsets;