* [RFC] Limit attempts to place breakpoints on _start, __start, and main in solib-svr4.c
@ 2010-11-29 23:02 Kevin Buettner
2010-11-30 0:07 ` Jan Kratochvil
0 siblings, 1 reply; 9+ messages in thread
From: Kevin Buettner @ 2010-11-29 23:02 UTC (permalink / raw)
To: gdb-patches
About a month and a half ago, I posted a patch which prevented
enable_break() in solib-svr4.c from placing breakpoints on _start,
__start, or main. See:
http://www.sourceware.org/ml/gdb-patches/2010-09/msg00309.html
Mark Kettenis objected to that patch on the grounds that placing
breakpoints on _start, __start, or main can still be useful. He
agreed, however, that it would be acceptable to skip attempts to
place these breakpoints when attaching to a process.
The patch below preserves the current behavior when a program is
started via GDB's "run" command, but disables attempts to place
breakpoints on _start, __start, and main for all other cases (attach,
follow-fork, core file case, etc.)
Comments?
* infcmd.c (post_create_inferior): Add new parameter,
`from_run_command'. Pass this argument to
`solib_create_inferior_hook'.
(run_command_1): Pass 1 as value for the `from_run_command'
argument in call to post_create_inferior().
(attach_command_post_wait): Pass 0 as value for the
`from_run_command' argument in call to post_create_inferior().
* inferior.h (post_create_inferior): Update declaration to
have an additional argument.
* corelow.c (core_open): Pass 0 as the value of the
`from_run_command' argument in call to post_create_inferior().
* infrun.c (start_remote): Likewise.
* tracepoint.c (tfile_open): Likewise.
* solib.c, solib.h (solib_create_inferior_hook): Add new
parameter, `from_run_command'.
* solist.c (struct target_so_ops): Add new parameter,
`from_run_command', to the `solib_create_inferior_hook' operation.
* solib.c (reload_shared_libraries): Pass 0 as value of the
`from_run_command' argument in call to solib_create_inferior_hook().
* infrun.c (follow_exec): Likewise.
* linux-nat.c (linux_child_follow_fork): Likewise.
* nto-procfs.c (procfs_post_attach, procfs_create_inferior):
Likewise.
* solib-darwin.c (darwin_solib_create_inferior_hook): Add
new parameter, `from_run_command'.
* solib-frv.c (frv_solib_create_inferior_hook): Likewise.
* solib-irix.c (irix_solib_create_inferior_hook): Likewise.
* solib-osf.c (osf_solib_create_inferior_hook): Likewise.
* solib-pa64.c (pa64_solib_create_inferior_hook): Likewise.
* solib-som.c (som_solib_create_inferior_hook): Likewise.
* solib-spu.c (spu_solib_create_inferior_hook): Likewise.
* solib-sunos.c (sunos_solib_create_inferior_hook): Likewise.
* solib-target.c (solib_target_solib_create_inferior_hook):
Likewise.
* solib-svr4.c (svr4_solib_create_inferior_hook): Likewise.
Pass `from_run_command' to enable_break().
(enable_break): Add new parameter, `from_run_command'. Don't
attempt to place a breakpoint on the bkpt_names[] symbols
unless `from_run_command' is non-zero.
Index: corelow.c
===================================================================
RCS file: /cvs/src/src/gdb/corelow.c,v
retrieving revision 1.104
diff -u -p -r1.104 corelow.c
--- corelow.c 6 Sep 2010 13:59:02 -0000 1.104
+++ corelow.c 29 Nov 2010 20:54:29 -0000
@@ -415,7 +415,7 @@ core_open (char *filename, int from_tty)
switch_to_thread (thread->ptid);
}
- post_create_inferior (&core_ops, from_tty);
+ post_create_inferior (&core_ops, from_tty, 0);
/* Now go through the target stack looking for threads since there
may be a thread_stratum target loaded on top of target core by
Index: infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.269
diff -u -p -r1.269 infcmd.c
--- infcmd.c 1 Jul 2010 15:36:15 -0000 1.269
+++ infcmd.c 29 Nov 2010 20:54:30 -0000
@@ -390,7 +390,8 @@ strip_bg_char (char **args)
should be stopped. */
void
-post_create_inferior (struct target_ops *target, int from_tty)
+post_create_inferior (struct target_ops *target, int from_tty,
+ int from_run_command)
{
/* Be sure we own the terminal in case write operations are performed. */
target_terminal_ours ();
@@ -411,7 +412,7 @@ post_create_inferior (struct target_ops
#ifdef SOLIB_CREATE_INFERIOR_HOOK
SOLIB_CREATE_INFERIOR_HOOK (PIDGET (inferior_ptid));
#else
- solib_create_inferior_hook (from_tty);
+ solib_create_inferior_hook (from_tty, from_run_command);
#endif
}
@@ -579,7 +580,7 @@ run_command_1 (char *args, int from_tty,
/* Pass zero for FROM_TTY, because at this point the "run" command
has done its thing; now we are setting up the running program. */
- post_create_inferior (¤t_target, 0);
+ post_create_inferior (¤t_target, 0, 1);
/* Start the target running. Do not use -1 continuation as it would skip
breakpoint right at the entry point. */
@@ -2290,7 +2291,7 @@ attach_command_post_wait (char *args, in
/* Take any necessary post-attaching actions for this platform. */
target_post_attach (PIDGET (inferior_ptid));
- post_create_inferior (¤t_target, from_tty);
+ post_create_inferior (¤t_target, from_tty, 0);
/* Install inferior's terminal modes. */
target_terminal_inferior ();
Index: inferior.h
===================================================================
RCS file: /cvs/src/src/gdb/inferior.h,v
retrieving revision 1.145
diff -u -p -r1.145 inferior.h
--- inferior.h 24 Jun 2010 15:17:30 -0000 1.145
+++ inferior.h 29 Nov 2010 20:54:30 -0000
@@ -267,7 +267,7 @@ void set_step_info (struct frame_info *f
/* From infcmd.c */
-extern void post_create_inferior (struct target_ops *, int);
+extern void post_create_inferior (struct target_ops *, int, int);
extern void attach_command (char *, int);
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.457
diff -u -p -r1.457 infrun.c
--- infrun.c 24 Nov 2010 19:08:30 -0000 1.457
+++ infrun.c 29 Nov 2010 20:54:30 -0000
@@ -848,7 +848,7 @@ follow_exec (ptid_t pid, char *execd_pat
#ifdef SOLIB_CREATE_INFERIOR_HOOK
SOLIB_CREATE_INFERIOR_HOOK (PIDGET (inferior_ptid));
#else
- solib_create_inferior_hook (0);
+ solib_create_inferior_hook (0, 0);
#endif
jit_inferior_created_hook ();
@@ -2141,7 +2141,7 @@ start_remote (int from_tty)
/* Now that the inferior has stopped, do any bookkeeping like
loading shared libraries. We want to do this before normal_stop,
so that the displayed frame is up to date. */
- post_create_inferior (¤t_target, from_tty);
+ post_create_inferior (¤t_target, from_tty, 0);
normal_stop ();
}
Index: linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-nat.c,v
retrieving revision 1.186
diff -u -p -r1.186 linux-nat.c
--- linux-nat.c 2 Nov 2010 01:37:32 -0000 1.186
+++ linux-nat.c 29 Nov 2010 20:54:30 -0000
@@ -767,7 +767,7 @@ holding the child stopped. Try \"set de
breakpoint. If a "cloned-VM" event was propagated
better throughout the core, this wouldn't be
required. */
- solib_create_inferior_hook (0);
+ solib_create_inferior_hook (0, 0);
}
/* Let the thread_db layer learn about this new process. */
@@ -949,7 +949,7 @@ Attaching after process %d fork to child
shared libraries, and install the solib event breakpoint.
If a "cloned-VM" event was propagated better throughout
the core, this wouldn't be required. */
- solib_create_inferior_hook (0);
+ solib_create_inferior_hook (0, 0);
}
/* Let the thread_db layer learn about this new process. */
Index: nto-procfs.c
===================================================================
RCS file: /cvs/src/src/gdb/nto-procfs.c,v
retrieving revision 1.54
diff -u -p -r1.54 nto-procfs.c
--- nto-procfs.c 7 Jul 2010 16:15:16 -0000 1.54
+++ nto-procfs.c 29 Nov 2010 20:54:30 -0000
@@ -651,7 +651,7 @@ static void
procfs_post_attach (pid_t pid)
{
if (exec_bfd)
- solib_create_inferior_hook (0);
+ solib_create_inferior_hook (0, 0);
}
static ptid_t
@@ -1212,7 +1212,7 @@ procfs_create_inferior (struct target_op
if (exec_bfd != NULL
|| (symfile_objfile != NULL && symfile_objfile->obfd != NULL))
- solib_create_inferior_hook (0);
+ solib_create_inferior_hook (0, 0);
}
static void
Index: solib-darwin.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-darwin.c,v
retrieving revision 1.14
diff -u -p -r1.14 solib-darwin.c
--- solib-darwin.c 16 May 2010 23:49:58 -0000 1.14
+++ solib-darwin.c 29 Nov 2010 20:54:30 -0000
@@ -296,7 +296,7 @@ darwin_special_symbol_handling (void)
/* Shared library startup support. See documentation in solib-svr4.c */
static void
-darwin_solib_create_inferior_hook (int from_tty)
+darwin_solib_create_inferior_hook (int from_tty, int from_run_command)
{
struct minimal_symbol *msymbol;
char **bkpt_namep;
Index: solib-frv.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-frv.c,v
retrieving revision 1.35
diff -u -p -r1.35 solib-frv.c
--- solib-frv.c 16 May 2010 23:49:58 -0000 1.35
+++ solib-frv.c 29 Nov 2010 20:54:30 -0000
@@ -979,7 +979,7 @@ frv_relocate_main_executable (void)
SYNOPSIS
- void frv_solib_create_inferior_hook ()
+ void frv_solib_create_inferior_hook (int from_tty, int from_run_commnad)
DESCRIPTION
@@ -994,7 +994,7 @@ frv_relocate_main_executable (void)
*/
static void
-frv_solib_create_inferior_hook (int from_tty)
+frv_solib_create_inferior_hook (int from_tty, int from_run_command)
{
/* Relocate main executable. */
frv_relocate_main_executable ();
Index: solib-irix.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-irix.c,v
retrieving revision 1.35
diff -u -p -r1.35 solib-irix.c
--- solib-irix.c 16 May 2010 23:49:58 -0000 1.35
+++ solib-irix.c 29 Nov 2010 20:54:30 -0000
@@ -392,7 +392,7 @@ enable_break (void)
SYNOPSIS
- void solib_create_inferior_hook (int from_tty)
+ void solib_create_inferior_hook (int from_tty, int from_run_command)
DESCRIPTION
@@ -437,7 +437,7 @@ enable_break (void)
*/
static void
-irix_solib_create_inferior_hook (int from_tty)
+irix_solib_create_inferior_hook (int from_tty, int from_run_command)
{
struct inferior *inf;
struct thread_info *tp;
Index: solib-osf.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-osf.c,v
retrieving revision 1.30
diff -u -p -r1.30 solib-osf.c
--- solib-osf.c 16 May 2010 23:49:58 -0000 1.30
+++ solib-osf.c 29 Nov 2010 20:54:30 -0000
@@ -307,7 +307,7 @@ osf_clear_solib (void)
Also, what if child has exit()ed? Must exit loop somehow. */
static void
-osf_solib_create_inferior_hook (int from_tty)
+osf_solib_create_inferior_hook (int from_tty, int from_run_command)
{
struct inferior *inf;
struct thread_info *tp;
Index: solib-pa64.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-pa64.c,v
retrieving revision 1.25
diff -u -p -r1.25 solib-pa64.c
--- solib-pa64.c 16 May 2010 23:49:58 -0000 1.25
+++ solib-pa64.c 29 Nov 2010 20:54:30 -0000
@@ -329,7 +329,7 @@ bfd_lookup_symbol (bfd *abfd, char *symn
with shared libraries mapped shareable. */
static void
-pa64_solib_create_inferior_hook (int from_tty)
+pa64_solib_create_inferior_hook (int from_tty, int from_run_command)
{
struct minimal_symbol *msymbol;
unsigned int dld_flags, status;
Index: solib-som.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-som.c,v
retrieving revision 1.30
diff -u -p -r1.30 solib-som.c
--- solib-som.c 17 Oct 2010 17:45:16 -0000 1.30
+++ solib-som.c 29 Nov 2010 20:54:30 -0000
@@ -183,7 +183,7 @@ struct {
means running until the "_start" is called. */
static void
-som_solib_create_inferior_hook (int from_tty)
+som_solib_create_inferior_hook (int from_tty, int from_run_command)
{
enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch);
struct minimal_symbol *msymbol;
Index: solib-spu.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-spu.c,v
retrieving revision 1.12
diff -u -p -r1.12 solib-spu.c
--- solib-spu.c 25 Jun 2010 22:00:59 -0000 1.12
+++ solib-spu.c 29 Nov 2010 20:54:30 -0000
@@ -464,7 +464,7 @@ ocl_enable_break (struct objfile *objfil
/* Create inferior hook. */
static void
-spu_solib_create_inferior_hook (int from_tty)
+spu_solib_create_inferior_hook (int from_tty, int from_run_command)
{
/* Handle SPE stand-alone executables. */
if (spu_standalone_p ())
@@ -491,7 +491,7 @@ spu_solib_create_inferior_hook (int from
}
/* Call SVR4 hook -- this will re-insert the SVR4 solib breakpoints. */
- svr4_so_ops.solib_create_inferior_hook (from_tty);
+ svr4_so_ops.solib_create_inferior_hook (from_tty, from_run_command);
/* If the inferior is statically linked against libspe, we need to install
our own solib breakpoint right now. Otherwise, it will be installed by
Index: solib-sunos.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-sunos.c,v
retrieving revision 1.39
diff -u -p -r1.39 solib-sunos.c
--- solib-sunos.c 8 Jan 2010 22:52:03 -0000 1.39
+++ solib-sunos.c 29 Nov 2010 20:54:30 -0000
@@ -741,7 +741,7 @@ sunos_special_symbol_handling (void)
*/
static void
-sunos_solib_create_inferior_hook (int from_tty)
+sunos_solib_create_inferior_hook (int from_tty, int from_run_command)
{
struct thread_info *tp;
struct inferior *inf;
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 29 Nov 2010 20:54:30 -0000
@@ -1342,7 +1342,7 @@ exec_entry_point (struct bfd *abfd, stru
*/
static int
-enable_break (struct svr4_info *info, int from_tty)
+enable_break (struct svr4_info *info, int from_tty, int from_run_command)
{
struct minimal_symbol *msymbol;
const char * const *bkpt_namep;
@@ -1607,17 +1607,24 @@ enable_break (struct svr4_info *info, in
}
}
- for (bkpt_namep = bkpt_names; *bkpt_namep != NULL; bkpt_namep++)
+ /* If all of the above have failed, and we're running the program from
+ scratch, try placing a breakpoint on one of the names in the bkpt_names[]
+ list. */
+
+ if (from_run_command)
{
- 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,
- ¤t_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,
+ ¤t_target);
+ create_solib_event_breakpoint (target_gdbarch, sym_addr);
+ return 1;
+ }
}
}
return 0;
@@ -2110,7 +2117,7 @@ svr4_relocate_main_executable (void)
SYNOPSIS
- void svr4_solib_create_inferior_hook (int from_tty)
+ void svr4_solib_create_inferior_hook (int from_tty, int from_run_command)
DESCRIPTION
@@ -2155,7 +2162,7 @@ svr4_relocate_main_executable (void)
*/
static void
-svr4_solib_create_inferior_hook (int from_tty)
+svr4_solib_create_inferior_hook (int from_tty, int from_run_command)
{
#if defined(_SCO_DS)
struct inferior *inf;
@@ -2171,7 +2178,7 @@ svr4_solib_create_inferior_hook (int fro
if (!svr4_have_link_map_offsets ())
return;
- if (!enable_break (info, from_tty))
+ if (!enable_break (info, from_tty, from_run_command))
return;
#if defined(_SCO_DS)
Index: solib-target.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-target.c,v
retrieving revision 1.16
diff -u -p -r1.16 solib-target.c
--- solib-target.c 15 Jun 2010 17:57:38 -0000 1.16
+++ solib-target.c 29 Nov 2010 20:54:30 -0000
@@ -307,7 +307,7 @@ solib_target_special_symbol_handling (vo
}
static void
-solib_target_solib_create_inferior_hook (int from_tty)
+solib_target_solib_create_inferior_hook (int from_tty, int from_run_command)
{
/* Nothing needed. */
}
Index: solib.c
===================================================================
RCS file: /cvs/src/src/gdb/solib.c,v
retrieving revision 1.141
diff -u -p -r1.141 solib.c
--- solib.c 5 Nov 2010 01:40:28 -0000 1.141
+++ solib.c 29 Nov 2010 20:54:30 -0000
@@ -1217,21 +1217,22 @@ clear_solib (void)
SYNOPSIS
- void solib_create_inferior_hook (int from_tty)
+ void solib_create_inferior_hook (int from_tty, int from_run_command)
DESCRIPTION
When gdb starts up the inferior, it nurses it along (through the
- shell) until it is ready to execute it's first instruction. At this
- point, this function gets called via expansion of the macro
- SOLIB_CREATE_INFERIOR_HOOK. */
+ shell) until it is ready to execute it's first instruction.
+
+ The flag FROM_RUN_COMMAND will be 1 if this function is invoked due
+ to the running of a new program; otherwise it is 0. */
void
-solib_create_inferior_hook (int from_tty)
+solib_create_inferior_hook (int from_tty, int from_run_command)
{
struct target_so_ops *ops = solib_ops (target_gdbarch);
- ops->solib_create_inferior_hook (from_tty);
+ ops->solib_create_inferior_hook (from_tty, from_run_command);
}
/* GLOBAL FUNCTION
@@ -1396,7 +1397,7 @@ reload_shared_libraries (char *ignored,
#ifdef SOLIB_CREATE_INFERIOR_HOOK
SOLIB_CREATE_INFERIOR_HOOK (PIDGET (inferior_ptid));
#else
- solib_create_inferior_hook (from_tty);
+ solib_create_inferior_hook (from_tty, 0);
#endif
}
Index: solib.h
===================================================================
RCS file: /cvs/src/src/gdb/solib.h,v
retrieving revision 1.28
diff -u -p -r1.28 solib.h
--- solib.h 8 Jan 2010 22:52:04 -0000 1.28
+++ solib.h 29 Nov 2010 20:54:30 -0000
@@ -42,7 +42,7 @@ extern int solib_read_symbols (struct so
addresses to which they are linked, and sufficient information to
read in their symbols at a later time. */
-extern void solib_create_inferior_hook (int from_tty);
+extern void solib_create_inferior_hook (int from_tty, int from_run_command);
/* If ADDR lies in a shared library, return its name. */
Index: solist.h
===================================================================
RCS file: /cvs/src/src/gdb/solist.h,v
retrieving revision 1.32
diff -u -p -r1.32 solist.h
--- solist.h 22 Apr 2010 23:15:41 -0000 1.32
+++ solist.h 29 Nov 2010 20:54:30 -0000
@@ -94,7 +94,7 @@ struct target_so_ops
void (*clear_solib) (void);
/* Target dependent code to run after child process fork. */
- void (*solib_create_inferior_hook) (int from_tty);
+ void (*solib_create_inferior_hook) (int from_tty, int from_run_command);
/* Do additional symbol handling, lookup, etc. after symbols
for a shared object have been loaded. */
Index: tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.c,v
retrieving revision 1.197
diff -u -p -r1.197 tracepoint.c
--- tracepoint.c 1 Nov 2010 07:00:13 -0000 1.197
+++ tracepoint.c 29 Nov 2010 20:54:31 -0000
@@ -3312,7 +3312,7 @@ tfile_open (char *filename, int from_tty
inferior_ptid = pid_to_ptid (TFILE_PID);
add_thread_silent (inferior_ptid);
- post_create_inferior (&tfile_ops, from_tty);
+ post_create_inferior (&tfile_ops, from_tty, 0);
#if 0
/* FIXME this will get defined in MI patch submission */
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [RFC] Limit attempts to place breakpoints on _start, __start, and main in solib-svr4.c
2010-11-29 23:02 [RFC] Limit attempts to place breakpoints on _start, __start, and main in solib-svr4.c Kevin Buettner
@ 2010-11-30 0:07 ` Jan Kratochvil
2010-11-30 5:15 ` Kevin Buettner
2010-12-01 1:06 ` Kevin Buettner
0 siblings, 2 replies; 9+ messages in thread
From: Jan Kratochvil @ 2010-11-30 0:07 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
Hello Kevin,
On Tue, 30 Nov 2010 00:02:33 +0100, Kevin Buettner wrote:
> -enable_break (struct svr4_info *info, int from_tty)
> +enable_break (struct svr4_info *info, int from_tty, int from_run_command)
I haven't tried it myself but is there a reason why not to use
`struct inferior->attach_flag' instead?
Thanks,
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC] Limit attempts to place breakpoints on _start, __start, and main in solib-svr4.c
2010-11-30 0:07 ` Jan Kratochvil
@ 2010-11-30 5:15 ` Kevin Buettner
2010-12-01 1:06 ` Kevin Buettner
1 sibling, 0 replies; 9+ messages in thread
From: Kevin Buettner @ 2010-11-30 5:15 UTC (permalink / raw)
To: gdb-patches
Hi Jan,
On Tue, 30 Nov 2010 01:07:07 +0100
Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
> I haven't tried it myself but is there a reason why not to use
> `struct inferior->attach_flag' instead?
I'll look into using `attach_flag' and let you know...
Thanks!
Kevin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC] Limit attempts to place breakpoints on _start, __start, and main in solib-svr4.c
2010-11-30 0:07 ` Jan Kratochvil
2010-11-30 5:15 ` Kevin Buettner
@ 2010-12-01 1:06 ` Kevin Buettner
2010-12-01 23:03 ` Jan Kratochvil
1 sibling, 1 reply; 9+ messages in thread
From: Kevin Buettner @ 2010-12-01 1:06 UTC (permalink / raw)
To: gdb-patches; +Cc: Jan Kratochvil
On Tue, 30 Nov 2010 01:07:07 +0100
Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
> > -enable_break (struct svr4_info *info, int from_tty)
> > +enable_break (struct svr4_info *info, int from_tty, int from_run_command)
>
> I haven't tried it myself but is there a reason why not to use
> `struct inferior->attach_flag' instead?
Hi Jan,
This appeared to be a very promising suggestion. One of the things
that I find unfortunate about my patch is that it touches more code
than I'd like. Therefore, I was excited when you suggested the use
of `attach_flag' because it'd reduce that multi-page patch that I
posted to just a few lines.
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". A cursory glance at remote.c shows that `attach_flag' is set
appropriately in several places, but a somewhat deeper analysis
reveals that post_create_inferior() (which, for svr4 shared libs,
eventually calls enable_break()) is called well in advance of the
code which sets `attach_flag'. Bummer.
The patch that I've posted does correctly handle the "target remote"
case. (It correctly handles the native cases too.)
Kevin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC] Limit attempts to place breakpoints on _start, __start, and main in solib-svr4.c
2010-12-01 1:06 ` Kevin Buettner
@ 2010-12-01 23:03 ` Jan Kratochvil
2010-12-03 21:04 ` Kevin Buettner
0 siblings, 1 reply; 9+ messages in thread
From: Jan Kratochvil @ 2010-12-01 23:03 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
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
Maybe the single-hit variant would be enough?
> The patch that I've posted does correctly handle the "target remote"
> case. (It correctly handles the native cases too.)
(The problem is it introduces two variants of "attach_flag".)
Thanks,
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC] Limit attempts to place breakpoints on _start, __start, and main in solib-svr4.c
2010-12-01 23:03 ` Jan Kratochvil
@ 2010-12-03 21:04 ` Kevin Buettner
2010-12-05 19:47 ` Mark Kettenis
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Kevin Buettner @ 2010-12-03 21:04 UTC (permalink / raw)
To: gdb-patches
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,
- ¤t_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,
+ ¤t_target);
+ create_solib_event_breakpoint (target_gdbarch, sym_addr);
+ return 1;
+ }
}
}
return 0;
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [RFC] Limit attempts to place breakpoints on _start, __start, and main in solib-svr4.c
2010-12-03 21:04 ` Kevin Buettner
@ 2010-12-05 19:47 ` Mark Kettenis
2010-12-07 7:41 ` Jan Kratochvil
2010-12-13 15:47 ` Kevin Buettner
2 siblings, 0 replies; 9+ messages in thread
From: Mark Kettenis @ 2010-12-05 19:47 UTC (permalink / raw)
To: kevinb; +Cc: gdb-patches
> Date: Fri, 3 Dec 2010 14:04:33 -0700
> From: Kevin Buettner <kevinb@redhat.com>
>
> On Thu, 2 Dec 2010 00:03:39 +0100
> Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
>
> 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?
Makes sense to me.
> * solib-svr4.c (enable_break): Don't attempt to place breakpoints,
> when attaching, on the names in bkpt_names: _start, __start, and
> main.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC] Limit attempts to place breakpoints on _start, __start, and main in solib-svr4.c
2010-12-03 21:04 ` Kevin Buettner
2010-12-05 19:47 ` Mark Kettenis
@ 2010-12-07 7:41 ` Jan Kratochvil
2010-12-13 15:47 ` Kevin Buettner
2 siblings, 0 replies; 9+ messages in thread
From: Jan Kratochvil @ 2010-12-07 7:41 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
Hi Kevin,
On Fri, 03 Dec 2010 22:04:33 +0100, Kevin Buettner wrote:
> 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.
if this functionality change is enough for now this patch looks safe to me.
> 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.
Therefore it also falls back to the current/safe mode in such case.
> Further comments?
> * solib-svr4.c (enable_break): Don't attempt to place breakpoints,
> when attaching, on the names in bkpt_names: _start, __start, and
> main.
As Mark also agrees with this patch I find it OK for the check-in.
Thanks,
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC] Limit attempts to place breakpoints on _start, __start, and main in solib-svr4.c
2010-12-03 21:04 ` Kevin Buettner
2010-12-05 19:47 ` Mark Kettenis
2010-12-07 7:41 ` Jan Kratochvil
@ 2010-12-13 15:47 ` Kevin Buettner
2 siblings, 0 replies; 9+ messages in thread
From: Kevin Buettner @ 2010-12-13 15:47 UTC (permalink / raw)
To: gdb-patches
On Fri, 3 Dec 2010 14:04:33 -0700
Kevin Buettner <kevinb@redhat.com> wrote:
> * solib-svr4.c (enable_break): Don't attempt to place breakpoints,
> when attaching, on the names in bkpt_names: _start, __start, and
> main.
Committed.
Kevin
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-12-13 15:47 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-29 23:02 [RFC] Limit attempts to place breakpoints on _start, __start, and main in solib-svr4.c Kevin Buettner
2010-11-30 0:07 ` Jan Kratochvil
2010-11-30 5:15 ` Kevin Buettner
2010-12-01 1:06 ` Kevin Buettner
2010-12-01 23:03 ` Jan Kratochvil
2010-12-03 21:04 ` Kevin Buettner
2010-12-05 19:47 ` Mark Kettenis
2010-12-07 7:41 ` Jan Kratochvil
2010-12-13 15:47 ` Kevin Buettner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox