Re: [obish?sym;rfa:doc] Wire up vsyscall

Re: [obish?sym;rfa:doc] Wire up vsyscall

[Roland McGrath]

> At present I know of the following problems:
> 
> 1. The code assumes ELF support in BFD
> Per recent BFD posts, I'm fixing this.

I didn't find what you're referring to in a quick scan of the archives.
Can you give me a specific URL in the mailing list archive, or an exact
subject line to search for?  I participated in the discussion about the
issue, but I don't recall a posting from you proposing changes to address it.

> 2. bfd_elf_bfd_from_remote_memory requires an existing bfd
> For the case:
> 	$ ./gdb
> 	(gdb) attach <pid>
> that isn't necessarially so.  1. will fix this.

I used a bfd argument this way because the functions I saw handy to do
target-integer-format extraction used a bfd argument.  Not knowing BFD too
well, I probably overlooked some other way to get at that info.  

> 3. inferior recycle VS inferior create
> Because GDB currently fudges things by recycling the inferior at each 
> run (instead of creating a new fresh inferior) symfile tries to reload 
> the vsyscall page.  Doesn't do any harm fortunatly.  Having a proper 
> inferior will fix this.

I'm not clear on what you mean by "reload the vsyscall page" here.  What
needs to happen on a repeated run is to repeat from scratch all the
vsyscall-page-related work that was done on the first run/attach.  
That is, all of the details can turn out different on each new exec:
whether or not AT_SYSINFO_EHDR is supplied at all; the address of the page;
the contents of the page, i.e. its symbol table and unwind info.


I don't know the status of the "catch exec" functionality.
But if that works at all, it needs to do this observer notification too.
Otherwise the patch looks good to me modulo the following tiny nits.

> +	/* FIXME: cagney/2004-05-06: Should not require an existing
> +	   BFD when trying to create a run-time BFD of the VSYSCALL
> +	   page in the inferior.  Unfortunatly that's the current
> +	   interface so for the moment bail.  Introducing a
> +	   ``bfd_runtime'' (a BFD created using the loaded image) file
> +	   format should fix this.  */
> +	return;

I think there should be an error thrown or at least a warning message here,
instead of just silent not-doing.  The user can then do "file" before
"attach" to make the backtraces work.  Of course the details of the
workaround don't matter if the problem is being fixed properly quite soon.

> +	printf_unfiltered ("Loaded system supplied DSO at 0x%s\n",
> +			   paddr_nz (sysinfo_ehdr));

I don't know if printf_unfiltered means this or not, but this output should
only appear under `set verbose on'.  Also, I would change the message to
fix the grammar and to be consistent with other gdb messages:

	"Reading symbols from system-supplied DSO at 0x%s\n"

> +  /* Want to know of each new inferior so that it's vsyscall info can
> +     be extracted.  */

Typo here: it's -> its.

> +@var{GDBN} has just created to a new inferior.  For @samp{run}, it is

Extra word here:	       ^^

> +immediatly after opening the inferior (and before any information on

Typo here: immediatly -> immediately.

> but remember I intend changing the second to:
> 
> ...
> #1 0x1234 in <signal trampoline>
> ...

That's a nice improvement anyway.  
I have never liked the hiding of the PC value.


Thanks,
Roland

[obish?sym;rfa:doc] Wire up vsyscall

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 918 bytes --]

Hello,

The attached patch adds a new observable event ``inferior created'' and 
then uses that to load up the vsyscall page (if its available).

At present I know of the following problems:

1. The code assumes ELF support in BFD
Per recent BFD posts, I'm fixing this.

2. bfd_elf_bfd_from_remote_memory requires an existing bfd
For the case:
	$ ./gdb
	(gdb) attach <pid>
that isn't necessarially so.  1. will fix this.

3. inferior recycle VS inferior create
Because GDB currently fudges things by recycling the inferior at each 
run (instead of creating a new fresh inferior) symfile tries to reload 
the vsyscall page.  Doesn't do any harm fortunatly.  Having a proper 
inferior will fix this.

Observer doco ok?
The symtab tweak is relatively obvious (I stole it from Roland's post) 
but still, comments?

As for tests, I'm thinking something to check for the absence of:
	0x1234 in ?????
in a backtrace.

Andrew

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 8314 bytes --]

2004-05-06  Andrew Cagney  <cagney@redhat.com>
	    Roland McGrath  <roland@redhat.com>

	* inftarg.c: Include "observer.h".
	(child_attach, child_create_inferior): Notify inferior_created.
	* corelow.c: Include "observer.h".
	(core_open): Notify inferior_created.
	* symfile-mem.c: Include "observer.h", "auxv.h", and
	"elf/common.h".
	(struct symbol_file_add_from_memory_args)
	(symbol_file_add_from_memory_wrapper, add_vsyscall_page)
	(_initialize_symfile_mem): Add an inferior_created observer, use
	to load the vsyscall page.
	* Makefile.in (symfile-mem.o, inftarg.o, corelow.o): Update
	dependencies.

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.560
diff -p -u -r1.560 Makefile.in
--- Makefile.in	4 May 2004 23:47:15 -0000	1.560
+++ Makefile.in	6 May 2004 18:46:39 -0000
@@ -1651,7 +1651,7 @@ corefile.o: corefile.c $(defs_h) $(gdb_s
 corelow.o: corelow.c $(defs_h) $(arch_utils_h) $(gdb_string_h) $(frame_h) \
 	$(inferior_h) $(symtab_h) $(command_h) $(bfd_h) $(target_h) \
 	$(gdbcore_h) $(gdbthread_h) $(regcache_h) $(regset_h) $(symfile_h) \
-	$(exec_h) $(readline_h) $(gdb_assert_h)
+	$(exec_h) $(readline_h) $(gdb_assert_h) $(observer_h)
 core-regset.o: core-regset.c $(defs_h) $(command_h) $(gdbcore_h) \
 	$(inferior_h) $(target_h) $(gdb_string_h) $(gregset_h)
 cp-abi.o: cp-abi.c $(defs_h) $(value_h) $(cp_abi_h) $(command_h) $(gdbcmd_h) \
@@ -1934,7 +1934,8 @@ infrun.o: infrun.c $(defs_h) $(gdb_strin
 	$(symfile_h) $(top_h) $(inf_loop_h) $(regcache_h) $(value_h) \
 	$(observer_h) $(language_h) $(gdb_assert_h)
 inftarg.o: inftarg.c $(defs_h) $(frame_h) $(inferior_h) $(target_h) \
-	$(gdbcore_h) $(command_h) $(gdb_stat_h) $(gdb_wait_h) $(inflow_h)
+	$(gdbcore_h) $(command_h) $(gdb_stat_h) $(gdb_wait_h) $(inflow_h) \
+	$(observer_h)
 infttrace.o: infttrace.c $(defs_h) $(frame_h) $(inferior_h) $(target_h) \
 	$(gdb_string_h) $(gdb_wait_h) $(command_h) $(gdbthread_h) \
 	$(gdbcore_h) $(infttrace_h)
@@ -2427,7 +2428,7 @@ symfile.o: symfile.c $(defs_h) $(bfdlink
 	$(gdb_string_h) $(gdb_stat_h)
 symfile-mem.o: symfile-mem.c $(defs_h) $(symtab_h) $(gdbcore_h) \
 	$(objfiles_h) $(gdbcmd_h) $(target_h) $(value_h) $(symfile_h) \
-	$(symfile_mem_h)
+	$(symfile_mem_h) $(observer_h) $(auxv_h) $(elf_common_h)
 symmisc.o: symmisc.c $(defs_h) $(symtab_h) $(gdbtypes_h) $(bfd_h) \
 	$(symfile_h) $(objfiles_h) $(breakpoint_h) $(command_h) \
 	$(gdb_obstack_h) $(language_h) $(bcache_h) $(block_h) $(gdb_regex_h) \
Index: corelow.c
===================================================================
RCS file: /cvs/src/src/gdb/corelow.c,v
retrieving revision 1.39
diff -p -u -r1.39 corelow.c
--- corelow.c	28 Apr 2004 16:36:25 -0000	1.39
+++ corelow.c	6 May 2004 18:46:40 -0000
@@ -43,7 +43,7 @@
 #include "symfile.h"
 #include "exec.h"
 #include "readline/readline.h"
-
+#include "observer.h"
 #include "gdb_assert.h"
 
 #ifndef O_BINARY
@@ -354,6 +354,10 @@ core_open (char *filename, int from_tty)
 
   ontop = !push_target (&core_ops);
   discard_cleanups (old_chain);
+
+  /* This is done first, before anything has a chance to query the
+     inferior for information such as symbols.  */
+  observer_notify_inferior_created (&core_ops);
 
   p = bfd_core_file_failing_command (core_bfd);
   if (p)
Index: inftarg.c
===================================================================
RCS file: /cvs/src/src/gdb/inftarg.c,v
retrieving revision 1.23
diff -p -u -r1.23 inftarg.c
--- inftarg.c	4 Feb 2004 21:49:55 -0000	1.23
+++ inftarg.c	6 May 2004 18:46:41 -0000
@@ -34,7 +34,7 @@
 #include <signal.h>
 #include <sys/types.h>
 #include <fcntl.h>
-
+#include "observer.h"
 #include "gdb_wait.h"
 #include "inflow.h"
 
@@ -232,6 +232,10 @@ child_attach (char *args, int from_tty)
 
     inferior_ptid = pid_to_ptid (pid);
     push_target (&child_ops);
+
+    /* Do this first, before anything has had a chance to query the
+       inferiors symbol table or similar.  */
+    observer_notify_inferior_created (&current_target);
   }
 #endif /* ATTACH_DETACH */
 }
@@ -364,6 +368,7 @@ child_create_inferior (char *exec_file, 
   fork_inferior (exec_file, allargs, env, ptrace_me, ptrace_him, NULL, NULL);
 #endif
   /* We are at the first instruction we care about.  */
+  observer_notify_inferior_created (&current_target);
   /* Pedal to the metal... */
   proceed ((CORE_ADDR) -1, TARGET_SIGNAL_0, 0);
 }
Index: symfile-mem.c
===================================================================
RCS file: /cvs/src/src/gdb/symfile-mem.c,v
retrieving revision 1.1
diff -p -u -r1.1 symfile-mem.c
--- symfile-mem.c	2 May 2004 10:14:01 -0000	1.1
+++ symfile-mem.c	6 May 2004 18:46:41 -0000
@@ -53,6 +53,9 @@
 #include "value.h"
 #include "symfile.h"
 #include "symfile-mem.h"
+#include "observer.h"
+#include "auxv.h"
+#include "elf/common.h"
 
 
 /* Read inferior memory at ADDR to find the header of a loaded object file
@@ -137,6 +140,57 @@ Must use symbol-file or exec-file before
   symbol_file_add_from_memory (templ, addr, from_tty);
 }
 
+/* When ever a new inferior is created, try to load its vsyscall
+   page.  */
+
+struct symbol_file_add_from_memory_args
+{
+  struct bfd *bfd;
+  CORE_ADDR sysinfo_ehdr;
+  int from_tty;
+};
+
+static int
+symbol_file_add_from_memory_wrapper (struct ui_out *uiout, void *data)
+{
+  struct symbol_file_add_from_memory_args *args = data;
+
+  symbol_file_add_from_memory (args->bfd, args->sysinfo_ehdr, args->from_tty);
+  return 0;
+}
+
+static void
+add_vsyscall_page (struct target_ops *target)
+{
+  CORE_ADDR sysinfo_ehdr;
+  
+  if (target_auxv_search (target, AT_SYSINFO_EHDR, &sysinfo_ehdr) > 0
+      && sysinfo_ehdr != (CORE_ADDR) 0)
+    {
+      struct bfd *bfd;
+      struct symbol_file_add_from_memory_args args;
+	
+      if (core_bfd != NULL)
+	bfd = core_bfd;
+      else if (exec_bfd != NULL)
+	bfd = exec_bfd;
+      else
+	/* FIXME: cagney/2004-05-06: Should not require an existing
+	   BFD when trying to create a run-time BFD of the VSYSCALL
+	   page in the inferior.  Unfortunatly that's the current
+	   interface so for the moment bail.  Introducing a
+	   ``bfd_runtime'' (a BFD created using the loaded image) file
+	   format should fix this.  */
+	return;
+      args.bfd = bfd;
+      args.sysinfo_ehdr = sysinfo_ehdr;
+      args.from_tty = 0;
+      if (catch_exceptions (uiout, symbol_file_add_from_memory_wrapper,
+			    &args, NULL, RETURN_MASK_ALL) >= 0)
+	printf_unfiltered ("Loaded system supplied DSO at 0x%s\n",
+			   paddr_nz (sysinfo_ehdr));
+    }
+}
 \f
 void
 _initialize_symfile_mem ()
@@ -148,4 +202,7 @@ Load the symbols out of memory from a dy
 Give an expression for the address of the file's shared object file header.",
            &cmdlist);
 
+  /* Want to know of each new inferior so that it's vsyscall info can
+     be extracted.  */
+  observer_attach_inferior_created (add_vsyscall_page);
 }
Index: doc/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/doc/ChangeLog,v
retrieving revision 1.409
diff -p -u -r1.409 ChangeLog
--- doc/ChangeLog	1 May 2004 16:52:30 -0000	1.409
+++ doc/ChangeLog	6 May 2004 18:46:44 -0000
@@ -1,3 +1,7 @@
+2004-05-06  Andrew Cagney  <cagney@redhat.com>
+
+	* observer.texi (GDB Observers): Document "inferior_created".
+
 2004-05-01  Andrew Cagney  <cagney@redhat.com>
 
 	* gdbint.texinfo (Target Architecture Definition): Delete
Index: doc/observer.texi
===================================================================
RCS file: /cvs/src/src/gdb/doc/observer.texi,v
retrieving revision 1.5
diff -p -u -r1.5 observer.texi
--- doc/observer.texi	30 Apr 2004 07:38:50 -0000	1.5
+++ doc/observer.texi	6 May 2004 18:46:45 -0000
@@ -77,3 +77,11 @@ The inferior has stopped for real.
 @deftypefun void target_changed (struct target_ops *@var{target})
 The target's register contents have changed.
 @end deftypefun
+
+@deftypefun void inferior_created (struct target_ops *@var{objfile})
+@var{GDBN} has just created to a new inferior.  For @samp{run}, it is
+called while the inferior is still stopped at the entry-point
+instruction.  For @samp{attach} and @samp{core}, it is called
+immediatly after opening the inferior (and before any information on
+the inferior has been printed).
+@end deftypefun

Re: [obish?sym;rfa:doc] Wire up vsyscall

[Andrew Cagney]

> At present I know of the following problems:

4.  backtrace changes:

#0  handler (sig=26, info=0xfeed7c50, context=0xfeed7cd0) at 
/home/cygnus/cagney
/PENDING/2004-05-06-add-vsyscall/src/gdb/testsuite/gdb.base/siginfo.c:31
#1  0x0093e440 in __kernel_sigreturn ()
#2  0x0804848a in main () at 
/home/cygnus/cagney/PENDING/2004-05-06-add-vsyscall
/src/gdb/testsuite/gdb.base/siginfo.c:66

vs

#0  handler (sig=26, info=0xfee1ea80, context=0xfee1eb00) at 
/home/cygnus/cagney
/GDB/src/gdb/testsuite/gdb.base/siginfo.c:31
#1  <signal handler called>
#2  0x0804848a in main () at 
/home/cygnus/cagney/GDB/src/gdb/testsuite/gdb.base/

but remember I intend changing the second to:

...
#1 0x1234 in <signal trampoline>
...

Andrew

Re: [obish?sym;rfa:doc] Wire up vsyscall

[Daniel Jacobowitz]

On Thu, May 06, 2004 at 04:54:55PM -0400, Andrew Cagney wrote:
> >At present I know of the following problems:
> 
> 4.  backtrace changes:
> 
> #0  handler (sig=26, info=0xfeed7c50, context=0xfeed7cd0) at 
> /home/cygnus/cagney
> /PENDING/2004-05-06-add-vsyscall/src/gdb/testsuite/gdb.base/siginfo.c:31
> #1  0x0093e440 in __kernel_sigreturn ()
> #2  0x0804848a in main () at 
> /home/cygnus/cagney/PENDING/2004-05-06-add-vsyscall
> /src/gdb/testsuite/gdb.base/siginfo.c:66
> 
> vs
> 
> #0  handler (sig=26, info=0xfee1ea80, context=0xfee1eb00) at 
> /home/cygnus/cagney
> /GDB/src/gdb/testsuite/gdb.base/siginfo.c:31
> #1  <signal handler called>
> #2  0x0804848a in main () at 
> /home/cygnus/cagney/GDB/src/gdb/testsuite/gdb.base/
> 
> but remember I intend changing the second to:
> 
> ...
> #1 0x1234 in <signal trampoline>
> ...

In the meantime, here's the patch from my Debian backport which should
fix this.  Pending a way to indicate 'abnormal frame' status in the
CFI, we don't want to use it; frame_unwind_address_in_block will hit.

-- 
Daniel Jacobowitz

2004-01-25  Daniel Jacobowitz  <drow@mvista.com>

	* i386-tdep.c (i386_gdbarch_init): Check for signal frame first.
	* i386-linux-tdep.c (i386_linux_pc_in_sigtramp): Handle
	__kernel_sigreturn.

Index: gdb-6.1/gdb/i386-linux-tdep.c
===================================================================
--- gdb-6.1.orig/gdb/i386-linux-tdep.c	2004-04-05 13:26:42.000000000 -0400
+++ gdb-6.1/gdb/i386-linux-tdep.c	2004-04-05 13:26:46.000000000 -0400
@@ -227,6 +227,10 @@ i386_linux_pc_in_sigtramp (CORE_ADDR pc,
      exported from the shared C library, so the trampoline may appear to
      be part of the preceding function.  This should always be sigaction,
      __sigaction, or __libc_sigaction (all aliases to the same function).  */
+
+  if (name && strcmp (name, "__kernel_sigreturn") == 0)
+    return 1;
+
   if (name == NULL || strstr (name, "sigaction") != NULL)
     return (i386_linux_sigtramp_start (pc) != 0
 	    || i386_linux_rt_sigtramp_start (pc) != 0);
Index: gdb-6.1/gdb/i386-tdep.c
===================================================================
--- gdb-6.1.orig/gdb/i386-tdep.c	2004-04-05 13:26:42.000000000 -0400
+++ gdb-6.1/gdb/i386-tdep.c	2004-04-05 13:26:46.000000000 -0400
@@ -2013,6 +2013,10 @@ i386_gdbarch_init (struct gdbarch_info i
   /* Helper for function argument information.  */
   set_gdbarch_fetch_pointer_argument (gdbarch, i386_fetch_pointer_argument);
 
+  /* The signal handler might have dwarf2 CFI, via the vsyscall DSO, so check
+     for this first.  */
+  frame_unwind_append_sniffer (gdbarch, i386_sigtramp_frame_sniffer);
+
   /* Hook in the DWARF CFI frame unwinder.  */
   frame_unwind_append_sniffer (gdbarch, dwarf2_frame_sniffer);
 
@@ -2021,7 +2025,6 @@ i386_gdbarch_init (struct gdbarch_info i
   /* Hook in ABI-specific overrides, if they have been registered.  */
   gdbarch_init_osabi (info, gdbarch);
 
-  frame_unwind_append_sniffer (gdbarch, i386_sigtramp_frame_sniffer);
   frame_unwind_append_sniffer (gdbarch, i386_frame_sniffer);
 
   /* If we have a register mapping, enable the generic core file

Re: [obish?sym;rfa:doc] Wire up vsyscall

[Daniel Jacobowitz]

On Thu, May 06, 2004 at 05:48:54PM -0700, Roland McGrath wrote:
> > At present I know of the following problems:
> > 
> > 1. The code assumes ELF support in BFD
> > Per recent BFD posts, I'm fixing this.
> 
> I didn't find what you're referring to in a quick scan of the archives.
> Can you give me a specific URL in the mailing list archive, or an exact
> subject line to search for?  I participated in the discussion about the
> issue, but I don't recall a posting from you proposing changes to address it.

See the binutils list, where Andrew has been reimplementing the
underlying I/O mechanism lately.

> > 3. inferior recycle VS inferior create
> > Because GDB currently fudges things by recycling the inferior at each 
> > run (instead of creating a new fresh inferior) symfile tries to reload 
> > the vsyscall page.  Doesn't do any harm fortunatly.  Having a proper 
> > inferior will fix this.

I don't see how this is different than any other shared library.  I
think we already set a flag that causes the objfile to be discarded
at re-run (OBJF_SHARED) unless this has been lost somewhere?

> I don't know the status of the "catch exec" functionality.
> But if that works at all, it needs to do this observer notification too.
> Otherwise the patch looks good to me modulo the following tiny nits.

It doesn't work.  It's broken by design; I have patches that make it
"work" again, but I never submitted them because both the code and the
interface are so ugly.

> > +	/* FIXME: cagney/2004-05-06: Should not require an existing
> > +	   BFD when trying to create a run-time BFD of the VSYSCALL
> > +	   page in the inferior.  Unfortunatly that's the current
> > +	   interface so for the moment bail.  Introducing a
> > +	   ``bfd_runtime'' (a BFD created using the loaded image) file
> > +	   format should fix this.  */
> > +	return;
> 
> I think there should be an error thrown or at least a warning message here,
> instead of just silent not-doing.  The user can then do "file" before
> "attach" to make the backtraces work.

I agree.

> > +	printf_unfiltered ("Loaded system supplied DSO at 0x%s\n",
> > +			   paddr_nz (sysinfo_ehdr));
> 
> I don't know if printf_unfiltered means this or not, but this output should
> only appear under `set verbose on'.  Also, I would change the message to
> fix the grammar and to be consistent with other gdb messages:
> 
> 	"Reading symbols from system-supplied DSO at 0x%s\n"

Either set verbose, or at least propogate from_tty here.  Right now the
thread_db message is the only thing in startup that disregards from_tty
(I've been meaning to fix this for ages).

-- 
Daniel Jacobowitz

Re: [obish?sym;rfa:doc] Wire up vsyscall

[Andrew Cagney]

On Thu, May 06, 2004 at 04:54:55PM -0400, Andrew Cagney wrote:

>At present I know of the following problems:

4.  backtrace changes:

#0  handler (sig=26, info=0xfeed7c50, context=0xfeed7cd0) at 
/home/cygnus/cagney
/PENDING/2004-05-06-add-vsyscall/src/gdb/testsuite/gdb.base/siginfo.c:31
#1  0x0093e440 in __kernel_sigreturn ()
#2  0x0804848a in main () at 
/home/cygnus/cagney/PENDING/2004-05-06-add-vsyscall
/src/gdb/testsuite/gdb.base/siginfo.c:66

vs

#0  handler (sig=26, info=0xfee1ea80, context=0xfee1eb00) at 
/home/cygnus/cagney
/GDB/src/gdb/testsuite/gdb.base/siginfo.c:31
#1  <signal handler called>
#2  0x0804848a in main () at 
/home/cygnus/cagney/GDB/src/gdb/testsuite/gdb.base/

but remember I intend changing the second to:

...
#1 0x1234 in <signal trampoline>
...


In the meantime, here's the patch from my Debian backport which should
fix this.  Pending a way to indicate 'abnormal frame' status in the
CFI, we don't want to use it; frame_unwind_address_in_block will hit.
2004-01-25  Daniel Jacobowitz  <drow@mvista.com>
Even with the above frame display change, this is needed.  The frame 
needs to identify it's type as SIGTRAMP_FRAME.

Mark?

Andrew

	* i386-tdep.c (i386_gdbarch_init): Check for signal frame first.
	* i386-linux-tdep.c (i386_linux_pc_in_sigtramp): Handle
	__kernel_sigreturn.
Index: gdb-6.1/gdb/i386-linux-tdep.c
===================================================================
--- gdb-6.1.orig/gdb/i386-linux-tdep.c	2004-04-05 13:26:42.000000000 -0400
+++ gdb-6.1/gdb/i386-linux-tdep.c	2004-04-05 13:26:46.000000000 -0400
@@ -227,6 +227,10 @@ i386_linux_pc_in_sigtramp (CORE_ADDR pc,
      exported from the shared C library, so the trampoline may appear to
      be part of the preceding function.  This should always be sigaction,
      __sigaction, or __libc_sigaction (all aliases to the same function).  */
+
+  if (name && strcmp (name, "__kernel_sigreturn") == 0)
+    return 1;
+
   if (name == NULL || strstr (name, "sigaction") != NULL)
     return (i386_linux_sigtramp_start (pc) != 0
 	    || i386_linux_rt_sigtramp_start (pc) != 0);
Index: gdb-6.1/gdb/i386-tdep.c
===================================================================
--- gdb-6.1.orig/gdb/i386-tdep.c	2004-04-05 13:26:42.000000000 -0400
+++ gdb-6.1/gdb/i386-tdep.c	2004-04-05 13:26:46.000000000 -0400
@@ -2013,6 +2013,10 @@ i386_gdbarch_init (struct gdbarch_info i
   /* Helper for function argument information.  */
   set_gdbarch_fetch_pointer_argument (gdbarch, i386_fetch_pointer_argument);
 
+  /* The signal handler might have dwarf2 CFI, via the vsyscall DSO, so check
+     for this first.  */
+  frame_unwind_append_sniffer (gdbarch, i386_sigtramp_frame_sniffer);
+
   /* Hook in the DWARF CFI frame unwinder.  */
   frame_unwind_append_sniffer (gdbarch, dwarf2_frame_sniffer);
 
@@ -2021,7 +2025,6 @@ i386_gdbarch_init (struct gdbarch_info i
   /* Hook in ABI-specific overrides, if they have been registered.  */
   gdbarch_init_osabi (info, gdbarch);
 
-  frame_unwind_append_sniffer (gdbarch, i386_sigtramp_frame_sniffer);
   frame_unwind_append_sniffer (gdbarch, i386_frame_sniffer);
 
   /* If we have a register mapping, enable the generic core file

Re: [obish?sym;rfa:doc] Wire up vsyscall

[Andrew Cagney]

On Thu, May 06, 2004 at 05:48:54PM -0700, Roland McGrath wrote:

> At present I know of the following problems:
> 
> 1. The code assumes ELF support in BFD
> Per recent BFD posts, I'm fixing this.

I didn't find what you're referring to in a quick scan of the archives.
Can you give me a specific URL in the mailing list archive, or an exact
subject line to search for?  I participated in the discussion about the
issue, but I don't recall a posting from you proposing changes to address it.


See the binutils list, where Andrew has been reimplementing the
underlying I/O mechanism lately.

> 3. inferior recycle VS inferior create
> Because GDB currently fudges things by recycling the inferior at each 
> run (instead of creating a new fresh inferior) symfile tries to reload 
> the vsyscall page.  Doesn't do any harm fortunatly.  Having a proper 
> inferior will fix this.


I don't see how this is different than any other shared library.  I
think we already set a flag that causes the objfile to be discarded
at re-run (OBJF_SHARED) unless this has been lost somewhere?
As roland wrote:

I'm not clear on what you mean by "reload the vsyscall page" here.  What
needs to happen on a repeated run is to repeat from scratch all the
vsyscall-page-related work that was done on the first run/attach.  
That is, all of the details can turn out different on each new exec:
whether or not AT_SYSINFO_EHDR is supplied at all; the address of the page;
the contents of the page, i.e. its symbol table and unwind info.
GDB should start from scratch with each new inferior.  While I've 
correctly written this new code that way, the existing symfile code and 
many chunks of GDB are not.  Instead of a nice crisp new inferior, that 
code recycles (re-loads) existing globals containing values left over 
from the previous inferior.

> +	/* FIXME: cagney/2004-05-06: Should not require an existing
> +	   BFD when trying to create a run-time BFD of the VSYSCALL
> +	   page in the inferior.  Unfortunatly that's the current
> +	   interface so for the moment bail.  Introducing a
> +	   ``bfd_runtime'' (a BFD created using the loaded image) file
> +	   format should fix this.  */
> +	return;

I think there should be an error thrown or at least a warning message here,
instead of just silent not-doing.  The user can then do "file" before
"attach" to make the backtraces work.
- you can't throw an error, that aborts the run command

> +	printf_unfiltered ("Loaded system supplied DSO at 0x%s\n",
> +			   paddr_nz (sysinfo_ehdr));

I don't know if printf_unfiltered means this or not, but this output should
only appear under `set verbose on'.  Also, I would change the message to
fix the grammar and to be consistent with other gdb messages:
	"Reading symbols from system-supplied DSO at 0x%s\n"


Either set verbose, or at least propogate from_tty here.
I'll tweak.

Andrew

Re: [obish?sym;rfa:doc] Wire up vsyscall

[Mark Kettenis]

   Date: Mon, 10 May 2004 17:27:51 -0400
   From: Andrew Cagney <cagney@gnu.org>

   > On Thu, May 06, 2004 at 04:54:55PM -0400, Andrew Cagney wrote:
   > 
   >>>> >At present I know of the following problems:
   >>
   >>> 
   >>> 4.  backtrace changes:
   >>> 
   >>> #0  handler (sig=26, info=0xfeed7c50, context=0xfeed7cd0) at 
   >>> /home/cygnus/cagney
   >>> /PENDING/2004-05-06-add-vsyscall/src/gdb/testsuite/gdb.base/siginfo.c:31
   >>> #1  0x0093e440 in __kernel_sigreturn ()
   >>> #2  0x0804848a in main () at 
   >>> /home/cygnus/cagney/PENDING/2004-05-06-add-vsyscall
   >>> /src/gdb/testsuite/gdb.base/siginfo.c:66
   >>> 
   >>> vs
   >>> 
   >>> #0  handler (sig=26, info=0xfee1ea80, context=0xfee1eb00) at 
   >>> /home/cygnus/cagney
   >>> /GDB/src/gdb/testsuite/gdb.base/siginfo.c:31
   >>> #1  <signal handler called>
   >>> #2  0x0804848a in main () at 
   >>> /home/cygnus/cagney/GDB/src/gdb/testsuite/gdb.base/
   >>> 
   >>> but remember I intend changing the second to:
   >>> 
   >>> ...
   >>> #1 0x1234 in <signal trampoline>
   >>> ...
   > 
   > 
   > In the meantime, here's the patch from my Debian backport which should
   > fix this.  Pending a way to indicate 'abnormal frame' status in the
   > CFI, we don't want to use it; frame_unwind_address_in_block will hit.
   > 2004-01-25  Daniel Jacobowitz  <drow@mvista.com>

   Even with the above frame display change, this is needed.  The frame 
   needs to identify it's type as SIGTRAMP_FRAME.

   Mark?

Hmm.  The DWARF CFI in the vsyscall DSO is deliberately generated such
that frame_unwind_in_block does the right thing (there is a nop in
front of __kernel_sigreturn, which is included in the FDE range, such
that us substracting one from the PC will still give us the right
Dwarf CFI).

To what extent do we need to know about SIGTRAMP_FRAME for other
purposes?  I guess we need it to get stepping into/through signal
trampolines working properly, but I'd like to be certain about it.

Mark

Re: [obish?sym;rfa:doc] Wire up vsyscall

[Andrew Cagney]

   > In the meantime, here's the patch from my Debian backport which should
   > fix this.  Pending a way to indicate 'abnormal frame' status in the
   > CFI, we don't want to use it; frame_unwind_address_in_block will hit.
   > 2004-01-25  Daniel Jacobowitz  <drow@mvista.com>
   Even with the above frame display change, this is needed.  The frame 
   needs to identify it's type as SIGTRAMP_FRAME.

   Mark?

Hmm.  The DWARF CFI in the vsyscall DSO is deliberately generated such
that frame_unwind_in_block does the right thing (there is a nop in
front of __kernel_sigreturn, which is included in the FDE range, such
that us substracting one from the PC will still give us the right
Dwarf CFI).
To what extent do we need to know about SIGTRAMP_FRAME for other
purposes?  I guess we need it to get stepping into/through signal
trampolines working properly, but I'd like to be certain about it.
At least that and (see above) frame_unwind_address_in_block.

Andrew

Re: [obish?sym;rfa:doc] Wire up vsyscall

[Daniel Jacobowitz]

On Mon, May 10, 2004 at 11:40:01PM +0200, Mark Kettenis wrote:
>    Date: Mon, 10 May 2004 17:27:51 -0400
>    From: Andrew Cagney <cagney@gnu.org>
> 
>    > On Thu, May 06, 2004 at 04:54:55PM -0400, Andrew Cagney wrote:
>    > 
>    >>>> >At present I know of the following problems:
>    >>
>    >>> 
>    >>> 4.  backtrace changes:
>    >>> 
>    >>> #0  handler (sig=26, info=0xfeed7c50, context=0xfeed7cd0) at 
>    >>> /home/cygnus/cagney
>    >>> /PENDING/2004-05-06-add-vsyscall/src/gdb/testsuite/gdb.base/siginfo.c:31
>    >>> #1  0x0093e440 in __kernel_sigreturn ()
>    >>> #2  0x0804848a in main () at 
>    >>> /home/cygnus/cagney/PENDING/2004-05-06-add-vsyscall
>    >>> /src/gdb/testsuite/gdb.base/siginfo.c:66
>    >>> 
>    >>> vs
>    >>> 
>    >>> #0  handler (sig=26, info=0xfee1ea80, context=0xfee1eb00) at 
>    >>> /home/cygnus/cagney
>    >>> /GDB/src/gdb/testsuite/gdb.base/siginfo.c:31
>    >>> #1  <signal handler called>
>    >>> #2  0x0804848a in main () at 
>    >>> /home/cygnus/cagney/GDB/src/gdb/testsuite/gdb.base/
>    >>> 
>    >>> but remember I intend changing the second to:
>    >>> 
>    >>> ...
>    >>> #1 0x1234 in <signal trampoline>
>    >>> ...
>    > 
>    > 
>    > In the meantime, here's the patch from my Debian backport which should
>    > fix this.  Pending a way to indicate 'abnormal frame' status in the
>    > CFI, we don't want to use it; frame_unwind_address_in_block will hit.
>    > 2004-01-25  Daniel Jacobowitz  <drow@mvista.com>
> 
>    Even with the above frame display change, this is needed.  The frame 
>    needs to identify it's type as SIGTRAMP_FRAME.
> 
>    Mark?
> 
> Hmm.  The DWARF CFI in the vsyscall DSO is deliberately generated such
> that frame_unwind_in_block does the right thing (there is a nop in
> front of __kernel_sigreturn, which is included in the FDE range, such
> that us substracting one from the PC will still give us the right
> Dwarf CFI).

That's not the only place where you have to worry about
frame_unwind_address_in_block.  The other one is in the code that was
interrupted by the signal.  If __kernel_sigreturn appears to be a
normal frame, then we will subtract one to find the block of
__kernel_sigreturn's "caller", which will move us before the beginning
of the function if we got the signal during the first instruction
(which happens in the GDB testsuite, which is how I noticed the
problem).

> To what extent do we need to know about SIGTRAMP_FRAME for other
> purposes?  I guess we need it to get stepping into/through signal
> trampolines working properly, but I'd like to be certain about it.

I'm not sure we'll need it for that also.  Maybe, but no reason jumps
out at me.

-- 
Daniel Jacobowitz

Re: [obish?sym;rfa:doc] Wire up vsyscall

[Daniel Jacobowitz]

On Tue, May 11, 2004 at 12:30:41PM -0400, Andrew Cagney wrote:
> >>To what extent do we need to know about SIGTRAMP_FRAME for other
> >>>purposes?  I guess we need it to get stepping into/through signal
> >>>trampolines working properly, but I'd like to be certain about it.
> >
> >
> >I'm not sure we'll need it for that also.  Maybe, but no reason jumps
> >out at me.
> 
> Er, infrun single-steps through trampolines rather than skipping over them.

Oh right.  This would leave us sitting in the trampoline instead of
automatically skipping them; makes sense.

-- 
Daniel Jacobowitz

Re: [obish?sym;rfa:doc] Wire up vsyscall

[Andrew Cagney]

On Tue, May 11, 2004 at 12:30:41PM -0400, Andrew Cagney wrote:

>>To what extent do we need to know about SIGTRAMP_FRAME for other

>>>purposes?  I guess we need it to get stepping into/through signal
>>>trampolines working properly, but I'd like to be certain about it.

>
>
>I'm not sure we'll need it for that also.  Maybe, but no reason jumps
>out at me.

Er, infrun single-steps through trampolines rather than skipping over them.


Oh right.  This would leave us sitting in the trampoline instead of
automatically skipping them; makes sense.
(note the choice of word - trampoline - there are many types of 
trampoline and for each GDB either steps through it or returns from it)

Another [SIGTRAMP_FRAME] attribute is how to display it when doing a 
backtrace or stop.

Andrew

Re: [obish?sym;rfa:doc] Wire up vsyscall

[Mark Kettenis]

   Date: Tue, 11 May 2004 20:28:17 -0400
   From: Andrew Cagney <cagney@gnu.org>

   Another [SIGTRAMP_FRAME] attribute is how to display it when doing a 
   backtrace or stop.

I'm somewhat reluctant to throw the CFI completely overboard.  How
about extending the DWARF2 unwinder such that we can set the
[SIGTRAMP_FRAME] attribute based on the function's name?

Mark

Revamp sniffer; Was: [obish?sym;rfa:doc] Wire up vsyscall

[Andrew Cagney]

2004-01-25  Daniel Jacobowitz  <drow@mvista.com>

	* i386-tdep.c (i386_gdbarch_init): Check for signal frame first.
	* i386-linux-tdep.c (i386_linux_pc_in_sigtramp): Handle
	__kernel_sigreturn.
[...]
I'm somewhat reluctant to throw the CFI completely overboard.
Yes.

How
about extending the DWARF2 unwinder such that we can set the
[SIGTRAMP_FRAME] attribute based on the function's name?
The frame attributes (tramp et.al.) in many cases are orthogonal to the 
unwind mechanism, but sometimes they are not).

At present the frame sniffer is focused solely on identifying and 
returning an unwinder.  The frame's type being identified as an 
unexpected sid effect.  I think this should be inverted vis:

frame_sniffer () ->
	- attributes
	- unwinder (or NULL, indicating sniff for unwinder)
	- base (or NULL, indicating sniff for base)
	- ...
so while the type is determined up front, other things are optional. 
That way here the tramp code would look like:

  if (name == _kernel_sigreturn)
    if (cfi at address)
      return { attribute=TRAMP; unwinder=cfi_unwinder; base=NULL };
    else
      return { attribute=TRAMP; unwinder=alt_unwinder; base=NULL };
this also opens the way for us doing:

  if (name == solib-trampoline)
    return { attribute=TRAMP|CALLEE; unwinder=NULL; base=NULL }
i.e., it's a trampoline (need to skip it) but let something more generic 
identify the unwinder.

Andrew

Re: [obish?sym;rfa:doc] Wire up vsyscall

[Andrew Cagney]

Eli,

How does the doco here look? (everone appears ok on this part of the 
mechanism).

Andrew
2004-05-10  Andrew Cagney  <cagney@redhat.com>

	* observer.texi (GDB Observers): Document "inferior_created".

Index: doc/observer.texi
===================================================================
RCS file: /cvs/src/src/gdb/doc/observer.texi,v
retrieving revision 1.6
diff -p -u -r1.6 observer.texi
--- doc/observer.texi	7 May 2004 22:51:55 -0000	1.6
+++ doc/observer.texi	17 May 2004 20:05:44 -0000
@@ -82,3 +82,11 @@ The inferior has stopped for real.
 @deftypefun void target_changed (struct target_ops *@var{target})
 The target's register contents have changed.
 @end deftypefun
+
+@deftypefun void inferior_created (struct target_ops *@var{objfile}, int @var{from_tty})
+@var{GDBN} has just created to a new inferior.  For @samp{run}, it is
+called while the inferior is still stopped at the entry-point
+instruction.  For @samp{attach} and @samp{core}, it is called
+immediatly after opening the inferior (and before any information on
+the inferior has been printed).
+@end deftypefun
From kevinb@redhat.com Mon May 17 20:19:00 2004
From: Kevin Buettner <kevinb@redhat.com>
To: gdb-patches@sources.redhat.com
Subject: Re: [obish?sym;rfa:doc] Wire up vsyscall
Date: Mon, 17 May 2004 20:19:00 -0000
Message-id: <20040517131914.332fa347@saguaro>
References: <409A8C2A.2010605@gnu.org> <40A91C36.7000900@gnu.org>
X-SW-Source: 2004-05/msg00506.html
Content-length: 530

On Mon, 17 May 2004 16:10:30 -0400
Andrew Cagney <cagney@gnu.org> wrote:

> +@deftypefun void inferior_created (struct target_ops *@var{objfile}, int @var{from_tty})
> +@var{GDBN} has just created to a new inferior.  For @samp{run}, it is
> +called while the inferior is still stopped at the entry-point
> +instruction.  For @samp{attach} and @samp{core}, it is called
> +immediatly after opening the inferior (and before any information on
> +the inferior has been printed).
> +@end deftypefun

s/immediatly/immediately/

Kevin

Re: [obish?sym;rfa:doc] Wire up vsyscall

[Eli Zaretskii]

> Date: Mon, 17 May 2004 13:19:14 -0700
> From: Kevin Buettner <kevinb@redhat.com>
> 
> On Mon, 17 May 2004 16:10:30 -0400
> Andrew Cagney <cagney@gnu.org> wrote:
> 
> > +@deftypefun void inferior_created (struct target_ops *@var{objfile}, int @var{from_tty})
> > +@var{GDBN} has just created to a new inferior.  For @samp{run}, it is
> > +called while the inferior is still stopped at the entry-point
> > +instruction.  For @samp{attach} and @samp{core}, it is called
> > +immediatly after opening the inferior (and before any information on
> > +the inferior has been printed).
> > +@end deftypefun
> 
> s/immediatly/immediately/

Oops, missed the original message.

Andrew, in addition to the typo caught by Kevin, please correct the
wording some more, viz.:

 - "@var{GDBN}" should be "@value{GDBN}".

 - "has just created to a new inferior" is unclear and incorrect
   English.  Sounds like some word is missing or should be deleted.

 - The "it" in "For @samp{run}", it is called ..." is not very clear;
   I think "For @samp{run}, this observer is called ..." is better.

 - "after opening the inferior" is possibly inaccurate: GDB doesn't
   really ``open'' the inferior, right?

Re: [obish?sym;rfa:doc] Wire up vsyscall

[Andrew Cagney]

Oops, missed the original message.

Andrew, in addition to the typo caught by Kevin, please correct the
wording some more, viz.:
 - "@var{GDBN}" should be "@value{GDBN}".

 - "has just created to a new inferior" is unclear and incorrect
   English.  Sounds like some word is missing or should be deleted.
 - The "it" in "For @samp{run}", it is called ..." is not very clear;
   I think "For @samp{run}, this observer is called ..." is better.
 - "after opening the inferior" is possibly inaccurate: GDB doesn't
   really ``open'' the inferior, right?
I think this is better ...

@deftypefun void inferior_created (struct target_ops *@var{objfile}, int 
@var{from_tty})
@value{GDBN} has just connected to an inferior.  For @samp{run}, this
observer is called while the inferior is still stopped at the
entry-point instruction.  For @samp{attach} and @samp{core}, this
observer is called immediately after connecting to the inferior, and
before any information on the inferior has been printed).
@end deftypefun

... but the ``connecting to the inferior'' near the end is struggling a bit.

Andrew

Re: [obish?sym;rfa:doc] Wire up vsyscall

[Eli Zaretskii]

> Date: Tue, 18 May 2004 16:09:44 -0400
> From: Andrew Cagney <cagney@gnu.org>
> 
> I think this is better ...
> 
> @deftypefun void inferior_created (struct target_ops *@var{objfile}, int 
> @var{from_tty})
> @value{GDBN} has just connected to an inferior.  For @samp{run}, this
> observer is called while the inferior is still stopped at the
> entry-point instruction.  For @samp{attach} and @samp{core}, this
> observer is called immediately after connecting to the inferior, and
> before any information on the inferior has been printed).
> @end deftypefun

Yes, much better.

Of course, following RMS's advice to avoid passive, "@value{GDBN}
calls this observer..." instead of "this observer is called..." would
be even better ;-) 

> ... but the ``connecting to the inferior'' near the end is struggling a bit.

The only thing that bothers me is whether ``connecting'' is something
that the reader will instantly understand.  Other than that, I don't
see any problems here.

Re: [obish?sym;rfa:doc] Wire up vsyscall

[Andrew Cagney]

So:

@deftypefun void inferior_created (struct target_ops *@var{objfile}, int 
@var{from_tty})
@value{GDBN} has just connected to an inferior.  For @samp{run},
@value{GDBN} calls this observer while the inferior is still stopped
at the entry-point instruction.  For @samp{attach} and @samp{core},
@value{GDBN} calls this observer immediately after connecting to the
inferior, and before any information on the inferior has been
printed).
@end deftypefun

Re: [obish?sym;rfa:doc] Wire up vsyscall

[Eli Zaretskii]

> Date: Wed, 19 May 2004 10:46:58 -0400
> From: Andrew Cagney <cagney@gnu.org>
> 
> So:
> 
> @deftypefun void inferior_created (struct target_ops *@var{objfile}, int 
> @var{from_tty})
> @value{GDBN} has just connected to an inferior.  For @samp{run},
> @value{GDBN} calls this observer while the inferior is still stopped
> at the entry-point instruction.  For @samp{attach} and @samp{core},
> @value{GDBN} calls this observer immediately after connecting to the
> inferior, and before any information on the inferior has been
> printed).
> @end deftypefun

Perfect (if you remove that extra parent after "printed").

Thanks.

Re: [obish?sym;rfa:doc] Wire up vsyscall

[Eli Zaretskii]

> Date: Wed, 19 May 2004 22:10:31 +0200
> From: "Eli Zaretskii" <eliz@gnu.org>
> 
> Perfect (if you remove that extra parent after "printed").
                                    ^^^^^^
Should have been "paren", of course ;-)

Re: Revamp sniffer; Was: [obish?sym;rfa:doc] Wire up vsyscall

[Andrew Cagney]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2418 bytes --]

Mark,

Want do you want to do here?  Commit daniel's patch for the short term 
while I clean this up proper over the medium term (appending it to the 
vsyscall list)?

Andrew
---  Begin Message  ---

From : Andrew Cagney <cagney at gnu dot org>
To : Mark Kettenis <kettenis at chello dot nl>
Cc : drow at false dot org, gdb-patches at sources dot redhat dot com, roland at redhat dot com
Date : Mon, 17 May 2004 13:14:48 -0400
Subject : Revamp sniffer; Was: [obish?sym;rfa:doc] Wire up vsyscall
Delivery-date : Mon, 17 May 2004 14:17:10 -0400
Envelope-to : cagney@gnu.org
References : < 409A8C2A.2010605@gnu.org > < 409AA61F.8050807@gnu.org > < 20040507012549.GA30182@nevyn.them.org > < 409FF3D7.5000106@gnu.org > < 200405102140.i4ALe13A001092@elgar.kettenis.dyndns.org > < 20040511145344.GA15501@nevyn.them.org > < 40A0FFB1.8030407@gnu.org > < 20040511172619.GA19487@nevyn.them.org > < 40A16FA1.9060506@gnu.org > < 200405152057.i4FKvdt2000817@elgar.kettenis.dyndns.org >

2004-01-25  Daniel Jacobowitz  <drow@mvista.com>

	* i386-tdep.c (i386_gdbarch_init): Check for signal frame first.
	* i386-linux-tdep.c (i386_linux_pc_in_sigtramp): Handle
	__kernel_sigreturn.
[...]
I'm somewhat reluctant to throw the CFI completely overboard.
Yes.

How
about extending the DWARF2 unwinder such that we can set the
[SIGTRAMP_FRAME] attribute based on the function's name?
The frame attributes (tramp et.al.) in many cases are orthogonal to the 
unwind mechanism, but sometimes they are not).

At present the frame sniffer is focused solely on identifying and 
returning an unwinder.  The frame's type being identified as an 
unexpected sid effect.  I think this should be inverted vis:

frame_sniffer () ->
	- attributes
	- unwinder (or NULL, indicating sniff for unwinder)
	- base (or NULL, indicating sniff for base)
	- ...
so while the type is determined up front, other things are optional. 
That way here the tramp code would look like:

  if (name == _kernel_sigreturn)
    if (cfi at address)
      return { attribute=TRAMP; unwinder=cfi_unwinder; base=NULL };
    else
      return { attribute=TRAMP; unwinder=alt_unwinder; base=NULL };
this also opens the way for us doing:

  if (name == solib-trampoline)
    return { attribute=TRAMP|CALLEE; unwinder=NULL; base=NULL }
i.e., it's a trampoline (need to skip it) but let something more generic 
identify the unwinder.

Andrew




---  End Message  ---

Re: Revamp sniffer; Was: [obish?sym;rfa:doc] Wire up vsyscall

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 5 bytes --]

ping

[-- Attachment #2: Attached Message --]
[-- Type: message/rfc822, Size: 9162 bytes --]

[-- Attachment #2.1.1: Type: text/plain, Size: 175 bytes --]

Mark,

Want do you want to do here?  Commit daniel's patch for the short term 
while I clean this up proper over the medium term (appending it to the 
vsyscall list)?

Andrew

[-- Attachment #2.1.2: Attached Message --]
[-- Type: message/rfc822, Size: 4991 bytes --]

From: Andrew Cagney <cagney@gnu.org>
To: Mark Kettenis <kettenis@chello.nl>
Cc: drow@false.org, gdb-patches@sources.redhat.com, roland@redhat.com
Subject: Revamp sniffer; Was: [obish?sym;rfa:doc] Wire up vsyscall
Date: Mon, 17 May 2004 13:14:48 -0400
Message-ID: <40A8F308.9040900@gnu.org>

> 2004-01-25  Daniel Jacobowitz  <drow@mvista.com>
> 
> 	* i386-tdep.c (i386_gdbarch_init): Check for signal frame first.
> 	* i386-linux-tdep.c (i386_linux_pc_in_sigtramp): Handle
> 	__kernel_sigreturn.
> 
[...]
> I'm somewhat reluctant to throw the CFI completely overboard.

Yes.

> How
> about extending the DWARF2 unwinder such that we can set the
> [SIGTRAMP_FRAME] attribute based on the function's name?

The frame attributes (tramp et.al.) in many cases are orthogonal to the 
unwind mechanism, but sometimes they are not).

At present the frame sniffer is focused solely on identifying and 
returning an unwinder.  The frame's type being identified as an 
unexpected sid effect.  I think this should be inverted vis:

frame_sniffer () ->
	- attributes
	- unwinder (or NULL, indicating sniff for unwinder)
	- base (or NULL, indicating sniff for base)
	- ...

so while the type is determined up front, other things are optional. 
That way here the tramp code would look like:

   if (name == _kernel_sigreturn)
     if (cfi at address)
       return { attribute=TRAMP; unwinder=cfi_unwinder; base=NULL };
     else
       return { attribute=TRAMP; unwinder=alt_unwinder; base=NULL };

this also opens the way for us doing:

   if (name == solib-trampoline)
     return { attribute=TRAMP|CALLEE; unwinder=NULL; base=NULL }

i.e., it's a trampoline (need to skip it) but let something more generic 
identify the unwinder.

Andrew

Re: Revamp sniffer; Was: [obish?sym;rfa:doc] Wire up vsyscall

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 1750 bytes --]

Ok,

I just figured out why amd64 GNU/Linux signal trampolines don't work 
correctly and the reason is scary!

After signal delivery (e.x., sigaltstack testcase) the stack looks like:

   sentinel: SENTINEL_FRAME
   catcher: NORMAL_FRAME, dwarf2 unwinder
   <sigtramp>: SIGTRAMP_FRAME, sigtramp unwinder
   thrower: NORMAL_FRAME, dwarf2 unwinder

while later after catcher has "finish"ed it looks like:

   sentinel: SENTINEL_FRAME
   __restore_rt: NORMAL_FRAME, dwarf2 unwinder
   <corrupt stack>

Notice how the first time round the trampoline was correctly identified 
but not the second.  What happened?

In the first case:

- linux jumps to catcher setting it's return address to <sigtramp>'s 
entry point
- gdb correctly identifies catcher as a callee or normal frame
- gdb goes to unwind catcher and starts sniffing <sigtramp>
- the dwarf2 sniffer, when sniffing <sigtramp>, asks for that frame's 
address-in-block, since the catcher is a normal frame, that translates 
to <catcher's-return-address>-1
- <catcher's-return-address>-1 translates into <sigtramp>-1
- since <sigtramp>-1 doesn't have any CFI (lucky eh?) dwarf2 dismisses 
the frame
- the sigtramp unwinder gets to take a wiff and identifies the frame

while in the second case (no catcher):

- the dwarf2 sniffer, when sniffing <sigtramp>, asks for that frame's 
address-in-block, since the next frame is a sentinel that translates 
into <catcher's-return-address>
- <catcher's-return-address> translates into <sigtramp>
- <sigtramp> does have CFI so dwarf2 claims the frame marking it 
"normal", outch!

So for the moment, I think the sigtramp frame should always be sniffed 
before the dwarf2 frame.  We can revisit the possability of modifing 
dwarf2 later, much later.

Andrew

[-- Attachment #2: Attached Message --]
[-- Type: message/rfc822, Size: 13206 bytes --]

[-- Attachment #2.1.1: Type: text/plain, Size: 5 bytes --]

ping

[-- Attachment #2.1.2: Attached Message --]
[-- Type: message/rfc822, Size: 9162 bytes --]

[-- Attachment #2.1.2.1.1: Type: text/plain, Size: 175 bytes --]

Mark,

Want do you want to do here?  Commit daniel's patch for the short term 
while I clean this up proper over the medium term (appending it to the 
vsyscall list)?

Andrew

[-- Attachment #2.1.2.1.2: Attached Message --]
[-- Type: message/rfc822, Size: 4991 bytes --]

From: Andrew Cagney <cagney@gnu.org>
To: Mark Kettenis <kettenis@chello.nl>
Cc: drow@false.org, gdb-patches@sources.redhat.com, roland@redhat.com
Subject: Revamp sniffer; Was: [obish?sym;rfa:doc] Wire up vsyscall
Date: Mon, 17 May 2004 13:14:48 -0400
Message-ID: <40A8F308.9040900@gnu.org>

> 2004-01-25  Daniel Jacobowitz  <drow@mvista.com>
> 
> 	* i386-tdep.c (i386_gdbarch_init): Check for signal frame first.
> 	* i386-linux-tdep.c (i386_linux_pc_in_sigtramp): Handle
> 	__kernel_sigreturn.
> 
[...]
> I'm somewhat reluctant to throw the CFI completely overboard.

Yes.

> How
> about extending the DWARF2 unwinder such that we can set the
> [SIGTRAMP_FRAME] attribute based on the function's name?

The frame attributes (tramp et.al.) in many cases are orthogonal to the 
unwind mechanism, but sometimes they are not).

At present the frame sniffer is focused solely on identifying and 
returning an unwinder.  The frame's type being identified as an 
unexpected sid effect.  I think this should be inverted vis:

frame_sniffer () ->
	- attributes
	- unwinder (or NULL, indicating sniff for unwinder)
	- base (or NULL, indicating sniff for base)
	- ...

so while the type is determined up front, other things are optional. 
That way here the tramp code would look like:

   if (name == _kernel_sigreturn)
     if (cfi at address)
       return { attribute=TRAMP; unwinder=cfi_unwinder; base=NULL };
     else
       return { attribute=TRAMP; unwinder=alt_unwinder; base=NULL };

this also opens the way for us doing:

   if (name == solib-trampoline)
     return { attribute=TRAMP|CALLEE; unwinder=NULL; base=NULL }

i.e., it's a trampoline (need to skip it) but let something more generic 
identify the unwinder.

Andrew

Re: Revamp sniffer; Was: [obish?sym;rfa:doc] Wire up vsyscall

[Roland McGrath]

I don't have all the background on gdb internals here, but I think I
understand what you've described.

On the x86, the same issue has been considered.  The signal trampoline code
provided by the kernel in the vsyscall DSO (which is what gets used with
current glibc on kernels supporting it) has CFI that starts one byte before
the actual trampoline code.  This comment is from the
arch/i386/kernel/vsyscall-sigreturn.S, which supplies the handwritten CFI
data in Linux 2.6 kernels:

	/* HACK: The dwarf2 unwind routines will subtract 1 from the
	   return address to get an address in the middle of the
	   presumed call instruction.  Since we didn't get here via
	   a call, we need to include the nop before the real start
	   to make up for it.  */

In the x86-64 case, there is no kernel-supplied trampoline and current
glibc supplies one and gives it CFI.  OTOH, on the x86 on kernels that do
not supply the vsyscall DSO, glibc supplies the trampoline and it provides
no CFI for it, so whatever traditional name-matching or disassembly magic
gdb does takes place instead.  

If I understood your description correctly, the source of the current
problem scenario is that glibc supplies CFI for the trampoline code
(__restore_rt), but that CFI is not written to match how the peculiar frame
will look.  So, glibc could change its CFI to use the same hack that the
x86 kernel CFI uses for the analogous code, or it could omit that CFI
entirely and expect gdb to recognize the name and/or instruction sequence.

My inclination is to omit the CFI because that's how it is on x86 in the
analogous case.  (If in future x86-64 has kernel-supplied trampoline code,
we expect it will be in the form of a vsyscall DSO that supplies CFI via
existing glibc support, as on x86.  In that case, the CFI will use the same
sort of hack as the x86 CFI does.)  Would that make things better?


Thanks,
Roland

Re: Revamp sniffer; Was: [obish?sym;rfa:doc] Wire up vsyscall

[Andrew Cagney]

> If I understood your description correctly, the source of the current
> problem scenario is that glibc supplies CFI for the trampoline code
> (__restore_rt), but that CFI is not written to match how the peculiar frame
> will look.  So, glibc could change its CFI to use the same hack that the
> x86 kernel CFI uses for the analogous code, or it could omit that CFI
> entirely and expect gdb to recognize the name and/or instruction sequence.
> 
> My inclination is to omit the CFI because that's how it is on x86 in the
> analogous case.  (If in future x86-64 has kernel-supplied trampoline code,
> we expect it will be in the form of a vsyscall DSO that supplies CFI via
> existing glibc support, as on x86.  In that case, the CFI will use the same
> sort of hack as the x86 CFI does.)  Would that make things better?

Ah!  I guess it would help.  GDB will still need to do something more 
immediate though.

Andrew

Re: Revamp sniffer; Was: [obish?sym;rfa:doc] Wire up vsyscall

[Roland McGrath]

> > If I understood your description correctly, the source of the current
> > problem scenario is that glibc supplies CFI for the trampoline code
> > (__restore_rt), but that CFI is not written to match how the peculiar frame
> > will look.  So, glibc could change its CFI to use the same hack that the
> > x86 kernel CFI uses for the analogous code, or it could omit that CFI
> > entirely and expect gdb to recognize the name and/or instruction sequence.
> > 
> > My inclination is to omit the CFI because that's how it is on x86 in the
> > analogous case.  (If in future x86-64 has kernel-supplied trampoline code,
> > we expect it will be in the form of a vsyscall DSO that supplies CFI via
> > existing glibc support, as on x86.  In that case, the CFI will use the same
> > sort of hack as the x86 CFI does.)  Would that make things better?
> 
> Ah!  I guess it would help.  GDB will still need to do something more 
> immediate though.

I guess it can do the magic name-matching first and ignore CFI if it matches,
but I don't really have anything to add to the gdb part of the discussion.

Which change would gdb people prefer in glibc?  No CFI, or CFI that is
fixed with the kludge to match one byte before the entry point?


Thanks,
Roland

Re: Revamp sniffer; Was: [obish?sym;rfa:doc] Wire up vsyscall

[Mark Kettenis]

   Date: Thu, 24 Jun 2004 13:59:04 -0700
   Content-Type: text/plain; charset=us-ascii

   I guess it can do the magic name-matching first and ignore CFI if
   it matches, but I don't really have anything to add to the gdb part
   of the discussion.

   Which change would gdb people prefer in glibc?  No CFI, or CFI that
   is fixed with the kludge to match one byte before the entry point?

I'd prefer the one-byte-before kludge, since it's consistent with the
vsyscall stuff on i386.

Mark