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] JIT read_debug_info callback takes target symfile addr


Hi,

The attached patch makes an incompatible change to the JIT reader
interface to pass the target address of the symfile to the
read_debug_info, instead of eagerly copying the symfile to GDB and
passing the local address to the .so.  In many cases this allows the
target to be simpler, as in many cases it's not necessary to allocate a
symfile object at all.  In the case where you do need the memory in the
GDB address space, well the target_read() callback is there for you.

I have a patch coming that will allow JIT readers to be implemented in
Guile.  It uses the JIT reader interface, though loaded from an
extension instead of a shared library.  This patch would help extensions
to be able to do JIT unwinding, as extensions have a much richer
interface to GDB and in particular can query types and sizes from the
inferior, without having to hard-code them from the reader side.  What
you want is the target address, not the target's memory.

WDYT?

>From 983216b213af8c2cf5853a814d2559062018fce4 Mon Sep 17 00:00:00 2001
From: Andy Wingo <wingo@igalia.com>
Date: Mon, 23 Feb 2015 15:06:09 +0100
Subject: [PATCH] JIT read_debug_info callback takes target symfile address

gdb/ChangeLog:
2015-02-23  Andy Wingo  <wingo@igalia.com>

	* NEWS: Announce JIT reader interface change.
	* jit-reader.in (GDB_READER_INTERFACE_VERSION): Bump to 2.
	(gdb_read_debug_info): Update typedef to express the memory as a
	target address, and update the comment.
	* jit.c (jit_reader_try_read_symtab): Don't read the symfile from
	target memory.  The JIT reader can do that if it likes using the
	gdb_symbol_callbacks structure.

gdb/testsuite/ChangeLog:
2015-02-23  Andy Wingo  <wingo@igalia.com>

	* gdb.base/jitreader.c (read_debug_info): Update for changes in
	JIT reader interface and jithost.c test.
	* gdb.base/jithost.c (main): No need to allocate symfile objects
	with the new GDB_READER_INTERFACE_VERSION.
	* gdb.base/jithost.h: Removed.
---
 gdb/ChangeLog                      | 10 ++++++++++
 gdb/NEWS                           |  9 +++++++++
 gdb/jit-reader.in                  | 15 +++++++++------
 gdb/jit.c                          | 34 +++++++++++-----------------------
 gdb/testsuite/ChangeLog            |  8 ++++++++
 gdb/testsuite/gdb.base/jithost.c   | 11 +++--------
 gdb/testsuite/gdb.base/jithost.h   | 27 ---------------------------
 gdb/testsuite/gdb.base/jitreader.c | 22 +++++++++++++---------
 8 files changed, 63 insertions(+), 73 deletions(-)
 delete mode 100644 gdb/testsuite/gdb.base/jithost.h

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6901e5a..40dc8e7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2015-02-23  Andy Wingo  <wingo@igalia.com>
+
+	* NEWS: Announce JIT reader interface change.
+	* jit-reader.in (GDB_READER_INTERFACE_VERSION): Bump to 2.
+	(gdb_read_debug_info): Update typedef to express the memory as a
+	target address, and update the comment.
+	* jit.c (jit_reader_try_read_symtab): Don't read the symfile from
+	target memory.  The JIT reader can do that if it likes using the
+	gdb_symbol_callbacks structure.
+
 2015-02-20  Andy Wingo  <wingo@igalia.com>
 
 	* guile/scm-value.c (gdbscm_value_dynamic_type): Fix typo in which
diff --git a/gdb/NEWS b/gdb/NEWS
index b79b162..2438c60 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,15 @@
 
 *** Changes since GDB 7.9
 
+* The JIT debug info reader interface has changed incompatibly: the
+  "gdb_read_debug_info" callback now receives the symfile_addr argument
+  in the target address space, not GDB's address space.
+  Correspondingly, the GDB_READER_INTERFACE_VERSION has been bumped to
+  2, which should cause any attempt to load an existing jit-reader
+  shared object to fail.  Likewise the gdb_read_debug_info typedef has
+  changed, so simple recompilation will cause an error, which is good as
+  the JIT readers will need to be fixed manually.
+
 * The "info source" command now displays the producer string if it was
   present in the debug info.  This typically includes the compiler version
   and may include things like its command line arguments.
diff --git a/gdb/jit-reader.in b/gdb/jit-reader.in
index db676ea..9687018 100644
--- a/gdb/jit-reader.in
+++ b/gdb/jit-reader.in
@@ -26,7 +26,7 @@ extern "C" {
 
 /* Versioning information.  See gdb_reader_funcs.  */
 
-#define GDB_READER_INTERFACE_VERSION 1
+#define GDB_READER_INTERFACE_VERSION 2
 
 /* Readers must be released under a GPL compatible license.  To
    declare that the reader is indeed released under a GPL compatible
@@ -279,17 +279,20 @@ struct gdb_unwind_callbacks
 
 struct gdb_reader_funcs;
 
-/* Parse the debug info off a block of memory, pointed to by MEMORY
-   (already copied to GDB's address space) and MEMORY_SZ bytes long.
-   The implementation has to use the functions in CB to actually emit
-   the parsed data into GDB.  SELF is the same structure returned by
+/* Parse the debug info from the SYMFILE_SIZE bytes of memory located at
+   SYMFILE_ADDR on the target.  These values are taken from the
+   corresponding "struct jit_code_entry" node that the target has
+   registered using the GDB JIT compilation interface.  The
+   implementation has to use the functions in CB to actually emit the
+   parsed data into GDB.  SELF is the same structure returned by
    gdb_init_reader.
 
    Return GDB_FAIL on failure and GDB_SUCCESS on success.  */
 
 typedef enum gdb_status (gdb_read_debug_info) (struct gdb_reader_funcs *self,
                                                struct gdb_symbol_callbacks *cb,
-                                               void *memory, long memory_sz);
+                                               GDB_CORE_ADDR symfile_addr,
+                                               long symfile_size);
 
 /* Unwind the current frame, CB is the set of unwind callbacks that
    are to be used to do this.
diff --git a/gdb/jit.c b/gdb/jit.c
index d4f1d7e..ab1e9ad 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -849,10 +849,12 @@ jit_reader_try_read_symtab (struct jit_code_entry *code_entry,
                             CORE_ADDR entry_addr)
 {
   void *gdb_mem;
-  int status;
+  enum gdb_status status;
   jit_dbg_reader_data priv_data;
   struct gdb_reader_funcs *funcs;
   volatile struct gdb_exception e;
+  GDB_CORE_ADDR symfile_addr;
+  long symfile_size;
   struct gdb_symbol_callbacks callbacks =
     {
       jit_object_open_impl,
@@ -867,34 +869,20 @@ jit_reader_try_read_symtab (struct jit_code_entry *code_entry,
       &priv_data
     };
 
-  priv_data = entry_addr;
-
   if (!jit_reader_loaded ())
     return 0;
 
-  gdb_mem = xmalloc (code_entry->symfile_size);
-
-  status = 1;
-  TRY_CATCH (e, RETURN_MASK_ALL)
-    if (target_read_memory (code_entry->symfile_addr, gdb_mem,
-                            code_entry->symfile_size))
-      status = 0;
-  if (e.reason < 0)
-    status = 0;
-
-  if (status)
-    {
-      funcs = loaded_jit_reader->functions;
-      if (funcs->read (funcs, &callbacks, gdb_mem, code_entry->symfile_size)
-          != GDB_SUCCESS)
-        status = 0;
-    }
+  priv_data = entry_addr;
+  funcs = loaded_jit_reader->functions;
+  symfile_addr = code_entry->symfile_addr;
+  symfile_size = code_entry->symfile_size;
+  status = funcs->read (funcs, &callbacks, symfile_addr, symfile_size);
 
-  xfree (gdb_mem);
-  if (jit_debug && status == 0)
+  if (jit_debug && status == GDB_FAIL)
     fprintf_unfiltered (gdb_stdlog,
                         "Could not read symtab using the loaded JIT reader.\n");
-  return status;
+
+  return status == GDB_SUCCESS;
 }
 
 /* Try to read CODE_ENTRY using BFD.  ENTRY_ADDR is the address of the
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 0008005..1976d63 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2015-02-23  Andy Wingo  <wingo@igalia.com>
+
+	* gdb.base/jitreader.c (read_debug_info): Update for changes in
+	JIT reader interface and jithost.c test.
+	* gdb.base/jithost.c (main): No need to allocate symfile objects
+	with the new GDB_READER_INTERFACE_VERSION.
+	* gdb.base/jithost.h: Removed.
+
 2015-02-23  Sanjoy Das <sanjoy@playingwithpointers.com>
 
 	* gdb.base/jit-reader.exp: New file. Test case for the jit-reader
diff --git a/gdb/testsuite/gdb.base/jithost.c b/gdb/testsuite/gdb.base/jithost.c
index 413c4cd..b8876f5 100644
--- a/gdb/testsuite/gdb.base/jithost.c
+++ b/gdb/testsuite/gdb.base/jithost.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 2009-2012 Free Software Foundation, Inc.
+/* Copyright (C) 2009-2012, 2015 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -23,7 +23,6 @@
 #include <sys/mman.h>
 
 #include JIT_READER_H  /* Please see jit-reader.exp for an explanation.  */
-#include "jithost.h"
 #include "jit-protocol.h"
 
 void __attribute__((noinline)) __jit_debug_register_code () { }
@@ -42,12 +41,8 @@ int main (int argc, char **argv)
   code[0] = 0xcc; /* SIGTRAP  */
   code[1] = 0xc3; /* RET  */
 
-  struct jithost_abi *symfile = malloc (sizeof (struct jithost_abi));
-  symfile->begin = code;
-  symfile->end = code + 2;
-
-  only_entry.symfile_addr = symfile;
-  only_entry.symfile_size = sizeof (struct jithost_abi);
+  only_entry.symfile_addr = code;
+  only_entry.symfile_size = 2;
 
   __jit_debug_descriptor.first_entry = &only_entry;
   __jit_debug_descriptor.relevant_entry = &only_entry;
diff --git a/gdb/testsuite/gdb.base/jithost.h b/gdb/testsuite/gdb.base/jithost.h
deleted file mode 100644
index 52ca87a..0000000
--- a/gdb/testsuite/gdb.base/jithost.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/* Copyright (C) 2009-2012 Free Software Foundation, Inc.
-
-   This file is part of GDB.
-
-   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/>.  */
-
-#ifndef JITHOST_H
-#define JITHOST_H
-
-struct jithost_abi
-{
-  const char *begin;
-  const char *end;
-};
-
-#endif /* JITHOST_H */
diff --git a/gdb/testsuite/gdb.base/jitreader.c b/gdb/testsuite/gdb.base/jitreader.c
index 0c935c4..9efdf2b 100644
--- a/gdb/testsuite/gdb.base/jitreader.c
+++ b/gdb/testsuite/gdb.base/jitreader.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 2009-2012 Free Software Foundation, Inc.
+/* Copyright (C) 2009-2012, 2015 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -21,7 +21,6 @@
 #include <string.h>
 
 #include JIT_READER_H  /* Please see jit-reader.exp for an explanation.  */
-#include "jithost.h"
 
 GDB_DECLARE_GPL_COMPATIBLE_READER;
 
@@ -40,18 +39,23 @@ struct reader_state
 static enum gdb_status
 read_debug_info (struct gdb_reader_funcs *self,
 		 struct gdb_symbol_callbacks *cbs,
-                 void *memory, long memory_sz)
+                 GDB_CORE_ADDR symfile_addr, long symfile_size)
 {
-  struct jithost_abi *symfile = memory;
-  struct gdb_object *object = cbs->object_open (cbs);
-  struct gdb_symtab *symtab = cbs->symtab_open (cbs, object, "");
-  GDB_CORE_ADDR begin = (GDB_CORE_ADDR) symfile->begin;
-  GDB_CORE_ADDR end = (GDB_CORE_ADDR) symfile->end;
+  struct gdb_object *object;
+  struct gdb_symtab *symtab;
+  GDB_CORE_ADDR begin, end;
 
-  cbs->block_open (cbs, symtab, NULL, begin, end, "jit_function_00");
+  /* For this test, there is no additional metadata in a symbol, so it the code
+     itself can serve as the symfile.  */
+  begin = symfile_addr;
+  end = symfile_addr + symfile_size;
 
+  object = cbs->object_open (cbs);
+  symtab = cbs->symtab_open (cbs, object, "");
+  cbs->block_open (cbs, symtab, NULL, begin, end, "jit_function_00");
   cbs->symtab_close (cbs, symtab);
   cbs->object_close (cbs, object);
+
   return GDB_SUCCESS;
 }
 
-- 
2.1.4


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