[catch-syscall] [patch] Drop entry_breakpoint at all

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi SÃrgio,

checked why there exists entry_breakpoint at all:
breakpoint:
   This is currently needed for us to know when to start setting
   up catchpoints for syscalls in the inferior.  If we don't do that,
   then we would set a "catch syscall" too early, which would
   catch syscalls from ld.so and/or libc (and we don't want that).

I do not agree with this intention as I find perfectly valid to `catch
syscall's for debugging of the ld.so startup.  Even `set debug infrun 1' etc.
applies to this pre-main startup phase.  User is free and suggested to type
`start' before typing `catch syscall'.

This is the first patch, to drop this new breakpoint entry_breakpoint.

The second part removes target_enable_tracesysgood() and makes it just
a GNU/Linux-specific feature hooking the linux_enable_tracesysgood() call
formerly at entry_breakpoint to be at the common first possible attach/start
point of the inferior.

My reasons for Linuxizing target_enable_tracesysgood():
(a1) Already the name suggests it is based on PTRACE_O_TRACESYSGOOD which is
     a Linux-specific issue.  Other OSes would have a different name and even
     different ways how to trace the syscalls.
(a2) It makes it easy to hook the linux_enable_tracesysgood() call at the right
     place without some function wrappers (due to GDB missing OO framework).
(a3) So far only GNU/Linux implementation exists, implementors for other OSes
     would generalize the infrastructure appropriately themselves.

In fact I also find target_set_syscall_catchpoint() more Linux-specific as
other OSes may not have syscall numbers etc. and moreover (3) above.


Please update the branch against master, it is getting out of sync.

It fixes the regression in archer-rmoseley-demo-merge-2.  I understand
entry_breakpoint could be fixed properly somehow else but:
(b1) Before the removal bpstat_what() I find most easy to not to touch it.
(b2) I do not find useful the basic intention for creating entry_breakpoint
     (skip the pre-main phase) in the first place.


Regression tested on Fedora 10 x86_64-unknown-linux-gnu.


Regards,
Jan

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 09b3171..8b2e70b 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2309,14 +2309,6 @@ print_it_typical (bpstat bs)
       return PRINT_NOTHING;
       break;
 
-    case bp_entry_breakpoint:
-       /* Not sure how we will get here. 
-	 GDB should not stop for these breakpoints.  */
-      internal_error (__FILE__, __LINE__,
-                      _("Entry Breakpoint: gdb should not stop!\n"));
-      return PRINT_NOTHING;
-      break;    
-
     case bp_overlay_event:
       /* By analogy with the thread event, GDB should not stop for these. */
       printf_filtered (_("Overlay Event Breakpoint: gdb should not stop!\n"));
@@ -3121,10 +3113,6 @@ bpstat_what (bpstat bs)
       /* We hit the shared library event breakpoint.  */
       shlib_event,
 
-      /* We are in a entry breakpoint.  It means we have stopped
-         at the inferior's entrypoint (AT_ENTRY).  */
-      entry_breakpoint,
-
       /* This is just used to count how many enums there are.  */
       class_last
     };
@@ -3140,7 +3128,6 @@ bpstat_what (bpstat bs)
 #define clr BPSTAT_WHAT_CLEAR_LONGJMP_RESUME
 #define sr BPSTAT_WHAT_STEP_RESUME
 #define shl BPSTAT_WHAT_CHECK_SHLIBS
-#define entrybp BPSTAT_WHAT_ENTRY_BREAKPOINT
 
 /* "Can't happen."  Might want to print an error message.
    abort() is not out of the question, but chances are GDB is just
@@ -3187,28 +3174,25 @@ bpstat_what (bpstat bs)
   /*       kc    ss    sn    sgl    slr   clr   sr   shl
    */
 /*no_effect */
-    {kc, ss, sn, sgl, slr, clr, sr, shl, kc},
+    {kc, ss, sn, sgl, slr, clr, sr, shl},
 /*wp_silent */
-    {ss, ss, sn, ss, ss, ss, sr, shl, ss},
+    {ss, ss, sn, ss, ss, ss, sr, shl},
 /*wp_noisy */
-    {sn, sn, sn, sn, sn, sn, sr, shl, sn},
+    {sn, sn, sn, sn, sn, sn, sr, shl},
 /*bp_nostop */
-    {sgl, ss, sn, sgl, slr, slr, sr, shl, sgl},
+    {sgl, ss, sn, sgl, slr, slr, sr, shl},
 /*bp_silent */
-    {ss, ss, sn, ss, ss, ss, sr, shl, ss},
+    {ss, ss, sn, ss, ss, ss, sr, shl},
 /*bp_noisy */
-    {sn, sn, sn, sn, sn, sn, sr, shl, sn},
+    {sn, sn, sn, sn, sn, sn, sr, shl},
 /*long_jump */
-    {slr, ss, sn, slr, slr, err, sr, shl, slr},
+    {slr, ss, sn, slr, slr, err, sr, shl},
 /*long_resume */
-    {clr, ss, sn, err, err, err, sr, shl, clr},
+    {clr, ss, sn, err, err, err, sr, shl},
 /*step_resume */
-    {sr, sr, sr, sr, sr, sr, sr, sr, sr},
+    {sr, sr, sr, sr, sr, sr, sr, sr},
 /*shlib */
-    {shl, shl, shl, shl, shl, shl, sr, shl, shl},
-/* entry_breakpoint */
-    {entrybp, entrybp, entrybp, entrybp, entrybp, entrybp, sr, entrybp,
-      entrybp}
+    {shl, shl, shl, shl, shl, shl, sr, shl}
   };
 
 #undef kc
@@ -3221,7 +3205,6 @@ bpstat_what (bpstat bs)
 #undef sr
 #undef ts
 #undef shl
-#undef entrybp
   enum bpstat_what_main_action current_action = BPSTAT_WHAT_KEEP_CHECKING;
   struct bpstat_what retval;
 
@@ -3315,12 +3298,6 @@ bpstat_what (bpstat bs)
 	  bs_class = bp_silent;
 	  retval.call_dummy = 1;
 	  break;
-        case bp_entry_breakpoint:
-          if (bs->stop)
-            bs_class = entry_breakpoint;
-          else
-            bs_class = no_effect;
-          break;
 	}
       current_action = table[(int) bs_class][(int) current_action];
     }
@@ -3420,7 +3397,6 @@ print_one_breakpoint_location (struct breakpoint *b,
     {bp_thread_event, "thread events"},
     {bp_overlay_event, "overlay events"},
     {bp_catchpoint, "catchpoint"},
-    {bp_entry_breakpoint, "entry breakpoint"}
   };
   
   static char bpenables[] = "nynny";
@@ -3548,7 +3524,6 @@ print_one_breakpoint_location (struct breakpoint *b,
       case bp_shlib_event:
       case bp_thread_event:
       case bp_overlay_event:
-      case bp_entry_breakpoint:
 	if (opts.addressprint)
 	  {
 	    annotate_field (4);
@@ -4120,7 +4095,6 @@ allocate_bp_location (struct breakpoint *bpt)
     case bp_watchpoint_scope:
     case bp_call_dummy:
     case bp_shlib_event:
-    case bp_entry_breakpoint:
     case bp_thread_event:
     case bp_overlay_event:
       loc->loc_type = bp_loc_software_breakpoint;
@@ -4402,47 +4376,6 @@ disable_overlay_breakpoints (void)
     }
 }
 
-int
-create_entry_breakpoint (void)
-{
-  CORE_ADDR taddr, entry_addr;
-  struct breakpoint *b;
-
-  delete_entry_breakpoint ();
-
-  taddr = entry_point_address ();
-  /* Make certain that the address points at real code, and not a
-     function descriptor.  */
-  entry_addr = gdbarch_convert_from_func_ptr_addr (current_gdbarch, taddr,
-                                                   &current_target);
-
-  /* Setting the breakpoint.  */
-  b = create_internal_breakpoint (entry_addr, bp_entry_breakpoint);
-
-  b->enable_state = bp_enabled;
-  b->disposition = disp_del;
-  /* addr_string has to be used or breakpoint_re_set will delete me.  */
-  b->addr_string = xstrprintf ("AT_ENTRY (0x%s)", paddr (entry_addr));
-
-  update_global_location_list (1);
-
-  return 1;
-}
-
-/* Remove the breakpoint from create_entry_breakpoint.  To be called in the
-   same moments as remove_thread_event_breakpoints (or its caller
-   disable_thread_event_reporting) are being called.  */
-
-void
-delete_entry_breakpoint (void)
-{
-  struct breakpoint *b, *temp;
-
-  ALL_BREAKPOINTS_SAFE (b, temp)
-    if (b->type == bp_entry_breakpoint)
-      delete_breakpoint (b);
-}
-
 struct breakpoint *
 create_thread_event_breakpoint (CORE_ADDR address)
 {
@@ -5330,7 +5263,6 @@ mention (struct breakpoint *b)
       case bp_shlib_event:
       case bp_thread_event:
       case bp_overlay_event:
-      case bp_entry_breakpoint:
 	break;
       }
 
@@ -7638,7 +7570,6 @@ delete_command (char *arg, int from_tty)
 	    b->type != bp_shlib_event &&
 	    b->type != bp_thread_event &&
 	    b->type != bp_overlay_event &&
-	    b->type != bp_entry_breakpoint &&
 	    b->number >= 0)
 	  {
 	    breaks_to_delete = 1;
@@ -7656,7 +7587,6 @@ delete_command (char *arg, int from_tty)
 		b->type != bp_shlib_event &&
 		b->type != bp_thread_event &&
 		b->type != bp_overlay_event &&
-		b->type != bp_entry_breakpoint &&
 		b->number >= 0)
 	      delete_breakpoint (b);
 	  }
@@ -7963,9 +7893,6 @@ breakpoint_re_set_one (void *bint)
 	 Once it is set up, we do not want to touch it.  */
     case bp_thread_event:
 
-      /* Same for this one */
-    case bp_entry_breakpoint:
-
       /* Keep temporary breakpoints, which can be encountered when we step
          over a dlopen call and SOLIB_ADD is resetting the breakpoints.
          Otherwise these should have been blown away via the cleanup chain
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 4ef3c78..d176177 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -112,10 +112,6 @@ enum bptype
     bp_overlay_event, 
 
     bp_catchpoint,
-
-    /* This type is used to signal an internal breakpoint located at
-       the AT_ENTRY address.  */
-    bp_entry_breakpoint,
   };
 
 /* States of enablement of breakpoint. */
@@ -549,10 +545,6 @@ enum bpstat_what_main_action
        keep checking.  */
     BPSTAT_WHAT_CHECK_SHLIBS,
 
-    /* This internal breakpoint is used syscall catchpoints only after the
-       shell and the dynamic linker have already ran.  */
-    BPSTAT_WHAT_ENTRY_BREAKPOINT,
-
     /* This is just used to keep track of how many enums there are.  */
     BPSTAT_WHAT_LAST
   };
@@ -901,21 +893,6 @@ extern int catch_syscall_enabled (void);
    Returns 0 if not, greater than 0 if we are.  */
 extern int catching_syscall_number (int syscall_number);
 
-/* Function used to set an internal breakpoint at the AT_ENTRY
-   (a.k.a. the entry point of the inferior).
-
-   This is currently needed for us to know when to start setting
-   up catchpoints for syscalls in the inferior.  If we don't do that,
-   then we would set a "catch syscall" too early, which would
-   catch syscalls from ld.so and/or libc (and we don't want that).
-
-   Returns zero if there was an error setting this breakpoint,
-   or 1 if everything went OK.  */
-extern int create_entry_breakpoint (void);
-
-/* Remove the breakpoint from create_entry_breakpoint.  */
-extern void delete_entry_breakpoint (void);
-
 /* Tell a breakpoint to be quiet.  */
 extern void make_breakpoint_silent (struct breakpoint *);
 
diff --git a/gdb/infrun.c b/gdb/infrun.c
index eca040c..9944234 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3263,16 +3263,6 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (!gdbarch_get_longjmp_target)\n");
 	}
 	break;
 
-      case BPSTAT_WHAT_ENTRY_BREAKPOINT:
-        /* We hit the AT_ENTRY breakpoint, and now we have to enable
-           the PTRACE_O_TRACESYSGOOD option in the inferior *if* we
-           are catching syscalls.  */
-        if (debug_infrun)
-          fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_ENTRY_BREAKPOINT\n");
-        target_enable_tracesysgood (ecs->ptid);
-        ecs->event_thread->stepping_over_breakpoint = 1;
-        break;
-
       case BPSTAT_WHAT_LAST:
 	/* Not a real code, but listed here to shut up gcc -Wall.  */
 
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 20a85d1..c5c20f2 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -757,12 +758,6 @@ linux_child_post_startup_inferior (ptid_t ptid)
 {
   linux_enable_event_reporting (ptid);
   check_for_thread_db ();
-  /* We have to create the entry breakpoint here because
-     if we have 'catch syscall' enabled, we ought to know
-     when to enable PTRACE_O_TRACESYSGOOD.  Otherwise, we would
-     start catching syscalls from ld.so/libc (which is not
-     what we want).  */
-  create_entry_breakpoint ();
 }
 
 static int
@@ -1681,8 +1677,6 @@ linux_nat_detach (struct target_ops *ops, char *args, int from_tty)
   int status;
   enum target_signal sig;
 
-  delete_entry_breakpoint ();
-
   if (target_can_async_p ())
     linux_nat_async (NULL, 0);
 
@@ -2898,9 +2892,6 @@ linux_nat_wait (ptid_t ptid, struct target_waitstatus *ourstatus)
   if (debug_linux_nat_async)
     fprintf_unfiltered (gdb_stdlog, "LLW: enter\n");
 
-  if (ourstatus->kind == TARGET_WAITKIND_EXECD)
-    delete_entry_breakpoint ();
-
   /* The first time we get here after starting a new inferior, we may
      not have added it to the LWP list yet - this is the earliest
      moment at which we know its PID.  */
@@ -3335,9 +3326,6 @@ linux_nat_mourn_inferior (struct target_ops *ops)
        there are other viable forks to debug.  Delete the exiting
        one and context-switch to the first available.  */
     linux_fork_mourn_inferior ();
-
-  /* It would fail with a warning before the inherited mourn calls above.  */
-  delete_entry_breakpoint ();
 }
 
 static LONGEST

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 20a85d1..c5c20f2 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -750,6 +750,7 @@ linux_child_post_attach (int pid)
 {
   linux_enable_event_reporting (pid_to_ptid (pid));
   check_for_thread_db ();
+  linux_enable_tracesysgood (pid_to_ptid (pid));
 }
 
 static void
@@ -757,6 +758,7 @@ linux_child_post_startup_inferior (ptid_t ptid)
 {
   linux_enable_event_reporting (ptid);
   check_for_thread_db ();
+  linux_enable_tracesysgood (ptid);
 }
 
 static int
@@ -4287,8 +4275,6 @@ linux_target_install_ops (struct target_ops *t)
   t->to_follow_fork = linux_child_follow_fork;
   t->to_find_memory_regions = linux_nat_find_memory_regions;
   t->to_make_corefile_notes = linux_nat_make_corefile_notes;
-
-  t->to_enable_tracesysgood = linux_enable_tracesysgood;
   t->to_passed_by_entrypoint = linux_passed_by_entrypoint;
 
   super_xfer_partial = t->to_xfer_partial;
diff --git a/gdb/target.c b/gdb/target.c
index 40beda5..05c0c2e 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -458,7 +458,6 @@ update_current_target (void)
       INHERIT (to_remove_exec_catchpoint, t);
       INHERIT (to_passed_by_entrypoint, t);
       INHERIT (to_set_syscall_catchpoint, t);
-      INHERIT (to_enable_tracesysgood, t);
       INHERIT (to_has_exited, t);
       /* Do no inherit to_mourn_inferiour.  */
       INHERIT (to_can_run, t);
@@ -623,9 +622,6 @@ update_current_target (void)
   de_fault (to_set_syscall_catchpoint,
 	    (int (*) (int, int, int, int, int *))
 	    tcomplain);
-  de_fault (to_enable_tracesysgood,
-            (void (*) (ptid_t))
-            tcomplain);
   de_fault (to_has_exited,
 	    (int (*) (int, int, int *))
 	    return_zero);
diff --git a/gdb/target.h b/gdb/target.h
index 3d71e64..8e70ebd 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -410,7 +410,6 @@ struct target_ops
     int (*to_remove_exec_catchpoint) (int);
     int (*to_passed_by_entrypoint) (void);
     int (*to_set_syscall_catchpoint) (int, int, int, int, int *);
-    void (*to_enable_tracesysgood) (ptid_t);
     int (*to_has_exited) (int, int, int *);
     void (*to_mourn_inferior) (struct target_ops *);
     int (*to_can_run) (void);
@@ -919,12 +918,6 @@ int target_follow_fork (int follow_child);
      (*current_target.to_set_syscall_catchpoint) (pid, needed, any_count, \
 						  table_size, table)
 
-/* Enable PTRACE_O_TRACESYSGOOD in the inferior.
-   This is mainly used for the "catch syscall" feature.  */
-
-#define target_enable_tracesysgood(ptid) \
-     (*current_target.to_enable_tracesysgood) (ptid)
-
 /* Returns TRUE if PID has exited.  And, also sets EXIT_STATUS to the
    exit code of PID, if any.  */