[branch patch] core files as symfiles

[branch patch] core files as symfiles

[Roland McGrath]

The following patch causes core files to be implicitly read as symbol files
as well.  Using this along with my previous dwarf-frame.c patch vs current
kettenis_i386newframe-20030419-branch gdb, on Linux 2.5.69 on x86 the
backtrace of a thread in a system call from a core dump Just Works.

I've included the trivial symfile.c patch that was in with my dwarf-frame.c
patch again here too, since it's required for the corelow.c patch to work
and these patches work (but cause nothing interesting to happen)
independent of the dwarf-frame.c changes.


Thanks,
Roland


2003-05-12  Roland McGrath  <roland@redhat.com>

	* corelow.c (core_objfile): New variable.
	(core_close): If core_objfile is set, call free_objfile on it.
	(core_open): Open the core file with add_symbol_file, and save
	the struct objfile pointer in core_objfile.

	* symfile.c (symfile_bfd_open): Try bfd_check_format with bfd_core
	if bfd_object fails.

Index: corelow.c
===================================================================
RCS file: /cvs/src/src/gdb/corelow.c,v
retrieving revision 1.29
diff -B -p -u -r1.29 corelow.c
--- corelow.c	18 Jan 2003 15:55:51 -0000	1.29
+++ corelow.c	13 May 2003 02:57:07 -0000
@@ -38,6 +38,7 @@
 #include "gdbthread.h"
 #include "regcache.h"
 #include "symfile.h"
+#include "objfiles.h"
 #include <readline/readline.h>
 
 #ifndef O_BINARY
@@ -50,6 +51,8 @@
 
 static struct core_fns *core_file_fns = NULL;
 
+static struct objfile *core_objfile = NULL;
+
 /* The core_fns for a core file handler that is prepared to read the core
    file currently open on core_bfd. */
 
@@ -200,6 +203,8 @@ core_close (int quitting)
 	warning ("cannot close \"%s\": %s",
 		 name, bfd_errmsg (bfd_get_error ()));
       xfree (name);
+      if (core_objfile && core_objfile->obfd == core_bfd)
+	core_objfile->obfd = NULL;
       core_bfd = NULL;
       if (core_ops.to_sections)
 	{
@@ -209,6 +214,12 @@ core_close (int quitting)
 	}
     }
   core_vec = NULL;
+
+  if (core_objfile)
+    {
+      free_objfile (core_objfile);
+      core_objfile = NULL;
+    }
 }
 
 static void
@@ -263,6 +274,7 @@ core_open (char *filename, int from_tty)
   struct cleanup *old_chain;
   char *temp;
   bfd *temp_bfd;
+  struct objfile *temp_objfile;
   int ontop;
   int scratch_chan;
 
@@ -274,23 +286,42 @@ core_open (char *filename, int from_tty)
 	     : "No core file specified.");
     }
 
-  filename = tilde_expand (filename);
-  if (filename[0] != '/')
-    {
-      temp = concat (current_directory, "/", filename, NULL);
-      xfree (filename);
-      filename = temp;
-    }
+  old_chain = NULL;
 
-  old_chain = make_cleanup (xfree, filename);
+  /* Try to open the core file as a symbol file.  We do this first
+     so we can reuse the same bfd that the objfile points to.  */
+  temp_objfile = symbol_file_add (filename, 0, NULL, 0, 0);
+  if (temp_objfile != NULL)
+    old_chain = make_cleanup ((void (*) (void *)) free_objfile, temp_objfile);
 
-  scratch_chan = open (filename, O_BINARY | ( write_files ? O_RDWR : O_RDONLY ), 0);
-  if (scratch_chan < 0)
-    perror_with_name (filename);
+  if (temp_objfile != NULL && !write_files)
+    temp_bfd = temp_objfile->obfd;
+  else
+    {
+      /* symbol_file_add did the expansion and opening for us if it worked.  */
+
+      filename = tilde_expand (filename);
+      if (filename[0] != '/')
+	{
+	  temp = concat (current_directory, "/", filename, NULL);
+	  xfree (filename);
+	  filename = temp;
+	}
 
-  temp_bfd = bfd_fdopenr (filename, gnutarget, scratch_chan);
-  if (temp_bfd == NULL)
-    perror_with_name (filename);
+      if (old_chain == NULL)
+	old_chain = make_cleanup (xfree, filename);
+      else
+	make_cleanup (xfree, filename);
+
+      scratch_chan = open (filename,
+			   O_BINARY | (write_files ? O_RDWR : O_RDONLY), 0);
+      if (scratch_chan < 0)
+	perror_with_name (filename);
+
+      temp_bfd = bfd_fdopenr (filename, gnutarget, scratch_chan);
+      if (temp_bfd == NULL)
+	perror_with_name (filename);
+    }
 
   if (!bfd_check_format (temp_bfd, bfd_core) &&
       !gdb_check_format (temp_bfd))
@@ -299,7 +330,8 @@ core_open (char *filename, int from_tty)
       /* FIXME: should be checking for errors from bfd_close (for one thing,
          on error it does not free all the storage associated with the
          bfd).  */
-      make_cleanup_bfd_close (temp_bfd);
+      if (temp_objfile == NULL || temp_bfd != temp_objfile->obfd)
+	make_cleanup_bfd_close (temp_bfd);
       error ("\"%s\" is not a core dump: %s",
 	     filename, bfd_errmsg (bfd_get_error ()));
     }
@@ -308,6 +340,7 @@ core_open (char *filename, int from_tty)
 
   discard_cleanups (old_chain);	/* Don't free filename any more */
   unpush_target (&core_ops);
+  core_objfile = temp_objfile;
   core_bfd = temp_bfd;
   old_chain = make_cleanup (core_close_cleanup, 0 /*ignore*/);
 
--- symfile.c.~1.93.~	Fri Apr  4 14:29:12 2003
+++ symfile.c	Mon May 12 02:04:07 2003
@@ -1338,7 +1338,9 @@ symfile_bfd_open (char *name)
     }
   sym_bfd->cacheable = 1;
 
-  if (!bfd_check_format (sym_bfd, bfd_object))
+  if (!bfd_check_format (sym_bfd, bfd_object)
+      /* Yes, Virginia, core files can have symbols!  */
+      && !bfd_check_format (sym_bfd, bfd_core))
     {
       /* FIXME: should be checking for errors from bfd_close (for one thing,
          on error it does not free all the storage associated with the

Re: [branch patch] core files as symfiles

[Mark Kettenis]

   Date: Mon, 12 May 2003 19:58:57 -0700
   From: Roland McGrath <roland@redhat.com>

   The following patch causes core files to be implicitly read as symbol files
   as well.  Using this along with my previous dwarf-frame.c patch vs current
   kettenis_i386newframe-20030419-branch gdb, on Linux 2.5.69 on x86 the
   backtrace of a thread in a system call from a core dump Just Works.

   I've included the trivial symfile.c patch that was in with my dwarf-frame.c
   patch again here too, since it's required for the corelow.c patch to work
   and these patches work (but cause nothing interesting to happen)
   independent of the dwarf-frame.c changes.

The patch looks fine to me, although I'd like to see the opinion of
another GDB developer who's more familiar with this part of the code.
Since this code doesn't depend on any other changes in the
i386newframe branch, perhaps this should go into mainline, and we can
simply merge it into the branch from there.  If you think so, please
resubmit this without the reference to "branch" in the subject.

Mark

Re: [branch patch] core files as symfiles

[Daniel Jacobowitz]

On Thu, May 15, 2003 at 11:57:51PM +0200, Mark Kettenis wrote:
>    Date: Mon, 12 May 2003 19:58:57 -0700
>    From: Roland McGrath <roland@redhat.com>
> 
>    The following patch causes core files to be implicitly read as symbol files
>    as well.  Using this along with my previous dwarf-frame.c patch vs current
>    kettenis_i386newframe-20030419-branch gdb, on Linux 2.5.69 on x86 the
>    backtrace of a thread in a system call from a core dump Just Works.
> 
>    I've included the trivial symfile.c patch that was in with my dwarf-frame.c
>    patch again here too, since it's required for the corelow.c patch to work
>    and these patches work (but cause nothing interesting to happen)
>    independent of the dwarf-frame.c changes.
> 
> The patch looks fine to me, although I'd like to see the opinion of
> another GDB developer who's more familiar with this part of the code.

I can't claim to be familiar with this code, but the patch also looks
reasonable to me.

> Since this code doesn't depend on any other changes in the
> i386newframe branch, perhaps this should go into mainline, and we can
> simply merge it into the branch from there.  If you think so, please
> resubmit this without the reference to "branch" in the subject.

I think this would be a good idea.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [branch patch] core files as symfiles

[Andrew Cagney]

>> Did we settle on this being the solution though?

Not as far as I know.

> I think the
>> /proc/PID/auxv approach is a bit cleaner. 

Yep.  More importantly it solves the general case.

> We're talking about core files, so what you mean is an NT_AUXV note that
> would give the same information that /proc/PID/auxv would give for a live
> process.  It remains to see whether the auxv approach will be accepted in
> the kernel.  

Perhaphs someone should find out.  What's the best mailing list?

> If that is done, then the corelow.c patch will be superfluous.

Right.  And I think both GDB and the Linux kernel will be the better for it.

Andrew

Re: [branch patch] core files as symfiles

[Roland McGrath]

> Did we settle on this being the solution though? I think the
> /proc/PID/auxv approach is a bit cleaner. 

We're talking about core files, so what you mean is an NT_AUXV note that
would give the same information that /proc/PID/auxv would give for a live
process.  It remains to see whether the auxv approach will be accepted in
the kernel.  

If that is done, then the corelow.c patch will be superfluous.  I would
like feedback on the implementation issue of this patch regardless.  That
is, if anyone sees any potential problems with adding the core bfd as a
symfile (AFAIK it's a no-op to all cases but the new vsyscall issues), or
with my specific code changes.  Then I can address any nits, and verify
that it is a safe change for gdb generically speaking.  It is useful to
know ASAP that this very simple change works acceptably, even if it turns
out not to be necessary in the end.

Using the core file's phdrs (which is what happens implicitly using this
patch) has the benefit that it works now, i.e. with kernel support already
merged into Linux 2.5, and with gdb code I have already finished enough to
be in submittable form.  If the generic auxv exporting support is not
accepted in the kernel, then it is quite likely that some other way to know
the vsyscall DSO address in a live process (e.g. via /proc/PID/maps) *will*
be accepted, but no other core dump changes will go in and the existing
phdrs will be the only method for the core dump case.


Thanks,
Roland