Re: [RFC] Limit attempts to place breakpoints on _start, __start, and main in solib-svr4.c

[Kevin Buettner <kevinb at redhat dot com>]

On Thu, 2 Dec 2010 00:03:39 +0100
Jan Kratochvil <jan.kratochvil@redhat.com> wrote:

> On Wed, 01 Dec 2010 02:06:18 +0100, Kevin Buettner wrote:
> > My testing shows that use of `! current_inferior ()->attach_flag' as
> > the test in enable_break() works when attaching to a process started
> > natively.  I also see the correct behavior (in which a breakpoint is
> > placed on _start, et al) when the process is started via GDB's
> > "run" command.
> > 
> > The case that doesn't work - and, unfortunately, it's the case that
> > really matters to me - is connecting to a remote target via "target
> > remote".
> 
> GNU gdb (GDB) 7.2.50.20101201-cvs
> 
> killall -9 gdbserver;sleep 1h&p=$!;sleep 0.1;./gdbserver/gdbserver :1234 ./pause& ./gdb -nx -ex 'file ./pause' -ex 'target remote localhost:1234' 
> attach_flag=0
>  - correct
> 
> killall -9 gdbserver;./pause&p=$!;sleep 0.1;./gdbserver/gdbserver --attach :1234 $p& ./gdb -nx -ex 'file ./pause' -ex 'target remote localhost:1234' 
> attach_flag=1
>  - correct
> 
> killall -9 gdbserver;./pause&p=$!;sleep 0.1;./gdbserver/gdbserver --multi :1234& ./gdb -nx -ex 'file ./pause' -ex 'target extended-remote localhost:1234' -ex "attach $p"
> attach_flag=1
>  - correct
> 
> Probably in the first case you do not want to place those breakpoints?
> 
> But I think at least the "main" breakpoint is the one requested by Mark
> Kettenis to stay there even in the case solib_break_names[] bpts fail:
> 	http://sourceware.org/ml/gdb-patches/2010-09/msg00313.html

Hi Jan,

That's a nice demonstration of GDB's behavior when using gdbserver.
My earlier testing and analysis was flawed.  (FWIW, I was testing
only case 1.)

We do want to (potentially) attempt to place the breakpoints on
_start, __start, and main in case 1, but not for case 2 and 3.  You
have demonstrated that the patch appended below will work correctly
for gdbserver.  I have used variants of your examples to verify that
this is the case.  (I have a hacked static executable in which I
see a breakpoint placed on _start in case 1, but not for case 2 and 3 -
which is as it should be.)

The patch below won't necessarily work correctly with stubs which do
not implement the qAttached packet.  (gdbserver does implement
qAttached.)  My reading of remote.c indicates that stubs which do
not implement "qAttached" will end up causing attach_flag to be 0. 
This may or may not be what we want depending upon the situation.  It
is certainly not what we want when connecting to a kernel stub (intended
to debug a running kernel).  On the other hand, there may be other
stubs where this is in fact the correct answer.  I'm going to
recommend that my colleague (who pointed me at this problem a while
ago) implement qAttached in his kernel stubs.

I hereby withdraw my earlier patch in favor of the one below.

Further comments?

Kevin

	* solib-svr4.c (enable_break): Don't attempt to place breakpoints,
	when attaching, on the names in bkpt_names: _start, __start, and
	main.

Index: solib-svr4.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-svr4.c,v
retrieving revision 1.139
diff -u -p -r1.139 solib-svr4.c
--- solib-svr4.c	11 Oct 2010 08:50:33 -0000	1.139
+++ solib-svr4.c	3 Dec 2010 20:30:35 -0000
@@ -1607,17 +1607,20 @@ enable_break (struct svr4_info *info, in
 	}
     }
 
-  for (bkpt_namep = bkpt_names; *bkpt_namep != NULL; bkpt_namep++)
+  if (!current_inferior ()->attach_flag)
     {
-      msymbol = lookup_minimal_symbol (*bkpt_namep, NULL, symfile_objfile);
-      if ((msymbol != NULL) && (SYMBOL_VALUE_ADDRESS (msymbol) != 0))
+      for (bkpt_namep = bkpt_names; *bkpt_namep != NULL; bkpt_namep++)
 	{
-	  sym_addr = SYMBOL_VALUE_ADDRESS (msymbol);
-	  sym_addr = gdbarch_convert_from_func_ptr_addr (target_gdbarch,
-							 sym_addr,
-							 &current_target);
-	  create_solib_event_breakpoint (target_gdbarch, sym_addr);
-	  return 1;
+	  msymbol = lookup_minimal_symbol (*bkpt_namep, NULL, symfile_objfile);
+	  if ((msymbol != NULL) && (SYMBOL_VALUE_ADDRESS (msymbol) != 0))
+	    {
+	      sym_addr = SYMBOL_VALUE_ADDRESS (msymbol);
+	      sym_addr = gdbarch_convert_from_func_ptr_addr (target_gdbarch,
+							     sym_addr,
+							     &current_target);
+	      create_solib_event_breakpoint (target_gdbarch, sym_addr);
+	      return 1;
+	    }
 	}
     }
   return 0;