Re: [patch] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #3

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Fri, 30 Dec 2011 04:30:20 +0100, Joel Brobecker wrote:
> >  not preferred by me TBH.
> 
> I don't understand why, though. ON_STACK seems to be perfect, as
> we control exactly what's there,

There exist already so many security frameworks in the toolchain and Linux
kernel I expect it will have some compatibility problems with them.
But I was unable to see any problems at least with RHEl-6.2 SELinux targeted
policy and sandbox_t containment and I do not know any other real countercase.


> I keep staring at the diff,

In this case the diff is not much readable, one should read the patched code
IMO.


> and I can't figure out how the AT_SYMBOL case is falling through,

There was one leftover "break;", thanks.


> > +      /* FALLTHROUGH */
> > +    case AT_ENTRY_POINT:
> 
> Is this really a FALLTHROUGH?

It was not, it is now.


> I can't remember if you explicitly decided to use the second byte
> and then changed your mind, or whether this is a typo from the fact
> that the breakpoint instruction on x86 is 1 byte long? Suggest
> replacing with:
> 
>         Therefore, we adjust the return address by the length
>         of a breakpoint, guaranteeing that the unwinder finds
>         the correct function as the caller.

I agree my text was wrong.  It was instruction length placed there before.  It
is now breakpoint length in this patch but it was incorrectly described as
"byte" (thinking about x86* myself while writing it).


Thanks,
Jan


2011-12-30  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Joel Brobecker  <brobecker@adacore.com>

	Fix regression for gdb.cp/gdb2495.exp with gcc-4.7.
	* arch-utils.c (displaced_step_at_entry_point): Incrase BP_LEN skip to
	3 times.
	* infcall.c (call_function_by_hand) <AT_SYMBOL>: Move it upwards and
	fall through into AT_ENTRY_POINT.
	(call_function_by_hand) <AT_ENTRY_POINT>: New variable bp_len.  Adjust
	DUMMY_ADDR with it.
	* ppc-linux-tdep.c (ppc_linux_displaced_step_location): Increase
	PPC_INSN_SIZE skip to 3 times.

--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -86,7 +86,7 @@ displaced_step_at_entry_point (struct gdbarch *gdbarch)
      We don't want displaced stepping to interfere with those
      breakpoints, so leave space.  */
   gdbarch_breakpoint_from_pc (gdbarch, &addr, &bp_len);
-  addr += bp_len * 2;
+  addr += bp_len * 3;
 
   return addr;
 }
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -631,17 +631,6 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
 				args, nargs, target_values_type,
 				&real_pc, &bp_addr, get_current_regcache ());
       break;
-    case AT_ENTRY_POINT:
-      {
-	CORE_ADDR dummy_addr;
-
-	real_pc = funaddr;
-	dummy_addr = entry_point_address ();
-	/* A call dummy always consists of just a single breakpoint, so
-	   its address is the same as the address of the dummy.  */
-	bp_addr = dummy_addr;
-	break;
-      }
     case AT_SYMBOL:
       /* Some executables define a symbol __CALL_DUMMY_ADDRESS whose
 	 address is the location where the breakpoint should be
@@ -661,11 +650,39 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
 	    dummy_addr = gdbarch_convert_from_func_ptr_addr (gdbarch,
 							     dummy_addr,
 							     &current_target);
+	    /* A call dummy always consists of just a single breakpoint,
+	       so its address is the same as the address of the dummy.  */
+	    bp_addr = dummy_addr;
+	    break;
 	  }
-	else
-	  dummy_addr = entry_point_address ();
-	/* A call dummy always consists of just a single breakpoint,
-	   so it's address is the same as the address of the dummy.  */
+      }
+      /* FALLTHROUGH */
+    case AT_ENTRY_POINT:
+      {
+	CORE_ADDR dummy_addr;
+	int bp_len;
+
+	real_pc = funaddr;
+	dummy_addr = entry_point_address ();
+
+	/* If the inferior call throws an uncaught C++ exception,
+	   the inferior unwinder tries to unwind all frames, including
+	   our dummy frame.  The unwinder determines the address of
+	   the calling instruction by subtracting 1 to the return
+	   address.  So, using the entry point's address as the return
+	   address would lead the unwinder to use the unwinding
+	   information of the code immediately preceding the entry
+	   point.  This information, if found, is invalid for the dummy
+	   frame, and can potentially crash the inferior's unwinder.
+	   Therefore, we adjust the return address by the length of
+	   a breakpoint, guaranteeing that the unwinder finds the
+	   correct function as the caller.  */
+
+	gdbarch_breakpoint_from_pc (gdbarch, &dummy_addr, &bp_len);
+	dummy_addr += bp_len;
+
+	/* A call dummy always consists of just a single breakpoint, so
+	   its address is the same as the address of the dummy.  */
 	bp_addr = dummy_addr;
 	break;
       }
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -1075,7 +1075,7 @@ ppc_linux_displaced_step_location (struct gdbarch *gdbarch)
       /* Inferior calls also use the entry point as a breakpoint location.
 	 We don't want displaced stepping to interfere with those
 	 breakpoints, so leave space.  */
-      ppc_linux_entry_point_addr = addr + 2 * PPC_INSN_SIZE;
+      ppc_linux_entry_point_addr = addr + 3 * PPC_INSN_SIZE;
     }
 
   return ppc_linux_entry_point_addr;