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] Re: longjmp handling vs. glibc LD_POINTER_GUARD problems


A Saturday 17 May 2008 03:17:17, Pedro Alves wrote:
> A Wednesday 14 May 2008 19:16:23, Pedro Alves wrote:
> > A Wednesday 14 May 2008 19:00:18, Ulrich Weigand wrote:
> > > Why are we using the get_longjmp_target mechanism instead of
> > > just stepping through longjmp until we see where we come out?
> >
> > You tell me.  :-)  I had assumed there was a reason.  Perhaps
> > to support longjumping to a different stack, but that's hardly
> > a portable and frequent use case.  This seems to be the path
> > to go.
>
> Alright, here is a quick hack at it.  If this is the path to go,
> we can remove a bunch of gdbarch_get_longjmp_target implementations,
> and the code around longjmp_resume breakpoints.
>
> The patch implements the "keep stepping if going through a longjmp",
> and also, if landing somewhere inner than the current step-resume, we
> keep stepping, which is an alternative and simple implementation
> of what I proposed here:
>
> http://sourceware.org/ml/gdb-patches/2008-04/msg00162.html
>
> Tested on x86_64-unknown-linux-gnu.  The longjmp.exp passes all ok,
> and we become immune to pointer mangling.
>
> I've also simplified a bit the test, removing stuff that wasn't
> adding any value.
>


> One thing, why do I need to call get_frame_type before calling
> frame_unwind_id (first thing after frame cache invalidation)?  If I
> don't do that, I hit this gdb_assert:

Since this is now fixed, :
 http://sourceware.org/ml/gdb-patches/2008-05/msg00605.html

... here's an updated patch.  The tests are the same as before.  Tested on 
x86_86-unknown-linux-gnu, and confirmed longjmp.exp also passes
cleanly on x86-pc-linux-gnu.

What do you think?

-- 
Pedro Alves
2008-05-20  Pedro Alves  <pedro@codesourcery.com>

	* infrun.c (struct execution_control_state): Add
	stepping_through_longjmp and longjmp_caller_frame.
	(init_execution_control_state): Clear them.
	(context_switch): Context switch them.
	(handle_inferior_event): If stepping through longjmp, a SIGTRAP is
	not random.  When a longjmp breakpoint is hit, prepare to step
	until the other end.  Keep stepping while in an inner frame
	relative to the longjmp caller.  When coming out on the other end,
	check if the step-resume breakpoint frame is not needed anymore.
	(currently_stepping): Return true if stepping through longjmp.

	* gdbthread.h (struct thread_info): Add stepping_through_longjmp
	and longjmp_caller_frame.
	(save_infrun_state, load_infrun_state): Context switch them.
	* thread.c (save_infrun_state, load_infrun_state): Ditto.

---
 gdb/gdbthread.h |   14 +++++++-
 gdb/infrun.c    |   96 +++++++++++++++++++++++++++++++++++++++++---------------
 gdb/thread.c    |   12 +++++--
 3 files changed, 93 insertions(+), 29 deletions(-)

Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2008-05-20 22:41:40.000000000 +0100
+++ src/gdb/infrun.c	2008-05-20 23:41:17.000000000 +0100
@@ -1386,6 +1386,13 @@ struct execution_control_state
   int step_after_step_resume_breakpoint;
   int stepping_through_solib_after_catch;
   bpstat stepping_through_solib_catchpoints;
+
+  /* True if a longjmp call was detected while stepping, and we're
+     single-stepping until the other end.  */
+  int stepping_through_longjmp;
+  /* The frame that called longjmp.  */
+  struct frame_id longjmp_caller_frame;
+
   int new_thread_event;
   struct target_waitstatus tmpstatus;
   enum infwait_states infwait_state;
@@ -1556,6 +1563,7 @@ init_execution_control_state (struct exe
   ecs->infwait_state = infwait_normal_state;
   ecs->waiton_ptid = pid_to_ptid (-1);
   ecs->wp = &(ecs->ws);
+  ecs->stepping_through_longjmp = 0;
 }
 
 /* Return the cached copy of the last pid/waitstatus returned by
@@ -1605,7 +1613,9 @@ context_switch (struct execution_control
 			 ecs->stepping_over_breakpoint,
 			 ecs->stepping_through_solib_after_catch,
 			 ecs->stepping_through_solib_catchpoints,
-			 ecs->current_line, ecs->current_symtab);
+			 ecs->current_line, ecs->current_symtab,
+			 ecs->stepping_through_longjmp,
+			 ecs->longjmp_caller_frame);
 
       /* Load infrun state for the new thread.  */
       load_infrun_state (ecs->ptid, &prev_pc,
@@ -1615,7 +1625,9 @@ context_switch (struct execution_control
 			 &ecs->stepping_over_breakpoint,
 			 &ecs->stepping_through_solib_after_catch,
 			 &ecs->stepping_through_solib_catchpoints,
-			 &ecs->current_line, &ecs->current_symtab);
+			 &ecs->current_line, &ecs->current_symtab,
+			 &ecs->stepping_through_longjmp,
+			 &ecs->longjmp_caller_frame);
     }
 
   switch_to_thread (ecs->ptid);
@@ -2449,7 +2461,8 @@ handle_inferior_event (struct execution_
 	ecs->random_signal
 	  = !(bpstat_explains_signal (stop_bpstat)
 	      || stepping_over_breakpoint
-	      || (step_range_end && step_resume_breakpoint == NULL));
+	      || (step_range_end && step_resume_breakpoint == NULL)
+	      || ecs->stepping_through_longjmp);
       else
 	{
 	  ecs->random_signal = !bpstat_explains_signal (stop_bpstat);
@@ -2575,35 +2588,21 @@ process_event_stop_test:
     switch (what.main_action)
       {
       case BPSTAT_WHAT_SET_LONGJMP_RESUME:
-	/* If we hit the breakpoint at longjmp while stepping, we
-	   install a momentary breakpoint at the target of the
-	   jmp_buf.  */
-
+	/* If we hit the breakpoint at longjmp while stepping, prepare
+	   to step all the way through it.  */
 	if (debug_infrun)
 	  fprintf_unfiltered (gdb_stdlog,
 			      "infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME\n");
 
+	/* Step over this longjmp breakpoint.  */
 	ecs->stepping_over_breakpoint = 1;
 
-	if (!gdbarch_get_longjmp_target_p (current_gdbarch)
-	    || !gdbarch_get_longjmp_target (current_gdbarch,
-					    get_current_frame (), &jmp_buf_pc))
-	  {
-	    if (debug_infrun)
-	      fprintf_unfiltered (gdb_stdlog, "\
-infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (!gdbarch_get_longjmp_target)\n");
-	    keep_going (ecs);
-	    return;
-	  }
-
-	/* We're going to replace the current step-resume breakpoint
-	   with a longjmp-resume breakpoint.  */
-	if (step_resume_breakpoint != NULL)
-	  delete_step_resume_breakpoint (&step_resume_breakpoint);
-
-	/* Insert a breakpoint at resume address.  */
-	insert_longjmp_resume_breakpoint (jmp_buf_pc);
+	/* Store the frame id of the caller.  This is used to compare
+	   with any step-resume breakpoint set.  */
+	ecs->longjmp_caller_frame = frame_unwind_id (get_current_frame ());
 
+	/* See you on the other side!  */
+	ecs->stepping_through_longjmp = 1;
 	keep_going (ecs);
 	return;
 
@@ -2819,6 +2818,52 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (
       return;
     }
 
+  if (ecs->stepping_through_longjmp)
+    {
+      struct frame_info *frame = get_current_frame ();
+      struct frame_id frame_id = get_frame_id (frame);
+      struct gdbarch *gdbarch = get_frame_arch (frame);
+
+      if (frame_id_inner (gdbarch, frame_id, ecs->longjmp_caller_frame))
+	{
+	  if (debug_infrun)
+	    fprintf_unfiltered (gdb_stdlog,
+				"infrun: stepping through longjmp\n");
+	  /* Still not there.  */
+	  keep_going (ecs);
+	  return;
+	}
+
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog, "infrun: got out of longjmp\n");
+
+      /* We made it.  */
+      ecs->stepping_through_longjmp = 0;
+
+      /* If there's a step-resume breakpoint set, decide if we should
+	 keep stepping to the step-resume breakpoint, or if the
+	 longjmp took us outermost already, hence the step-resume
+	 breakpoint will never be hit, and we should stop now.  */
+      if (step_resume_breakpoint)
+	{
+	  if (frame_id_inner (gdbarch, frame_id,
+			      step_resume_breakpoint->frame_id))
+	    {
+	      if (debug_infrun)
+		fprintf_unfiltered (gdb_stdlog,
+				    "\
+infrun: longjmp-resume inner than step-resume\n");
+	    }
+	  else
+	    {
+	      if (debug_infrun)
+		fprintf_unfiltered (gdb_stdlog,
+				    "infrun: step-resume overran by longjmp\n");
+	      delete_step_resume_breakpoint (&step_resume_breakpoint);
+	    }
+	}
+    }
+
   if (step_resume_breakpoint)
     {
       if (debug_infrun)
@@ -3183,6 +3228,7 @@ currently_stepping (struct execution_con
 {
   return (((step_range_end && step_resume_breakpoint == NULL)
 	   || stepping_over_breakpoint)
+	  || ecs->stepping_through_longjmp
 	  || ecs->stepping_through_solib_after_catch
 	  || bpstat_should_step ());
 }
Index: src/gdb/gdbthread.h
===================================================================
--- src.orig/gdb/gdbthread.h	2008-05-20 22:41:40.000000000 +0100
+++ src/gdb/gdbthread.h	2008-05-20 23:25:02.000000000 +0100
@@ -63,6 +63,12 @@ struct thread_info
      when we finally do stop stepping.  */
   bpstat stepping_through_solib_catchpoints;
 
+  /* True if a longjmp call was detected while stepping, and we're
+     single-stepping until the other end.  */
+  int stepping_through_longjmp;
+  /* The frame that called longjmp.  */
+  struct frame_id longjmp_caller_frame;
+
   /* Private data used by the target vector implementation.  */
   struct private_thread_info *private;
 };
@@ -126,7 +132,9 @@ extern void save_infrun_state (ptid_t pt
 			       int       stepping_through_solib_after_catch,
 			       bpstat    stepping_through_solib_catchpoints,
 			       int       current_line,
-			       struct symtab *current_symtab);
+			       struct symtab *current_symtab,
+			       int stepping_through_longjmp,
+			       struct frame_id longjmp_caller_frame);
 
 /* infrun context switch: load the debugger state previously saved
    for the given thread.  */
@@ -141,7 +149,9 @@ extern void load_infrun_state (ptid_t pt
 			       int       *stepping_through_solib_affter_catch,
 			       bpstat    *stepping_through_solib_catchpoints,
 			       int       *current_line,
-			       struct symtab **current_symtab);
+			       struct symtab **current_symtab,
+			       int *stepping_through_longjmp,
+			       struct frame_id *longjmp_caller_frame);
 
 /* Switch from one thread to another.  */
 extern void switch_to_thread (ptid_t ptid);
Index: src/gdb/thread.c
===================================================================
--- src.orig/gdb/thread.c	2008-05-20 23:18:13.000000000 +0100
+++ src/gdb/thread.c	2008-05-20 23:25:02.000000000 +0100
@@ -325,7 +325,9 @@ load_infrun_state (ptid_t ptid,
 		   int *stepping_through_solib_after_catch,
 		   bpstat *stepping_through_solib_catchpoints,
 		   int *current_line,
-		   struct symtab **current_symtab)
+		   struct symtab **current_symtab,
+		   int *stepping_through_longjmp,
+		   struct frame_id *longjmp_caller_frame)
 {
   struct thread_info *tp;
 
@@ -348,6 +350,8 @@ load_infrun_state (ptid_t ptid,
     tp->stepping_through_solib_catchpoints;
   *current_line = tp->current_line;
   *current_symtab = tp->current_symtab;
+  *stepping_through_longjmp = tp->stepping_through_longjmp;
+  *longjmp_caller_frame = tp->longjmp_caller_frame;
 }
 
 /* Save infrun state for the thread PID.  */
@@ -364,7 +368,9 @@ save_infrun_state (ptid_t ptid,
 		   int stepping_through_solib_after_catch,
 		   bpstat stepping_through_solib_catchpoints,
 		   int current_line,
-		   struct symtab *current_symtab)
+		   struct symtab *current_symtab,
+		   int stepping_through_longjmp,
+		   struct frame_id longjmp_caller_frame)
 {
   struct thread_info *tp;
 
@@ -385,6 +391,8 @@ save_infrun_state (ptid_t ptid,
   tp->stepping_through_solib_catchpoints = stepping_through_solib_catchpoints;
   tp->current_line = current_line;
   tp->current_symtab = current_symtab;
+  tp->stepping_through_longjmp = stepping_through_longjmp;
+  tp->longjmp_caller_frame = longjmp_caller_frame;
 }
 
 /* Return true if TP is an active thread. */
2008-05-17  Pedro Alves  <pedro@codesourcery.com>

	* longjmp.c, longjmp.exp: Add tests to test ignoring inner
	longjmp resumes while stepping, and update current tests.

---
 gdb/testsuite/gdb.base/longjmp.c   |  103 ++++++++++++++++++++++++++++++++-----
 gdb/testsuite/gdb.base/longjmp.exp |   96 ++++++++++++++++++----------------
 2 files changed, 142 insertions(+), 57 deletions(-)

Index: src/gdb/testsuite/gdb.base/longjmp.c
===================================================================
--- src.orig/gdb/testsuite/gdb.base/longjmp.c	2008-05-04 20:43:31.000000000 +0100
+++ src/gdb/testsuite/gdb.base/longjmp.c	2008-05-17 02:36:56.000000000 +0100
@@ -19,6 +19,7 @@
 #include <setjmp.h>
 
 jmp_buf env;
+jmp_buf env2;
 
 volatile int longjmps = 0;
 volatile int resumes = 0;
@@ -33,7 +34,7 @@ call_longjmp (jmp_buf *buf)
 void
 hidden_longjmp (void)
 {
-  if (setjmp (env) == 0) /* longjmp caught */
+  if (setjmp (env) == 0)
     {
       call_longjmp (&env);
     }
@@ -41,41 +42,117 @@ hidden_longjmp (void)
     resumes++;
 }
 
+void
+hidden_longjmp_2 (void)
+{
+  if (setjmp (env) == 0)
+    {
+      if (setjmp (env2) == 0)
+	{
+	  longjmps++;
+	  longjmp (env2, 1);
+	}
+      else
+	{
+	  resumes++;
+	  longjmps++;
+	  longjmp (env, 1);
+	}
+    }
+  else
+    {
+      resumes++;
+    }
+}
+
+void
+hidden_longjmp_3_1 (void)
+{
+  if (setjmp (env2) == 0)
+    {
+      longjmps++;
+      longjmp (env2, 1);
+    }
+  else
+    {
+      resumes++;
+      longjmps++;
+      longjmp (env, 1);
+    }
+}
+
+void
+hidden_longjmp_3 (void)
+{
+  hidden_longjmp_3_1 ();
+}
+
 int
 main ()
 {
   volatile int i = 0;
 
   /* Pattern 1 - simple longjmp.  */
-  if (setjmp (env) == 0) /* patt1 */
+  if (setjmp (env) == 0)
     {
       longjmps++;
-      longjmp (env, 1);
+      longjmp (env, 1); /* patt1 */
     }
   else
     {
-      resumes++;
+      resumes++; /* resume1 */
     }
 
-  i = 1; /* miss_step_1 */
-
 
   /* Pattern 2 - longjmp from an inner function.  */
-  if (setjmp (env) == 0) /* patt2 */
+  if (setjmp (env) == 0)
     {
-      call_longjmp (&env);
+      call_longjmp (&env); /* patt2 */
     }
   else
     {
-      resumes++;
+      resumes++; /* resume2 */
     }
 
-  i = 2; /* miss_step_2 */
 
-  /* Pattern 3 - setjmp/longjmp inside stepped-over function.  */
-  hidden_longjmp (); /* patt3 */
+  /* Pattern 3 - prefer longjmp resume.
+
+     This tests if GDB chooses the longjmp-resume over the step-resume
+     breakpoint.  If GDB chooses wrongly, a step over the
+     hidden_longjmp_3 function will resume the inferior and pass
+     straight the else clause without stopping to step.  */
+  if (setjmp (env) == 0)
+    hidden_longjmp_3 (); /* patt3 */
+  else
+    resumes++; /* resume3 */
+
+
+  /* Pattern 4 - prefer longjmp resume after step.
+
+     Quite similar, but in this case, we step into hidden_longjmp_3
+     before next'ing over the longjmp.  In this case, the step-resume
+     breakpoint will be set in an inner stack.  Check if GDB still
+     detects that the longjmp-resume address is prefered.  */
+  if (setjmp (env) == 0)
+    hidden_longjmp_3 (); /* patt4 */
+  else
+    resumes++; /* resume4 */
+
+
+  /* Pattern 5 - setjmp/longjmp handled inside stepped-over function.
+
+     Test that we don't miss-handle internal setjmp/longjmp sequences.
+     A next over this should not stop at the longjmp resume
+     address.  */
+  hidden_longjmp (); /* patt5 */
+  i = 5; /* patt_end5.  */
+
+
+  /* Pattern 6 - nested setjmp/longjmp handled inside stepped-over
+     function.  */
+  hidden_longjmp_2 (); /* patt6 */
+  i = 6; /* patt_end6.  */
 
-  i = 3; /* patt_end3.  */
 
   return 0;
 }
Index: src/gdb/testsuite/gdb.base/longjmp.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/longjmp.exp	2008-05-04 20:43:31.000000000 +0100
+++ src/gdb/testsuite/gdb.base/longjmp.exp	2008-05-17 02:30:18.000000000 +0100
@@ -47,12 +47,12 @@ if ![runto_main] then {
    return 0
 }
 
-set bp_miss_step_1 [gdb_get_line_number "miss_step_1"]
-set bp_miss_step_2 [gdb_get_line_number "miss_step_2"]
-
 set bp_start_test_1 [gdb_get_line_number "patt1"]
 set bp_start_test_2 [gdb_get_line_number "patt2"]
 set bp_start_test_3 [gdb_get_line_number "patt3"]
+set bp_start_test_4 [gdb_get_line_number "patt4"]
+set bp_start_test_5 [gdb_get_line_number "patt5"]
+set bp_start_test_6 [gdb_get_line_number "patt6"]
 
 #
 # Pattern 1 - simple longjmp.
@@ -65,25 +65,7 @@ gdb_test "break $bp_start_test_1" \
     "breakpoint at pattern 1 start"
 gdb_test "continue" "patt1.*" "continue to breakpoint at pattern 1 start"
 
-# set safe-net break
-gdb_test "break $bp_miss_step_1" \
-    "Breakpoint.*at.* file .*$srcfile, line.*$bp_miss_step_1.*" \
-    "breakpoint at miss_step_1"
-
-gdb_test "next" "longjmps\\+\\+;.*" "next over setjmp (1)"
-gdb_test "next" "longjmp \\(env, 1\\);.*" "next to longjmp (1)"
-
-set msg "next over longjmp(1)"
-gdb_test_multiple "next" $msg {
-    -re ".*patt1.*" {
-	pass $msg
-	gdb_test "next" "resumes\\+\\+.*" "next into else block (1)"
-	gdb_test "next" "miss_step_1.*" "next into safety net (1)"
-    }
-    -re "miss_step_1.*" {
-	fail $msg
-    }
-}
+gdb_test "next" ".*resume1.*" "next over longjmp(1)"
 
 #
 # Pattern 2 - longjmp from an inner function.
@@ -96,28 +78,10 @@ gdb_test "break $bp_start_test_2" \
     "breakpoint at pattern 2 start"
 gdb_test "continue" "patt2.*" "continue to breakpoint at pattern 2 start"
 
-# set safe-net break
-gdb_test "break $bp_miss_step_2" \
-    "Breakpoint.*at.* file .*$srcfile, line.*$bp_miss_step_2.*" \
-    "breakpoint at miss_step_2"
-
-gdb_test "next" "call_longjmp.*" "next over setjmp (2)"
-
-set msg "next over call_longjmp (2)"
-gdb_test_multiple "next" $msg {
-    -re ".*patt2.*" {
-	pass $msg
-
-	gdb_test "next" "resumes\\+\\+.*" "next into else block (2)"
-	gdb_test "next" "miss_step_2.*" "next into safety net (2)"
-    }
-    -re "miss_step_2.*" {
-	fail $msg
-    }
-}
+gdb_test "next" ".*resume2.*" "next over call_longjmp (2)"
 
 #
-# Pattern 3 - setjmp/longjmp inside stepped-over function.
+# Pattern 3 - prefer longjmp resume
 #
 
 delete_breakpoints
@@ -125,6 +89,50 @@ delete_breakpoints
 gdb_test "break $bp_start_test_3" \
     "Breakpoint.*at.* file .*$srcfile, line.*$bp_start_test_3.*" \
     "breakpoint at pattern 3 start"
-gdb_test "continue" "patt3.*" "continue to breakpoint at pattern 3 start"
+gdb_test "continue" "patt3.*" \
+    "continue to breakpoint at pattern 3 start"
+
+gdb_test "next" "resume3.*" "next over hidden_longjmp_3 (3)"
+
+
+#
+# Pattern 4 - prefer longjmp resume after step
+#
+
+delete_breakpoints
+
+gdb_test "break $bp_start_test_4" \
+    "Breakpoint.*at.* file .*$srcfile, line.*$bp_start_test_4.*" \
+    "breakpoint at pattern 4 start"
+gdb_test "continue" "patt4.*" "continue to breakpoint at pattern 4 start"
+
+gdb_test "step" "hidden_longjmp_3_1 \\(\\).*" "step into hidden_longjmp_3 (4)"
+
+gdb_test "next" "resume4.*" "next over hidden_longjmp_3_1 (4)"
+
+#
+# Pattern 5 - setjmp/longjmp handled inside stepped-over function.
+#
+
+delete_breakpoints
+
+gdb_test "break $bp_start_test_5" \
+    "Breakpoint.*at.* file .*$srcfile, line.*$bp_start_test_5.*" \
+    "breakpoint at pattern 5 start"
+gdb_test "continue" "patt5.*" "continue to breakpoint at pattern 5 start"
+
+gdb_test "next" "patt_end5.*" "next over patt5"
+
+#
+# Pattern 6 - nested setjmp/longjmp handled inside stepped-over
+# function.
+#
+
+delete_breakpoints
+
+gdb_test "break $bp_start_test_6" \
+    "Breakpoint.*at.* file .*$srcfile, line.*$bp_start_test_6.*" \
+    "breakpoint at pattern 6 start"
+gdb_test "continue" "patt6.*" "continue to breakpoint at pattern 6 start"
 
-gdb_test "next" "longjmp caught.*" "next over patt3"
+gdb_test "next" "patt_end6.*" "next over patt6"

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