[PATCH 0/3] Fix recent breakage in the JIT reader interface

[PATCH 0/3] Fix recent breakage in the JIT reader interface

[Sanjoy Das]

These three patches fix an issue with the JIT reader interface and
adds a regression test for the same.

Sanjoy Das (3):
  Fix http://sourceware.org/bugzilla/show_bug.cgi?id=14550
  Make jit-reader-load accept absolute paths to reader shared objects.
  Add a test case for the jit-reader interface.

 gdb/ChangeLog                         |   12 +++
 gdb/doc/ChangeLog                     |    4 +
 gdb/doc/gdb.texinfo                   |   22 +++---
 gdb/jit.c                             |   19 ++++-
 gdb/testsuite/ChangeLog               |    8 ++
 gdb/testsuite/gdb.base/jit-reader.exp |   78 +++++++++++++++++++
 gdb/testsuite/gdb.base/jithost.c      |   67 ++++++++++++++++
 gdb/testsuite/gdb.base/jithost.h      |    9 +++
 gdb/testsuite/gdb.base/jitreader.c    |  136 +++++++++++++++++++++++++++++++++
 9 files changed, 343 insertions(+), 12 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/jit-reader.exp
 create mode 100644 gdb/testsuite/gdb.base/jithost.c
 create mode 100644 gdb/testsuite/gdb.base/jithost.h
 create mode 100644 gdb/testsuite/gdb.base/jitreader.c

-- 
1.7.10.4

[PATCH 3/3] Add a test case for the jit-reader interface.

[Sanjoy Das]

---
 gdb/testsuite/ChangeLog               |    8 ++
 gdb/testsuite/gdb.base/jit-reader.exp |   78 +++++++++++++++++++
 gdb/testsuite/gdb.base/jithost.c      |   67 ++++++++++++++++
 gdb/testsuite/gdb.base/jithost.h      |    9 +++
 gdb/testsuite/gdb.base/jitreader.c    |  136 +++++++++++++++++++++++++++++++++
 5 files changed, 298 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/jit-reader.exp
 create mode 100644 gdb/testsuite/gdb.base/jithost.c
 create mode 100644 gdb/testsuite/gdb.base/jithost.h
 create mode 100644 gdb/testsuite/gdb.base/jitreader.c

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index b458aba..f770475 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2012-09-18  Sanjoy Das <sanjoy@playingwithpointers.com>
+
+	* gdb.base/jit-reader.exp (new): Test case for the jit-reader
+	interface.
+	* gdb.base/jithost.c (new): Do.
+	* gdb.base/jithost.h (new): Do.
+	* gdb.base/jitreader.c (new) : Do.
+
 2012-09-17  Yao Qi  <yao@codesourcery.com>
 
 	* gdb.base/list.exp (set_listsize): Don't set arg to "unlimited"
diff --git a/gdb/testsuite/gdb.base/jit-reader.exp b/gdb/testsuite/gdb.base/jit-reader.exp
new file mode 100644
index 0000000..9a85b76
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-reader.exp
@@ -0,0 +1,78 @@
+# Copyright 2011-2012 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 ![istarget x86_64-*-linux-gnu*] then {
+    untested jit-reader.exp
+    return -1;
+}
+
+if {[skip_shlib_tests]} {
+    untested jit-reader.exp
+    return -1
+}
+
+if {[get_compiler_info]} {
+    warning "Could not get compiler info"
+    untested jit-reader.exp
+    return 1
+}
+
+#
+# test running programs
+#
+
+set jit_host jithost
+set jit_host_src ${jit_host}.c
+set jit_host_bin ${objdir}/${subdir}/${jit_host}
+
+set include_dir ${objdir}/../../
+
+if  { [gdb_compile "${srcdir}/${subdir}/${jit_host_src}" "${jit_host_bin}" \
+       executable  [list debug incdir=${include_dir}]] != "" } {
+    untested jit-reader.exp
+    return -1
+}
+
+set reader "jitreader"
+set reader_srcfile "${srcdir}/${subdir}/${reader}.c"
+set reader_binfile "${objdir}/${subdir}/${reader}.so"
+
+# Note: compiling without debug info: the library goes through symbol
+# renaming by munging on its symbol table, and that wouldn't work for .debug
+# sections.  Also, output for "info function" changes when debug info is resent.
+if { [gdb_compile_shlib ${reader_srcfile} ${reader_binfile} \
+	  [list debug incdir=${include_dir}]] != "" } {
+    untested jit-reader.exp
+    return -1
+}
+
+proc jit_reader_test {} {
+    global jit_host
+    global reader_binfile
+    global verbose
+
+    clean_restart $jit_host
+
+    if {$verbose > 0} {
+	gdb_test "set debug jit 1"
+    }
+
+    gdb_test_no_output "jit-reader-load ${reader_binfile}"
+    gdb_test "run" ".*"
+
+    gdb_test "bt" "jit_function_00.*"
+}
+
+jit_reader_test
diff --git a/gdb/testsuite/gdb.base/jithost.c b/gdb/testsuite/gdb.base/jithost.c
new file mode 100644
index 0000000..967245c
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jithost.c
@@ -0,0 +1,67 @@
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <sys/mman.h>
+
+#include "gdb/jit-reader.h"
+#include "jithost.h"
+
+typedef enum
+{
+  JIT_NOACTION = 0,
+  JIT_REGISTER_FN,
+  JIT_UNREGISTER_FN
+} jit_actions_t;
+
+struct jit_code_entry
+{
+  struct jit_code_entry *next_entry;
+  struct jit_code_entry *prev_entry;
+  void *symfile_addr;
+  uint64_t symfile_size;
+};
+
+struct jit_descriptor
+{
+  uint32_t version;
+  uint32_t action_flag;
+  struct jit_code_entry *relevant_entry;
+  struct jit_code_entry *first_entry;
+};
+
+void __attribute__((noinline)) __jit_debug_register_code () { }
+
+struct jit_descriptor __jit_debug_descriptor = { 1, 0, 0, 0 };
+struct jit_code_entry only_entry;
+
+typedef void (jit_function_t)();
+
+int main(int argc, char **argv)
+{
+  char *code = mmap(NULL, getpagesize(), PROT_WRITE | PROT_EXEC,
+                    MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+  jit_function_t *function = (jit_function_t *) code;
+
+  code[0] = 0xcc; // Generate a 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);
+
+  __jit_debug_descriptor.first_entry =
+    __jit_debug_descriptor.relevant_entry = &only_entry;
+  __jit_debug_descriptor.action_flag = JIT_REGISTER_FN;
+  __jit_debug_descriptor.version = 1;
+  __jit_debug_register_code();
+
+  function();
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/jithost.h b/gdb/testsuite/gdb.base/jithost.h
new file mode 100644
index 0000000..37cf042
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jithost.h
@@ -0,0 +1,9 @@
+#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
new file mode 100644
index 0000000..2438b0d
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jitreader.c
@@ -0,0 +1,136 @@
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "gdb/jit-reader.h"
+#include "jithost.h"
+
+GDB_DECLARE_GPL_COMPATIBLE_READER;
+
+enum RegisterMapping {
+    AMD64_RA = 16,
+    AMD64_RSP = 7,
+};
+
+struct reader_state {
+  uintptr_t code_begin;
+  uintptr_t code_end;
+};
+
+static enum gdb_status
+read_debug_info (struct gdb_reader_funcs* self,
+                 struct gdb_symbol_callbacks* cbs,
+                 void* memory, long memory_sz)
+{
+  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;
+
+  cbs->block_open (cbs, symtab, NULL, begin, end, "jit_function_00");
+
+  cbs->symtab_close (cbs, symtab);
+  cbs->object_close (cbs, object);
+  return GDB_SUCCESS;
+}
+
+static void
+free_reg_value(struct gdb_reg_value *value)
+{
+  free (value);
+}
+
+static void
+write_register (struct gdb_unwind_callbacks *callbacks, int dw_reg,
+                uintptr_t value)
+{
+  const int size = sizeof (uintptr_t);
+  struct gdb_reg_value *reg_val =
+    malloc (sizeof(struct gdb_reg_value) + size - 1);
+  reg_val->defined = 1;
+  reg_val->free = free_reg_value;
+
+  memcpy (reg_val->value, &value, size);
+  callbacks->reg_set (callbacks, dw_reg, reg_val);
+}
+
+static int
+read_register (struct gdb_unwind_callbacks *callbacks, int dw_reg, uintptr_t *value)
+{
+  const int size = sizeof (uintptr_t);
+  struct gdb_reg_value *reg_val = callbacks->reg_get (callbacks, dw_reg);
+  if (reg_val->size != size || !reg_val->defined)
+    {
+      reg_val->free (reg_val);
+      return 0;
+    }
+  memcpy (value, reg_val->value, size);
+  reg_val->free (reg_val);
+  return 1;
+}
+
+enum gdb_status
+unwind_frame(struct gdb_reader_funcs* self, struct gdb_unwind_callbacks* cbs)
+{
+  const int word_size = sizeof (uintptr_t);
+  uintptr_t this_sp, this_ip, prev_ip, prev_sp;
+  struct reader_state *state = (struct reader_state *) self->priv_data;
+
+  if (!read_register (cbs, AMD64_RA, &this_ip))
+    return GDB_FAIL;
+
+  if (this_ip >= state->code_end || this_ip < state->code_begin)
+    return GDB_FAIL;
+
+  printf("Unwinding %p\n", (void *) this_ip);
+
+  if (!read_register (cbs, AMD64_RSP, &this_sp))
+    return GDB_FAIL;
+
+  if (cbs->target_read (this_sp, &prev_ip, word_size) == GDB_FAIL)
+    return GDB_FAIL;
+
+  prev_sp = this_sp + word_size;
+  write_register (cbs, AMD64_RA, prev_ip);
+  write_register (cbs, AMD64_RSP, prev_sp);
+  return GDB_SUCCESS;
+}
+
+struct gdb_frame_id
+get_frame_id (struct gdb_reader_funcs* self, struct gdb_unwind_callbacks* cbs)
+{
+  struct reader_state *state = (struct reader_state *) self->priv_data;
+  struct gdb_frame_id frame_id;
+
+  uintptr_t sp;
+  read_register (cbs, AMD64_RSP, &sp);
+
+  frame_id.code_address = (GDB_CORE_ADDR) state->code_begin;
+  frame_id.stack_address = (GDB_CORE_ADDR) sp;
+
+  return frame_id;
+}
+
+void
+destroy_reader(struct gdb_reader_funcs* self)
+{
+  free (self->priv_data);
+  free (self);
+}
+
+extern struct gdb_reader_funcs* gdb_init_reader(void) {
+  struct reader_state *state = malloc (sizeof (struct reader_state));
+  struct gdb_reader_funcs *reader_funcs =
+    malloc (sizeof (struct gdb_reader_funcs));
+
+  reader_funcs->reader_version = GDB_READER_INTERFACE_VERSION;
+  reader_funcs->priv_data = state;
+  reader_funcs->read = read_debug_info;
+  reader_funcs->unwind = unwind_frame;
+  reader_funcs->get_frame_id = get_frame_id;
+  reader_funcs->destroy = destroy_reader;
+
+  return reader_funcs;
+}
-- 
1.7.10.4

Re: [PATCH 3/3] Add a test case for the jit-reader interface.

[Tom Tromey]

>>>>> "Sanjoy" == Sanjoy Das <sanjoy@playingwithpointers.com> writes:

Sanjoy> +2012-09-18  Sanjoy Das <sanjoy@playingwithpointers.com>

Thanks.  This is very nice.

Sanjoy> +	* gdb.base/jit-reader.exp (new): Test case for the jit-reader
Sanjoy> +	interface.
Sanjoy> +	* gdb.base/jithost.c (new): Do.
Sanjoy> +	* gdb.base/jithost.h (new): Do.
Sanjoy> +	* gdb.base/jitreader.c (new) : Do.

Those should be like:

	* gdb.base/jithost.c: New file.

Sanjoy> +# Note: compiling without debug info: the library goes through symbol
Sanjoy> +# renaming by munging on its symbol table, and that wouldn't work for .debug
Sanjoy> +# sections.  Also, output for "info function" changes when debug info is resent.

Bad copy&paste :)
I think this comment isn't relevant & can be deleted.

Sanjoy> +    gdb_test "run" ".*"

Use gdb_run_cmd instead.

Tom

[PATCH 2/3] Make jit-reader-load accept absolute paths to reader shared objects.

[Sanjoy Das]

---
 gdb/ChangeLog       |    5 +++++
 gdb/doc/ChangeLog   |    4 ++++
 gdb/doc/gdb.texinfo |   22 +++++++++++++---------
 gdb/jit.c           |    5 ++++-
 4 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a1c55e5..24ea360 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2012-09-18  Sanjoy Das <sanjoy@playingwithpointers.com>
 
+	* jit.c (jit_reader_load_command): Interpret the jit reader name
+	as an absolute path if it begins with a forward slash.
+
+2012-09-18  Sanjoy Das <sanjoy@playingwithpointers.com>
+
 	PR gdb/14550
 
 	* jit.c (finalize_symtab): Ensure that only the global block has a
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index adcd4ff..9b84dcc 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,7 @@
+2012-09-18  Sanjoy Das <sanjoy@playingwithpointers.com>
+	* gdb.texinfo (Using JIT Debug Info Readers): Change documentation
+	to reflect that jit-reader-load now supports absolute file-names.
+
 2012-09-17  Yao Qi  <yao@codesourcery.com>
 
 	* gdb.texinfo (List): Describe the meaning of 0 and -1 in
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index dc8860a..0939c24 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -33419,15 +33419,19 @@ Readers can be loaded and unloaded using the @code{jit-reader-load}
 and @code{jit-reader-unload} commands.
 
 @table @code
-@item jit-reader-load @var{reader-name}
-Load the JIT reader named @var{reader-name}.  On a UNIX system, this
-will usually load @file{@var{libdir}/gdb/@var{reader-name}}, where
-@var{libdir} is the system library directory, usually
-@file{/usr/local/lib}.  Only one reader can be active at a time;
-trying to load a second reader when one is already loaded will result
-in @value{GDBN} reporting an error.  A new JIT reader can be loaded by
-first unloading the current one using @code{jit-reader-load} and then
-invoking @code{jit-reader-load}.
+@item jit-reader-load @var{reader}
+Load the JIT reader named @var{reader}.  @var{reader} can either be
+the absolute path to or the file name of a shared object.  In the
+latter case, gdb will try to load the reader from a pre-configured
+directory, usually @file{@var{libdir}/gdb/} on a UNIX system (here
+@var{libdir} is the system library directory, often
+@file{/usr/local/lib}).
+
+Only one reader can be active at a time; trying to load a second
+reader when one is already loaded will result in @value{GDBN}
+reporting an error.  A new JIT reader can be loaded by first unloading
+the current one using @code{jit-reader-load} and then invoking
+@code{jit-reader-load}.
 
 @item jit-reader-unload
 Unload the currently loaded JIT reader.
diff --git a/gdb/jit.c b/gdb/jit.c
index 2f9d426..818238f 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -208,7 +208,10 @@ jit_reader_load_command (char *args, int from_tty)
   if (loaded_jit_reader != NULL)
     error (_("JIT reader already loaded.  Run jit-reader-unload first."));
 
-  so_name = xstrprintf ("%s/%s", jit_reader_dir, args);
+  if (args[0] == '/')
+    so_name = xstrdup (args);
+  else
+    so_name = xstrprintf ("%s/%s", jit_reader_dir, args);
   prev_cleanup = make_cleanup (xfree, so_name);
 
   loaded_jit_reader = jit_reader_load (so_name);
-- 
1.7.10.4

Re: [PATCH 2/3] Make jit-reader-load accept absolute paths to reader shared objects.

[Eli Zaretskii]

> From: Sanjoy Das <sanjoy@playingwithpointers.com>
> Cc: Sanjoy Das <sanjoy@playingwithpointers.com>
> Date: Tue, 18 Sep 2012 10:04:06 +0530
> 
> +@item jit-reader-load @var{reader}
> +Load the JIT reader named @var{reader}.  @var{reader} can either be
> +the absolute path to or the file name of a shared object.

GNU coding standards discourage using "path" to mean a file name.  So
I suggest to rephrase:

 @var{reader} is a shared object specified as either an absolute or
 a relative file name.

> +Only one reader can be active at a time; trying to load a second
> +reader when one is already loaded will result in @value{GDBN}
> +reporting an error.  A new JIT reader can be loaded by first unloading
> +the current one using @code{jit-reader-load} and then invoking
> +@code{jit-reader-load}.     ^^^^^^^^^^^^^^^

"jit-reader-unload", I presume.  (Yes, I know you were only copying
the existing mistake.)

The documentation parts are OK with those changes.

> +  if (args[0] == '/')

Why not use IS_ABSOLUTE_PATH here?  That would make the code more
portable.

Thanks.

Re: [PATCH 2/3] Make jit-reader-load accept absolute paths to reader shared objects.

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

>> +  if (args[0] == '/')

Eli> Why not use IS_ABSOLUTE_PATH here?  That would make the code more
Eli> portable.

I agree.  The code bits are ok with that change.

Tom

[PATCH 1/3] Fix http://sourceware.org/bugzilla/show_bug.cgi?id=14550

[Sanjoy Das]

---
 gdb/ChangeLog |    7 +++++++
 gdb/jit.c     |   14 ++++++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4ab15c4..a1c55e5 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2012-09-18  Sanjoy Das <sanjoy@playingwithpointers.com>
+
+	PR gdb/14550
+
+	* jit.c (finalize_symtab): Ensure that only the global block has a
+	NULL superblock.
+
 2012-09-17  Mike Wrighton  <wrighton@codesourcery.com>
 
 	* MAINTAINERS (Write After Approval): Add "Mike Wrighton".
diff --git a/gdb/jit.c b/gdb/jit.c
index 9e8f295..2f9d426 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -724,8 +724,18 @@ finalize_symtab (struct gdb_symtab *stab, struct objfile *objfile)
        gdb_block_iter = gdb_block_iter->next)
     {
       if (gdb_block_iter->parent != NULL)
-        BLOCK_SUPERBLOCK (gdb_block_iter->real_block) =
-          gdb_block_iter->parent->real_block;
+	{
+	  /* If the plugin specifically mentioned a parent block, we
+	     use that */
+	  BLOCK_SUPERBLOCK (gdb_block_iter->real_block) =
+	    gdb_block_iter->parent->real_block;
+	}
+      else
+	{
+	  /* and if not, we set a default parent block. */
+	  BLOCK_SUPERBLOCK (gdb_block_iter->real_block) =
+	    BLOCKVECTOR_BLOCK (symtab->blockvector, STATIC_BLOCK);
+	}
     }
 
   /* Free memory.  */
-- 
1.7.10.4

Re: [PATCH 1/3] Fix http://sourceware.org/bugzilla/show_bug.cgi?id=14550

[Tom Tromey]

>>>>> "Sanjoy" == Sanjoy Das <sanjoy@playingwithpointers.com> writes:

Sanjoy> +	  /* If the plugin specifically mentioned a parent block, we
Sanjoy> +	     use that */

The comment should end with a period and two spaces.

Sanjoy> +	  /* and if not, we set a default parent block. */

Similarly, and also the first letter should be capitalized.

Ok with those changes.

Tom

[PATCH 0/3] Address review, "fix recent breakage in the JIT reader interface"

[Sanjoy Das]

Changes from last series:

  Fix http://sourceware.org/bugzilla/show_bug.cgi?id=14550
   * Fix comment grammar.

  Make jit-reader-load accept absolute paths to reader shared objects.
   * Use IS_ABSOLUTE_PATH instead of ad-hoc check.
   * Don't use "path" to mean "file name".
   * Fix nit with jit-reader-load / jit-reader-unload.

  Add a test case for the jit-reader interface.
   * Remove bad copy-paste.
   * Use gdb_run_cmd instead of gdb_test foo ".*"

 gdb/ChangeLog                         |   12 +++
 gdb/doc/ChangeLog                     |    4 +
 gdb/doc/gdb.texinfo                   |   22 +++---
 gdb/jit.c                             |   20 ++++-
 gdb/testsuite/ChangeLog               |    8 ++
 gdb/testsuite/gdb.base/jit-reader.exp |   71 +++++++++++++++++
 gdb/testsuite/gdb.base/jithost.c      |   67 ++++++++++++++++
 gdb/testsuite/gdb.base/jithost.h      |    9 +++
 gdb/testsuite/gdb.base/jitreader.c    |  136 +++++++++++++++++++++++++++++++++
 9 files changed, 337 insertions(+), 12 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/jit-reader.exp
 create mode 100644 gdb/testsuite/gdb.base/jithost.c
 create mode 100644 gdb/testsuite/gdb.base/jithost.h
 create mode 100644 gdb/testsuite/gdb.base/jitreader.c

-- 
1.7.10.4

[PATCH 2/3] Make jit-reader-load accept absolute paths to reader shared objects.

[Sanjoy Das]

---
 gdb/ChangeLog       |    5 +++++
 gdb/doc/ChangeLog   |    4 ++++
 gdb/doc/gdb.texinfo |   22 +++++++++++++---------
 gdb/jit.c           |    6 +++++-
 4 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 13eaa5c..69e3e8f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2012-09-24  Sanjoy Das <sanjoy@playingwithpointers.com>
 
+	* jit.c (jit_reader_load_command): Interpret the jit reader name
+	as an absolute path if it begins with a forward slash.
+
+2012-09-18  Sanjoy Das <sanjoy@playingwithpointers.com>
+
 	PR gdb/14550
 
 	* jit.c (finalize_symtab): Ensure that only the global block has a
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index ba8231a..0546467 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,7 @@
+2012-09-24  Sanjoy Das <sanjoy@playingwithpointers.com>
+	* gdb.texinfo (Using JIT Debug Info Readers): Change documentation
+	to reflect that jit-reader-load now supports absolute file-names.
+
 2012-09-21  Yao Qi  <yao@codesourcery.com>
             Pedro Alves  <palves@redhat.com>
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 5fcbada..c28b803 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -33436,15 +33436,19 @@ Readers can be loaded and unloaded using the @code{jit-reader-load}
 and @code{jit-reader-unload} commands.
 
 @table @code
-@item jit-reader-load @var{reader-name}
-Load the JIT reader named @var{reader-name}.  On a UNIX system, this
-will usually load @file{@var{libdir}/gdb/@var{reader-name}}, where
-@var{libdir} is the system library directory, usually
-@file{/usr/local/lib}.  Only one reader can be active at a time;
-trying to load a second reader when one is already loaded will result
-in @value{GDBN} reporting an error.  A new JIT reader can be loaded by
-first unloading the current one using @code{jit-reader-load} and then
-invoking @code{jit-reader-load}.
+@item jit-reader-load @var{reader}
+Load the JIT reader named @var{reader}.  @var{reader} is a shared
+object specified as either an absolute or a relative file name.  In
+the latter case, gdb will try to load the reader from a pre-configured
+directory, usually @file{@var{libdir}/gdb/} on a UNIX system (here
+@var{libdir} is the system library directory, often
+@file{/usr/local/lib}).
+
+Only one reader can be active at a time; trying to load a second
+reader when one is already loaded will result in @value{GDBN}
+reporting an error.  A new JIT reader can be loaded by first unloading
+the current one using @code{jit-reader-unload} and then invoking
+@code{jit-reader-load}.
 
 @item jit-reader-unload
 Unload the currently loaded JIT reader.
diff --git a/gdb/jit.c b/gdb/jit.c
index eff2ed6..48a2dbb 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -25,6 +25,7 @@
 #include "breakpoint.h"
 #include "command.h"
 #include "dictionary.h"
+#include "filenames.h"
 #include "frame-unwind.h"
 #include "gdbcmd.h"
 #include "gdbcore.h"
@@ -208,7 +209,10 @@ jit_reader_load_command (char *args, int from_tty)
   if (loaded_jit_reader != NULL)
     error (_("JIT reader already loaded.  Run jit-reader-unload first."));
 
-  so_name = xstrprintf ("%s/%s", jit_reader_dir, args);
+  if (IS_ABSOLUTE_PATH(args))
+    so_name = xstrdup (args);
+  else
+    so_name = xstrprintf ("%s/%s", jit_reader_dir, args);
   prev_cleanup = make_cleanup (xfree, so_name);
 
   loaded_jit_reader = jit_reader_load (so_name);
-- 
1.7.10.4

Re: [PATCH 2/3] Make jit-reader-load accept absolute paths to reader shared objects.

[Eli Zaretskii]

> From: Sanjoy Das <sanjoy@playingwithpointers.com>
> Cc: Sanjoy Das <sanjoy@playingwithpointers.com>
> Date: Mon, 24 Sep 2012 10:22:04 +0530
> 
> +@item jit-reader-load @var{reader}
> +Load the JIT reader named @var{reader}.  @var{reader} is a shared
> +object specified as either an absolute or a relative file name.  In
> +the latter case, gdb will try to load the reader from a pre-configured
                    ^^^
@value{GDBN}

OK for the documentation part with that change.

Thanks.

Re: [PATCH 2/3] Make jit-reader-load accept absolute paths to reader shared objects.

[Tom Tromey]

>>>>> "Sanjoy" == Sanjoy Das <sanjoy@playingwithpointers.com> writes:

Sanjoy> +  if (IS_ABSOLUTE_PATH(args))

Missing a space before the open paren.

This one is ok with that change.

Tom

[PATCH 0/3] Address review, "fix recent breakage in the JIT reader interface"

[Sanjoy Das]

Hi,

Sorry for hiatus, got a little busy with university exams.  Here are
the fixed patches.

Changes from the last series:

Fix segfault when unwinding JIT frames using a custom reader.
<None>

Make jit-reader-load accept absolute paths to reader shared objects.
 * Fixed nit in IS_ABSOLUTE_PATH invocation.

Add a test case for the jit-reader interface.
 * jit_host_src and jit_host_bin use standard_testfile
 * jit_reader_bin uses standard_output_file instead of a raw ${objdir}

Thanks!
--
Sanjoy

[PATCH 2/3] Make jit-reader-load accept absolute paths to reader shared objects.

[Sanjoy Das]

---
 gdb/ChangeLog       |    5 +++++
 gdb/doc/ChangeLog   |    4 ++++
 gdb/doc/gdb.texinfo |   22 +++++++++++++---------
 gdb/jit.c           |    6 +++++-
 4 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e57e5c1..01ee95d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2012-10-08  Sanjoy Das <sanjoy@playingwithpointers.com>
 
+	* jit.c (jit_reader_load_command): Interpret the jit reader name
+	as an absolute path if it begins with a forward slash.
+
+2012-09-18  Sanjoy Das <sanjoy@playingwithpointers.com>
+
 	PR gdb/14550
 
 	* jit.c (finalize_symtab): Ensure that only the global block has a
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 05ac406..8cdca61 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,7 @@
+2012-10-05  Sanjoy Das <sanjoy@playingwithpointers.com>
+	* gdb.texinfo (Using JIT Debug Info Readers): Change documentation
+	to reflect that jit-reader-load now supports absolute file-names.
+
 2012-09-26  Siddhesh Poyarekar  <siddhesh@redhat.com>
 
 	* observer.texi (memory_changed): Expand parameter LEN to ssize_t.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 5fcbada..c28b803 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -33436,15 +33436,19 @@ Readers can be loaded and unloaded using the @code{jit-reader-load}
 and @code{jit-reader-unload} commands.
 
 @table @code
-@item jit-reader-load @var{reader-name}
-Load the JIT reader named @var{reader-name}.  On a UNIX system, this
-will usually load @file{@var{libdir}/gdb/@var{reader-name}}, where
-@var{libdir} is the system library directory, usually
-@file{/usr/local/lib}.  Only one reader can be active at a time;
-trying to load a second reader when one is already loaded will result
-in @value{GDBN} reporting an error.  A new JIT reader can be loaded by
-first unloading the current one using @code{jit-reader-load} and then
-invoking @code{jit-reader-load}.
+@item jit-reader-load @var{reader}
+Load the JIT reader named @var{reader}.  @var{reader} is a shared
+object specified as either an absolute or a relative file name.  In
+the latter case, gdb will try to load the reader from a pre-configured
+directory, usually @file{@var{libdir}/gdb/} on a UNIX system (here
+@var{libdir} is the system library directory, often
+@file{/usr/local/lib}).
+
+Only one reader can be active at a time; trying to load a second
+reader when one is already loaded will result in @value{GDBN}
+reporting an error.  A new JIT reader can be loaded by first unloading
+the current one using @code{jit-reader-unload} and then invoking
+@code{jit-reader-load}.
 
 @item jit-reader-unload
 Unload the currently loaded JIT reader.
diff --git a/gdb/jit.c b/gdb/jit.c
index eff2ed6..fb6658f 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -25,6 +25,7 @@
 #include "breakpoint.h"
 #include "command.h"
 #include "dictionary.h"
+#include "filenames.h"
 #include "frame-unwind.h"
 #include "gdbcmd.h"
 #include "gdbcore.h"
@@ -208,7 +209,10 @@ jit_reader_load_command (char *args, int from_tty)
   if (loaded_jit_reader != NULL)
     error (_("JIT reader already loaded.  Run jit-reader-unload first."));
 
-  so_name = xstrprintf ("%s/%s", jit_reader_dir, args);
+  if (IS_ABSOLUTE_PATH (args))
+    so_name = xstrdup (args);
+  else
+    so_name = xstrprintf ("%s/%s", jit_reader_dir, args);
   prev_cleanup = make_cleanup (xfree, so_name);
 
   loaded_jit_reader = jit_reader_load (so_name);
-- 
1.7.10.4

Re: [PATCH 2/3] Make jit-reader-load accept absolute paths to reader shared objects.

[Eli Zaretskii]

> From: Sanjoy Das <sanjoy@playingwithpointers.com>
> Cc: Sanjoy Das <sanjoy@playingwithpointers.com>
> Date: Mon,  8 Oct 2012 17:17:28 +0530
> 
> +Load the JIT reader named @var{reader}.  @var{reader} is a shared
> +object specified as either an absolute or a relative file name.  In
> +the latter case, gdb will try to load the reader from a pre-configured
                    ^^^
@value{GDB}

The patch to the manual is OK with this change.  Thanks.

Re: [PATCH 2/3] Make jit-reader-load accept absolute paths to reader shared objects.

[Tom Tromey]

>>>>> "Sanjoy" == Sanjoy Das <sanjoy@playingwithpointers.com> writes:

Sanjoy>  2012-10-08  Sanjoy Das <sanjoy@playingwithpointers.com>
 
Sanjoy> +	* jit.c (jit_reader_load_command): Interpret the jit reader name
Sanjoy> +	as an absolute path if it begins with a forward slash.

This is ok, but I think it should wait until the whole series is approved.

Tom

[PATCH 0/3] Address review, "fix recent breakage in the JIT reader interface"

[Sanjoy Das]

Changes from last series:

  Fix segfault when unwinding JIT frames using a custom reader.
    <None>

  Make jit-reader-load accept absolute paths to reader shared objects.
    Fix ${GDBN} mistake pointed out by Eli

  Add a test case for the jit-reader interface.
    Fix issues pointed out by Pedro.

 gdb/ChangeLog                         |   12 +++
 gdb/doc/ChangeLog                     |    4 +
 gdb/doc/gdb.texinfo                   |   22 +++--
 gdb/jit.c                             |   20 ++++-
 gdb/testsuite/ChangeLog               |    8 ++
 gdb/testsuite/gdb.base/jit-reader.exp |   73 +++++++++++++++
 gdb/testsuite/gdb.base/jithost.c      |   84 ++++++++++++++++++
 gdb/testsuite/gdb.base/jithost.h      |   27 ++++++
 gdb/testsuite/gdb.base/jitreader.c    |  156 +++++++++++++++++++++++++++++++++
 9 files changed, 394 insertions(+), 12 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/jit-reader.exp
 create mode 100644 gdb/testsuite/gdb.base/jithost.c
 create mode 100644 gdb/testsuite/gdb.base/jithost.h
 create mode 100644 gdb/testsuite/gdb.base/jitreader.c

-- 
1.7.10.4

[PATCH 2/3] Make jit-reader-load accept absolute paths to reader shared objects.

[Sanjoy Das]

---
 gdb/ChangeLog       |    5 +++++
 gdb/doc/ChangeLog   |    4 ++++
 gdb/doc/gdb.texinfo |   22 +++++++++++++---------
 gdb/jit.c           |    6 +++++-
 4 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0999b30..912d1e4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2012-11-04  Sanjoy Das <sanjoy@playingwithpointers.com>
 
+	* jit.c (jit_reader_load_command): Interpret the jit reader name
+	as an absolute path if it begins with a forward slash.
+
+2012-11-04  Sanjoy Das <sanjoy@playingwithpointers.com>
+
 	PR gdb/14550
 
 	* jit.c (finalize_symtab): Ensure that only the global block has a
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index a717c5a..468a3ed 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,7 @@
+2012-11-04  Sanjoy Das <sanjoy@playingwithpointers.com>
+	* gdb.texinfo (Using JIT Debug Info Readers): Change documentation
+	to reflect that jit-reader-load now supports absolute file-names.
+
 2012-11-03  Yao Qi  <yao@codesourcery.com>
 
 	* observer.texi (GDB Observers): Remove observer
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 21db475..d3bbc11 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -33470,15 +33470,19 @@ Readers can be loaded and unloaded using the @code{jit-reader-load}
 and @code{jit-reader-unload} commands.
 
 @table @code
-@item jit-reader-load @var{reader-name}
-Load the JIT reader named @var{reader-name}.  On a UNIX system, this
-will usually load @file{@var{libdir}/gdb/@var{reader-name}}, where
-@var{libdir} is the system library directory, usually
-@file{/usr/local/lib}.  Only one reader can be active at a time;
-trying to load a second reader when one is already loaded will result
-in @value{GDBN} reporting an error.  A new JIT reader can be loaded by
-first unloading the current one using @code{jit-reader-load} and then
-invoking @code{jit-reader-load}.
+@item jit-reader-load @var{reader}
+Load the JIT reader named @var{reader}.  @var{reader} is a shared
+object specified as either an absolute or a relative file name.  In
+the latter case, @value{GDBN} will try to load the reader from a
+pre-configured directory, usually @file{@var{libdir}/gdb/} on a UNIX
+system (here @var{libdir} is the system library directory, often
+@file{/usr/local/lib}).
+
+Only one reader can be active at a time; trying to load a second
+reader when one is already loaded will result in @value{GDBN}
+reporting an error.  A new JIT reader can be loaded by first unloading
+the current one using @code{jit-reader-unload} and then invoking
+@code{jit-reader-load}.
 
 @item jit-reader-unload
 Unload the currently loaded JIT reader.
diff --git a/gdb/jit.c b/gdb/jit.c
index eff2ed6..fb6658f 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -25,6 +25,7 @@
 #include "breakpoint.h"
 #include "command.h"
 #include "dictionary.h"
+#include "filenames.h"
 #include "frame-unwind.h"
 #include "gdbcmd.h"
 #include "gdbcore.h"
@@ -208,7 +209,10 @@ jit_reader_load_command (char *args, int from_tty)
   if (loaded_jit_reader != NULL)
     error (_("JIT reader already loaded.  Run jit-reader-unload first."));
 
-  so_name = xstrprintf ("%s/%s", jit_reader_dir, args);
+  if (IS_ABSOLUTE_PATH (args))
+    so_name = xstrdup (args);
+  else
+    so_name = xstrprintf ("%s/%s", jit_reader_dir, args);
   prev_cleanup = make_cleanup (xfree, so_name);
 
   loaded_jit_reader = jit_reader_load (so_name);
-- 
1.7.10.4

Re: [PATCH 2/3] Make jit-reader-load accept absolute paths to reader shared objects.

[Jan Kratochvil]

On Sun, 04 Nov 2012 18:03:50 +0100, Sanjoy Das wrote:
> -  so_name = xstrprintf ("%s/%s", jit_reader_dir, args);
> +  if (IS_ABSOLUTE_PATH (args))
> +    so_name = xstrdup (args);
> +  else
> +    so_name = xstrprintf ("%s/%s", jit_reader_dir, args);

GDB uses SLASH_STRING; although I see it defined as "/" unconditionally
(therefore even on MinGW).


Jan