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 05/12] entryval: Self tail calls verification


Hi,

the idea is GDB should never print a value that can be wrong.  If it is not
certain about some inferior value it should rather print <optimized out>.

this testcase:
#0  d (i=203) at ./gdb.arch/amd64-entry-value.cc:33
#1  0x00000000004005df in self (i=200) at ./gdb.arch/amd64-entry-value.cc:112
#2  0x0000000000400406 in main () at ./gdb.arch/amd64-entry-value.cc:125
->
#0  d (i=<optimized out>) at ./gdb.arch/amd64-entry-value.cc:33
#1  0x00000000004005df in self (i=<optimized out>) at ./gdb.arch/amd64-entry-value.cc:112
#2  0x0000000000400406 in main () at ./gdb.arch/amd64-entry-value.cc:125

Without this extension you can see it displays an impossible backtrace.
`i' at `d' entry would have to be 202, therefore `i' at `self' entry would
have to be 200.  But in such case `self' would not call `d'.

If a function can call - even via other functions - itself, the matching of
caller vs. callee PCs does not say much as we do not know how many cycles
through the self tail call loop could have happened.  Make such parameters
<optimized out>.

It does not try to find out which parameters could be passed unmodified or
which calls could / could not happen.  Existence of any call site means for
GDB the call could be made.  I do not think it should be much of a concern as
tail call frames were primarily intended for reconstructing exit paths after
detected errors.  The practical deployment may tell more.

Tail call frames are not applicable for example to GDB error() function (and
GDB functions similar to it) as they use stdarg which needs to use `call'
instruction for proper stack setup for the stack based stdarg passing.


Thanks,
Jan


gdb/
2011-07-18  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Protect entry values against self tail calls.
	* dwarf2loc.c (VEC (CORE_ADDR), free_addr_vecp)
	(func_verify_no_selftailcall): New.
	(dwarf_expr_dwarf_reg_entry_value): Call func_verify_no_selftailcall.

gdb/testsuite/
2011-07-18  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Protect entry values against self tail calls.
	* gdb.arch/amd64-entry-value.cc (self2, self): New.
	(main): Call self.
	* gdb.arch/amd64-entry-value.exp (self: breakhere, self: bt)
	(self: bt verbose): New tests.

--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -391,6 +391,100 @@ func_addr_to_tail_call_list (struct gdbarch *gdbarch, CORE_ADDR addr)
   return sym;
 }
 
+/* Define VEC (CORE_ADDR) functions.  */
+DEF_VEC_I (CORE_ADDR);
+
+/* Cleanup helper to free VEC (CORE_ADDR) **.  */
+
+static void
+free_addr_vecp (void *arg)
+{
+  VEC (CORE_ADDR) **vecp = arg;
+
+  if (*vecp)
+    {
+      VEC_free (CORE_ADDR, *vecp);
+      *vecp = NULL;
+    }
+}
+
+/* Verify function with entry point exact address ADDR can never call itself
+   via its tail calls (incl. transitively).  Throw NOT_FOUND_ERROR if it can
+   call itself via tail calls.
+
+   If a funtion can tail call itself its entry value based parameters are
+   unreliable.  There is no verification whether the value of some/all
+   parameters is unchanged through the self tail call, we expect if there is
+   a self tail call all the parameters can be modified.  */
+
+static void
+func_verify_no_selftailcall (struct gdbarch *gdbarch, CORE_ADDR verify_addr)
+{
+  struct obstack addr_obstack;
+  struct cleanup *old_chain;
+  CORE_ADDR addr;
+
+  /* Track here CORE_ADDRs which were already visited.  */
+  htab_t addr_hash;
+
+  /* The verification is completely unordered.  Track here function addresses
+     which still need to be iterated.  */
+  VEC (CORE_ADDR) *todo = NULL;
+
+  obstack_init (&addr_obstack);
+  old_chain = make_cleanup_obstack_free (&addr_obstack);   
+  addr_hash = htab_create_alloc_ex (64, core_addr_hash, core_addr_eq, NULL,
+				    &addr_obstack, hashtab_obstack_allocate,
+				    NULL);
+  make_cleanup_htab_delete (addr_hash);
+
+  make_cleanup (free_addr_vecp, &todo);
+
+  VEC_safe_push (CORE_ADDR, todo, verify_addr);
+  while (!VEC_empty (CORE_ADDR, todo))
+    {
+      struct symbol *func_sym;
+      struct call_site *call_site;
+
+      addr = VEC_pop (CORE_ADDR, todo);
+
+      func_sym = func_addr_to_tail_call_list (gdbarch, addr);
+
+      for (call_site = TYPE_TAIL_CALL_LIST (SYMBOL_TYPE (func_sym));
+	   call_site; call_site = call_site->tail_call_next)
+	{
+	  CORE_ADDR target_addr;
+	  void **slot;
+
+	  /* CALLER_FRAME with registers is not available for tail-call jumped
+	     frames.  */
+	  target_addr = call_site_to_target_addr (call_site, NULL);
+
+	  if (target_addr == verify_addr)
+	    {
+	      struct minimal_symbol *msym;
+	      
+	      msym = lookup_minimal_symbol_by_pc (verify_addr);
+	      throw_error (NOT_FOUND_ERROR, _("DW_OP_GNU_entry_value resolving "
+					      "has found function \"%s\" at %s "
+					      "can call itself via tail calls"),
+			   msym == NULL ? "???" : SYMBOL_PRINT_NAME (msym),
+			   paddress (gdbarch, verify_addr));
+	    }
+
+	  slot = htab_find_slot (addr_hash, &target_addr, INSERT);
+	  if (*slot == NULL)
+	    {
+	      *slot = obstack_copy (&addr_obstack, &target_addr,
+				    sizeof (target_addr));
+	      VEC_safe_push (CORE_ADDR, todo, target_addr);
+	    }
+	}
+    }
+
+  do_cleanups (old_chain);
+}
+
 /* Print user readable form of CALL_SITE->PC to gdb_stdlog.  Used only for
    TAILCALL_DEBUG.  */
 
@@ -760,6 +854,10 @@ dwarf_expr_dwarf_reg_entry_value (struct frame_info *frame, int dwarf_reg,
 		   paddress (caller_gdbarch, func_addr));
     }
 
+  /* No entry value based parameters would be reliable if this function can
+     call itself via tail calls.  */
+  func_verify_no_selftailcall (caller_gdbarch, func_addr);
+
   for (iparams = 0; iparams < call_site->parameter_count; iparams++)
     {
       parameter = &call_site->parameter[iparams];
--- a/gdb/testsuite/gdb.arch/amd64-entry-value.cc
+++ b/gdb/testsuite/gdb.arch/amd64-entry-value.cc
@@ -90,6 +90,29 @@ amb_a (int i)
   amb_b (i + 1);
 }
 
+static void __attribute__((noinline, noclone)) self (int i);
+
+static void __attribute__((noinline, noclone))
+self2 (int i)
+{
+  self (i);
+}
+
+static void __attribute__((noinline, noclone))
+self (int i)
+{
+  if (i == 200)
+    {
+      /* GCC would inline `self' as `cmovne' without the `self2' indirect.  */
+      self2 (i + 1);
+    }
+  else
+    {
+      e (v);
+      d (i + 2);
+    }
+}
+
 int
 main ()
 {
@@ -99,5 +122,6 @@ main ()
   else
     b (5);
   amb_a (100);
+  self (200);
   return 0;
 }
--- a/gdb/testsuite/gdb.arch/amd64-entry-value.exp
+++ b/gdb/testsuite/gdb.arch/amd64-entry-value.exp
@@ -76,3 +76,19 @@ gdb_continue_to_breakpoint "ambiguous: breakhere"
 # #6  0x00000000004003f8 in main () at gdb.arch/amd64-entry-value.cc:100
 gdb_test "bt" "^bt\r\n#0 +d \\(i=<optimized out>\\)\[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in amb_z \\(i=<optimized out>\\)\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in amb_y \\(i=<optimized out>\\)\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in amb_x \\(i=<optimized out>\\)\[^\r\n\]*\r\n#4 +0x\[0-9a-f\]+ in amb_b \\(i=101\\)\[^\r\n\]*\r\n#5 +0x\[0-9a-f\]+ in amb_a \\(i=100\\)\[^\r\n\]*\r\n#6 +0x\[0-9a-f\]+ in main \\(\\)\[^\r\n\]*" \
 	 "ambiguous: bt"
+
+
+# Test self tail calls verification.
+# GDB should not print the real value as it is ambiguous.
+
+gdb_continue_to_breakpoint "self: breakhere"
+
+# #0  d (i=<optimized out>) at gdb.arch/amd64-entry-value.cc:33
+# #1  0x00000000004005df in self (i=<optimized out>) at gdb.arch/amd64-entry-value.cc:111
+# #2  0x0000000000400406 in main () at gdb.arch/amd64-entry-value.cc:124
+gdb_test "bt" "^bt\r\n#0 +d \\(i=<optimized out>\\)\[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in self \\(i=<optimized out>\\)\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in main \\(\\)\[^\r\n\]*" \
+	 "self: bt"
+
+gdb_test_no_output "set verbose on"
+gdb_test "bt" "DW_OP_GNU_entry_value resolving has found function \"self\\(int\\)\" at 0x\[0-9a-f\]+ can call itself via tail calls\r\n.*" \
+	 "self: bt verbose"


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