Re: [patch] Support -fsplit-stack (previous frame inner to this frame)

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

On Tue, 23 Nov 2010 21:40:49 +0100, Tom Tromey wrote:
> A new frame type seems like an ok idea.

Maybe later if this patch still has concerns.  The chaining of frames
complication may not be worth it.


On Tue, 23 Nov 2010 23:38:11 +0100, Ian Lance Taylor wrote:
> The __morestack function has hidden visibility when using ELF, so if
> both the executable and a shared library are compiled with -fsplit-stack
> they will each have their own copy of __morestack.

That optimization makes no sense there:
 * This check is done only in the error case.
   (For frame_find_by_id it does not seem to be needed.)
 * Frames are already cached.
 * Symbols are already looked up for the PC when printing the frame.
 * Frame is not resolved until it needs to be printed.
 * Any lookup caching should be done rather in the symbols lookup code anyway.

No regressions on {x86_64,x86_64-m32,i686}-fedora14-linux-gnu.


Thanks,
Jan


gdb/
2010-11-24  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* frame.c (get_prev_frame_1) <UNWIND_INNER_ID>: New variables
	this_pc_in_block, morestack_msym and morestack_name.  Check for
	"__morestack" minimal symbol there.

gdb/testsuite/
2010-11-24  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/morestack.exp: New file.
	* gdb.base/morestack.c: New file.

--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1501,14 +1501,26 @@ get_prev_frame_1 (struct frame_info *this_frame)
       && frame_id_inner (get_frame_arch (this_frame->next), this_id,
 			 get_frame_id (this_frame->next)))
     {
-      if (frame_debug)
+      CORE_ADDR this_pc_in_block;
+      struct minimal_symbol *morestack_msym;
+      const char *morestack_name = NULL;
+      
+      /* gcc -fsplit-stack __morestack can continue the stack anywhere.  */
+      this_pc_in_block = get_frame_address_in_block (this_frame);
+      morestack_msym = lookup_minimal_symbol_by_pc (this_pc_in_block);
+      if (morestack_msym)
+	morestack_name = SYMBOL_LINKAGE_NAME (morestack_msym);
+      if (!morestack_name || strcmp (morestack_name, "__morestack") != 0)
 	{
-	  fprintf_unfiltered (gdb_stdlog, "-> ");
-	  fprint_frame (gdb_stdlog, NULL);
-	  fprintf_unfiltered (gdb_stdlog, " // this frame ID is inner }\n");
+	  if (frame_debug)
+	    {
+	      fprintf_unfiltered (gdb_stdlog, "-> ");
+	      fprint_frame (gdb_stdlog, NULL);
+	      fprintf_unfiltered (gdb_stdlog, " // this frame ID is inner }\n");
+	    }
+	  this_frame->stop_reason = UNWIND_INNER_ID;
+	  return NULL;
 	}
-      this_frame->stop_reason = UNWIND_INNER_ID;
-      return NULL;
     }
 
   /* Check that this and the next frame are not identical.  If they
--- /dev/null
+++ b/gdb/testsuite/gdb.base/morestack.c
@@ -0,0 +1,105 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Based on the gcc testcase `gcc/testsuite/gcc.dg/split-1.c'.  This test
+   needs to use setrlimit to set the stack size, so it can only run on Unix.
+   */
+
+#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/resource.h>
+#include <stdio.h>
+#include <sys/mman.h>
+
+/* Use a noinline function to ensure that the buffer is not removed
+   from the stack.  */
+static void use_buffer (char *buf) __attribute__ ((noinline));
+static void
+use_buffer (char *buf)
+{
+  buf[0] = '\0';
+}
+
+static volatile int marker_var;
+
+static void
+marker_miss (void)
+{
+  marker_var = 0;
+}
+
+static void
+marker_hit (void)
+{
+  marker_var = 0;
+}
+
+void *reserved;
+#define RESERVED_SIZE 0x1000000
+
+/* Each recursive call uses 10,000 bytes.  We call it 1000 times,
+   using a total of 10,000,000 bytes.  If -fsplit-stack is not
+   working, that will overflow our stack limit.  */
+
+static void
+down (int i)
+{
+  char buf[10000];
+  static void *last;
+
+  if (last && last < (void *) buf)
+    {
+      printf ("%d: %p < %p\n", i, last, buf);
+      marker_hit ();
+    }
+  last = buf;
+
+  if (i == 500)
+    {
+      if (munmap (reserved, RESERVED_SIZE) != 0)
+        abort ();
+      reserved = NULL;
+    }
+
+  if (i > 0)
+    {
+      use_buffer (buf);
+      down (i - 1);
+    }
+  else
+    marker_miss ();
+}
+
+int
+main (void)
+{
+  struct rlimit r;
+
+  reserved = mmap (NULL, RESERVED_SIZE, PROT_READ | PROT_WRITE,
+		   MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+  if (reserved == MAP_FAILED)
+    abort ();
+
+  /* We set a stack limit because we are usually invoked via make, and
+     make sets the stack limit to be as large as possible.  */
+  r.rlim_cur = 8192 * 1024;
+  r.rlim_max = 8192 * 1024;
+  if (setrlimit (RLIMIT_STACK, &r) != 0)
+    abort ();
+  down (1000);
+  return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/morestack.exp
@@ -0,0 +1,52 @@
+# Copyright (C) 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if [get_compiler_info "ignored"] {
+    return -1
+}
+
+if {$gcc_compiled == 0} {
+    return -1
+}
+
+set testfile morestack
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${testfile}.c {additional_flags=-fsplit-stack}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_breakpoint "marker_hit"
+gdb_breakpoint "marker_miss"
+
+set test "continue"
+gdb_test_multiple $test $test {
+    -re "marker_hit.*$gdb_prompt $" {
+	pass $test
+    }
+    -re "marker_miss.*$gdb_prompt $" {
+	# The testcase failed to violated the frame_id_inner condition by
+	# handing inner frame with higher (on the stack-grows-down arches)
+	# $sp address than the outer frame.
+	xfail $test
+	return 0
+    }
+}
+
+# FAIL was on `bt' producing:
+# Backtrace stopped: previous frame inner to this frame (corrupt stack?)
+gdb_test "up 3000" " in main .*"