JIT Interface Patches Landed

JIT Interface Patches Landed

[Sanjoy Das]

Hi all,

I've checked in the work I did to implement a (hopefully) simpler JIT
interface.

Thanks!
-- 
Sanjoy Das
http://playingwithpointers.com

Re: JIT Interface Patches Landed

[Stan Shebs]

On 11/20/11 2:13 AM, Sanjoy Das wrote:
> Hi all,
>
> I've checked in the work I did to implement a (hopefully) simpler JIT
> interface.
>

Did you recheck running with the testsuite?  I just saw four regressions 
in current jit.exp vs a testsuite run I did a week ago, but haven't 
investigated further.

Stan
stan@codesourcery.com

Regression for gdb.base/jit.exp and gdb.base/jit-so.exp [Re: JIT Interface Patches Landed]

[Jan Kratochvil]

On Mon, 21 Nov 2011 05:06:17 +0100, Stan Shebs wrote:
> On 11/20/11 2:13 AM, Sanjoy Das wrote:
> >Hi all,
> >
> >I've checked in the work I did to implement a (hopefully) simpler JIT
> >interface.
> >
> 
> Did you recheck running with the testsuite?  I just saw four
> regressions in current jit.exp vs a testsuite run I did a week ago,
> but haven't investigated further.

9894c3216f7cf02a51d8c2058cac8cba33dc6e69 is the first bad commit
commit 9894c3216f7cf02a51d8c2058cac8cba33dc6e69
Author: sanjoyd <sanjoyd>
Date:   Sun Nov 20 09:14:45 2011 +0000

    gdb/
	* jit.c: Include block.h, dictionary.h and frame-unwind.h.
	(add_objfile_entry, jit_target_read_impl, jit_object_open_impl)
	(jit_symtab_open_impl, compare_block, jit_block_open_impl)
	(jit_symtab_line_mapping_add_impl, jit_symtab_close_impl)
	(finalize_symtab, jit_object_close_impl)
	(jit_reader_try_read_symtab, jit_bfd_try_read_symtab)
	(free_objfile_data): New functions.
	(_initialize_jit): Register jit_objfile_data with a proper cleanup
	function.

-PASS: gdb.base/jit.exp: one_jit_test-1: info function jit_function
+FAIL: gdb.base/jit.exp: one_jit_test-1: info function jit_function
-PASS: gdb.base/jit.exp: one_jit_test-2: info function jit_function
+FAIL: gdb.base/jit.exp: one_jit_test-2: info function jit_function
-PASS: gdb.base/jit-so.exp: one_jit_test-1 info function jit_function
+FAIL: gdb.base/jit-so.exp: one_jit_test-1 info function jit_function
-PASS: gdb.base/jit-so.exp: one_jit_test-2 info function jit_function
+FAIL: gdb.base/jit-so.exp: one_jit_test-2 info function jit_function

Unable to find JITed code entry at address: 0x602030


Thanks,
Jan

Re: Regression for gdb.base/jit.exp and gdb.base/jit-so.exp [Re: JIT Interface Patches Landed]

[Sanjoy Das]

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

Hi,

Sorry for the delayed response.  I've attached the fix.  With this
patch GDB passes both jit.exp and jit-so.exp.

Thanks!
-- 
Sanjoy Das
http://playingwithpointers.com

[-- Attachment #2: bugfix.diff --]
[-- Type: text/x-diff, Size: 1874 bytes --]

diff --git a/gdb/jit.c b/gdb/jit.c
index 7bd6b36..6bfe1b6 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -713,7 +713,8 @@ jit_object_close_impl (struct gdb_symbol_callbacks *cb,
 /* Try to read CODE_ENTRY using the loaded jit reader (if any).  */
 
 static int
-jit_reader_try_read_symtab (struct jit_code_entry *code_entry)
+jit_reader_try_read_symtab (struct jit_code_entry *code_entry,
+                            CORE_ADDR entry_addr)
 {
   void *gdb_mem;
   int status;
@@ -735,7 +736,7 @@ jit_reader_try_read_symtab (struct jit_code_entry *code_entry)
       &priv_data
     };
 
-  priv_data = code_entry->symfile_addr;
+  priv_data = entry_addr;
 
   if (!loaded_jit_reader)
     return 0;
@@ -769,6 +770,7 @@ jit_reader_try_read_symtab (struct jit_code_entry *code_entry)
 
 static void
 jit_bfd_try_read_symtab (struct jit_code_entry *code_entry,
+                         CORE_ADDR entry_addr,
                          struct gdbarch *gdbarch)
 {
   bfd *nbfd;
@@ -832,7 +834,7 @@ JITed symbol file is not an object file, ignoring it.\n"));
   objfile = symbol_file_add_from_bfd (nbfd, 0, sai, OBJF_SHARED, NULL);
 
   do_cleanups (old_cleanups);
-  add_objfile_entry (objfile, code_entry->symfile_addr);
+  add_objfile_entry (objfile, entry_addr);
 }
 
 /* This function registers code associated with a JIT code entry.  It uses the
@@ -855,10 +857,10 @@ jit_register_code (struct gdbarch *gdbarch,
                         paddress (gdbarch, code_entry->symfile_addr),
                         pulongest (code_entry->symfile_size));
 
-  success = jit_reader_try_read_symtab (code_entry);
+  success = jit_reader_try_read_symtab (code_entry, entry_addr);
 
   if (!success)
-    jit_bfd_try_read_symtab (code_entry, gdbarch);
+    jit_bfd_try_read_symtab (code_entry, entry_addr, gdbarch);
 }
 
 /* This function unregisters JITed code and frees the corresponding

Re: Regression for gdb.base/jit.exp and gdb.base/jit-so.exp [Re: JIT Interface Patches Landed]

[Jan Kratochvil]

On Sat, 26 Nov 2011 14:17:45 +0100, Sanjoy Das wrote:
> Sorry for the delayed response.  I've attached the fix.  With this
> patch GDB passes both jit.exp and jit-so.exp.

OK for check-in, but with FSF ChangeLog and with description of the ENTRY_ADDR
parameter in the functions' comments.

ENTRY_ADDR name itself associates me with an execution entry point - which it
is not.


Thanks,
Jan

Re: Regression for gdb.base/jit.exp and gdb.base/jit-so.exp [Re: JIT Interface Patches Landed]

[Sanjoy Das]

Checked in with suggested corrections.

Thanks!
-- 
Sanjoy Das
http://playingwithpointers.com

Re: Regression for gdb.base/jit.exp and gdb.base/jit-so.exp [Re: JIT Interface Patches Landed]

[Jan Kratochvil]

On Sun, 27 Nov 2011 15:31:38 +0100, Sanjoy Das wrote:
> Checked in with suggested corrections.

Could you post to gdb-patches when the checked in version has not been posted
here yet?


Thanks,
Jan

Re: Regression for gdb.base/jit.exp and gdb.base/jit-so.exp [Re: JIT Interface Patches Landed]

[Sanjoy Das]

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

> Could you post to gdb-patches when the checked in version has not been posted
> here yet?

Attached.

I skipped the patch because it is already there at gdb-patches. :)

Thanks!
-- 
Sanjoy Das
http://playingwithpointers.com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch.diff --]
[-- Type: text/x-diff; name="patch.diff", Size: 2761 bytes --]

===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.13552
retrieving revision 1.13553
diff -u -r1.13552 -r1.13553
--- src/gdb/ChangeLog	2011/11/25 17:05:36	1.13552
+++ src/gdb/ChangeLog	2011/11/27 14:26:05	1.13553
@@ -1,3 +1,10 @@
+2011-11-27  Sanjoy Das  <sdas@igalia.com>
+
+	Fix regression in jit.exp.
+	* jit.c (jit_reader_try_read_symtab, jit_bfd_try_read_symtab)
+	(jit_register_code): Set the jit_objfile_data field to the correct
+	value.
+
 2011-11-25  Uros Bizjak  <ubizjak@gmail.com>
 
 	* alpha-tdep.c (br_opcode): New.
===================================================================
RCS file: /cvs/src/src/gdb/jit.c,v
retrieving revision 1.21
retrieving revision 1.22
diff -u -r1.21 -r1.22
--- src/gdb/jit.c	2011/11/20 09:18:48	1.21
+++ src/gdb/jit.c	2011/11/27 14:26:09	1.22
@@ -710,10 +710,13 @@
   xfree (obj);
 }
 
-/* Try to read CODE_ENTRY using the loaded jit reader (if any).  */
+/* Try to read CODE_ENTRY using the loaded jit reader (if any).
+   ENTRY_ADDR is the address of the object file (in the target's
+   address space) being read.  */
 
 static int
-jit_reader_try_read_symtab (struct jit_code_entry *code_entry)
+jit_reader_try_read_symtab (struct jit_code_entry *code_entry,
+                            CORE_ADDR entry_addr)
 {
   void *gdb_mem;
   int status;
@@ -735,7 +738,7 @@
       &priv_data
     };
 
-  priv_data = code_entry->symfile_addr;
+  priv_data = entry_addr;
 
   if (!loaded_jit_reader)
     return 0;
@@ -765,10 +768,12 @@
   return status;
 }
 
-/* Try to read CODE_ENTRY using BFD.  */
+/* Try to read CODE_ENTRY using BFD.  ENTRY_ADDR is the address of the
+   object file (in the target's address space) being read.  */
 
 static void
 jit_bfd_try_read_symtab (struct jit_code_entry *code_entry,
+                         CORE_ADDR entry_addr,
                          struct gdbarch *gdbarch)
 {
   bfd *nbfd;
@@ -832,7 +837,7 @@
   objfile = symbol_file_add_from_bfd (nbfd, 0, sai, OBJF_SHARED, NULL);
 
   do_cleanups (old_cleanups);
-  add_objfile_entry (objfile, code_entry->symfile_addr);
+  add_objfile_entry (objfile, entry_addr);
 }
 
 /* This function registers code associated with a JIT code entry.  It uses the
@@ -855,10 +860,10 @@
                         paddress (gdbarch, code_entry->symfile_addr),
                         pulongest (code_entry->symfile_size));
 
-  success = jit_reader_try_read_symtab (code_entry);
+  success = jit_reader_try_read_symtab (code_entry, entry_addr);
 
   if (!success)
-    jit_bfd_try_read_symtab (code_entry, gdbarch);
+    jit_bfd_try_read_symtab (code_entry, entry_addr, gdbarch);
 }
 
 /* This function unregisters JITed code and frees the corresponding

Re: Regression for gdb.base/jit.exp and gdb.base/jit-so.exp [Re: JIT Interface Patches Landed]

[Sanjoy Das]

> I skipped the patch because it is already there at gdb-patches. :)

I meant gdb-cvs.
-- 
Sanjoy Das
http://playingwithpointers.com

Re: Regression for gdb.base/jit.exp and gdb.base/jit-so.exp [Re: JIT Interface Patches Landed]

[Joel Brobecker]

> > I skipped the patch because it is already there at gdb-patches. :)
> 
> I meant gdb-cvs.

Unfortunately, not everyone is subscribed to gdb-cvs, and also
we do not get diffs with gdb-cvs (which is a real pity, if you
ask me). That's why it's important to always send patches to
gdb-patches.

-- 
Joel

Re: Regression for gdb.base/jit.exp and gdb.base/jit-so.exp [Re: JIT Interface Patches Landed]

[Sanjoy Das]

> Unfortunately, not everyone is subscribed to gdb-cvs, and also
> we do not get diffs with gdb-cvs (which is a real pity, if you
> ask me). That's why it's important to always send patches to
> gdb-patches.

Thanks!  I'll keep that in mind. :)
-- 
Sanjoy Das
http://playingwithpointers.com

Re: Regression for gdb.base/jit.exp and gdb.base/jit-so.exp [Re: JIT Interface Patches Landed]

[Jan Kratochvil]

On Sun, 27 Nov 2011 15:42:32 +0100, Sanjoy Das wrote:
> -/* Try to read CODE_ENTRY using the loaded jit reader (if any).  */
> +/* Try to read CODE_ENTRY using the loaded jit reader (if any).
> +   ENTRY_ADDR is the address of the object file (in the target's
> +   address space) being read.  */
>  
>  static int
> -jit_reader_try_read_symtab (struct jit_code_entry *code_entry)
> +jit_reader_try_read_symtab (struct jit_code_entry *code_entry,
> +                            CORE_ADDR entry_addr)

The problem is ENTRY_ADDR is not the object file address.

SYMFILE_ADDR is the object file address.  ENTRY_ADDR is the address of the
inferior struct jit_code_entry.  The bug was these two can be different.

Also the comment
	/* Remember a mapping from entry_addr to objfile.  */

	static void
	add_objfile_entry (struct objfile *objfile, CORE_ADDR entry)

is not saying much, there should be something like:
	/* Remember OBJFILE has been created for struct jit_code_entry located
	   at inferior address ENTRY.  */

Could you review these comments there?


Thanks,
Jan

Re: Regression for gdb.base/jit.exp and gdb.base/jit-so.exp [Re: JIT Interface Patches Landed]

[Sanjoy Das]

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

> Could you review these comments there?

I've checked in the attached patch.
-- 
Sanjoy Das
http://playingwithpointers.com

[-- Attachment #2: comments.diff --]
[-- Type: text/x-diff, Size: 1587 bytes --]

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c759514..a8d71f4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2011-11-29  Sanjoy Das  <sdas@igalia.com>
+
+	* jit.c (add_objfile_entry, jit_reader_try_read_symtab)
+	(jit_bfd_try_read_symtab): Fix comments.
+
 2011-11-28  DJ Delorie  <dj@redhat.com>
 
 	* NEWS: Mention RL78 simulator.
diff --git a/gdb/jit.c b/gdb/jit.c
index 099f79d..843c768 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -239,7 +239,8 @@ struct jit_inferior_data
   CORE_ADDR descriptor_addr;  /* &__jit_debug_descriptor  */
 };
 
-/* Remember a mapping from entry_addr to objfile.  */
+/* Remember OBJFILE has been created for struct jit_code_entry located
+   at inferior address ENTRY.  */
 
 static void
 add_objfile_entry (struct objfile *objfile, CORE_ADDR entry)
@@ -711,8 +712,8 @@ jit_object_close_impl (struct gdb_symbol_callbacks *cb,
 }
 
 /* Try to read CODE_ENTRY using the loaded jit reader (if any).
-   ENTRY_ADDR is the address of the object file (in the target's
-   address space) being read.  */
+   ENTRY_ADDR is the address of the struct jit_code_entry in the
+   inferior address space.  */
 
 static int
 jit_reader_try_read_symtab (struct jit_code_entry *code_entry,
@@ -769,7 +770,7 @@ jit_reader_try_read_symtab (struct jit_code_entry *code_entry,
 }
 
 /* Try to read CODE_ENTRY using BFD.  ENTRY_ADDR is the address of the
-   object file (in the target's address space) being read.  */
+   struct jit_code_entry in the inferior address space.  */
 
 static void
 jit_bfd_try_read_symtab (struct jit_code_entry *code_entry,

Re: Regression for gdb.base/jit.exp and gdb.base/jit-so.exp [Re: JIT Interface Patches Landed]

[Jan Kratochvil]

On Tue, 29 Nov 2011 09:37:10 +0100, Sanjoy Das wrote:
> I've checked in the attached patch.

It looks better understandable for me now.


Thanks,
Jan