public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [patch] Add tests for JIT debugging interface
@ 2011-01-11 23:28 Paul Pluzhnikov
  2011-01-12  7:02 ` Yao Qi
  2011-01-12 10:03 ` Pedro Alves
  0 siblings, 2 replies; 13+ messages in thread
From: Paul Pluzhnikov @ 2011-01-11 23:28 UTC (permalink / raw)
  To: gdb-patches; +Cc: ppluzhnikov

Greetings,

Currently there are no tests which verify that JIT debugging actually works.

Here is a patch to perform some minimal checks (ELF only).

Thanks,

--
Paul Pluzhnikov

testsuite/ChangeLog:

2011-01-11  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb.base/jit.exp: New file.
	* gdb.base/jit-main.c: New file.
	* gdb.base/jit-solib.c: New file.



Index: testsuite/gdb.base/jit-main.c
===================================================================
RCS file: testsuite/gdb.base/jit-main.c
diff -N testsuite/gdb.base/jit-main.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/jit-main.c	11 Jan 2011 23:17:54 -0000
@@ -0,0 +1,186 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.
+   */
+
+/* Simulate loading of JIT code.  */
+
+#include <elf.h>
+#include <link.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/stat.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;
+  const char *symfile_addr;
+  uint64_t symfile_size;
+};
+
+struct jit_descriptor
+{
+  uint32_t version;
+  /* This type should be jit_actions_t, but we use uint32_t
+     to be explicit about the bitwidth.  */
+  uint32_t action_flag;
+  struct jit_code_entry *relevant_entry;
+  struct jit_code_entry *first_entry;
+};
+
+/* GDB puts a breakpoint in this function.  */
+void __attribute__((noinline)) __jit_debug_register_code () { }
+
+/* Make sure to specify the version statically, because the
+   debugger may check the version before we can set it.  */
+struct jit_descriptor __jit_debug_descriptor = { 1, 0, 0, 0 };
+
+static void
+usage (const char *const argv0)
+{
+  fprintf (stderr, "Usage: %s library [count]\n", argv0);
+  exit (1);
+}
+
+/* Update .p_vaddr and .sh_addr as if the code was JITted to ADDR.  */
+
+static void
+update_locations (const void *const addr, int idx)
+{
+  const ElfW (Ehdr) *const ehdr = (ElfW (Ehdr) *)addr;
+  ElfW (Shdr) *const shdr = (ElfW (Shdr) *)((char *)addr + ehdr->e_shoff);
+  ElfW (Phdr) *const phdr = (ElfW (Phdr) *)((char *)addr + ehdr->e_phoff);
+  int i;
+
+  for (i = 0; i < ehdr->e_phnum; ++i)
+    if (phdr[i].p_type == PT_LOAD)
+      phdr[i].p_vaddr += (ElfW (Addr))addr;
+
+  for (i = 0; i < ehdr->e_shnum; ++i)
+    {
+      if (shdr[i].sh_type == SHT_STRTAB)
+        {
+          /* Note: we update both .strtab and .dynstr.  The latter would
+             not be correct if this were a regular shared library (.hash
+             would be wrong), but this is a simulation -- the library is
+             never exposed to the dynamic loader, so it all ends up ok.  */
+          char *const strtab = (char *)((ElfW (Addr))addr + shdr[i].sh_offset);
+          char *const strtab_end = strtab + shdr[i].sh_size;
+          char *p;
+
+          for (p = strtab; p < strtab_end; p += strlen(p) + 1)
+            if (strcmp(p, "jit_function_XXXX") == 0)
+              sprintf(p, "jit_function_%04d", idx);
+        }
+
+      if (shdr[i].sh_flags & SHF_ALLOC)
+        shdr[i].sh_addr += (ElfW (Addr))addr;
+    }
+}
+
+int main(int argc, char *argv[])
+{
+  if (argc < 2)
+    usage(argv[0]);
+  else
+    {
+      const char *const libname = argv[1];
+      int i, fd, count = 1;
+      struct stat st;
+
+      if (argc > 2)
+        count = atoi (argv[2]);
+
+      if ((fd = open (libname, O_RDONLY)) == -1)
+        {
+          fprintf (stderr, "open (\"%s\", O_RDONLY): %s\n", libname, strerror (errno));
+          exit (1);
+        }
+
+      if (fstat (fd, &st) != 0)
+        {
+          fprintf (stderr, "fstat (\"%d\"): %s\n", fd, strerror (errno));
+          exit (1);
+        }
+
+      for (i = 0; i < count; ++i)
+        {
+          const void *const addr = mmap (0, st.st_size, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0);
+          struct jit_code_entry *const entry = calloc (1, sizeof (*entry));
+
+          if (addr == MAP_FAILED)
+            {
+              fprintf (stderr, "mmap: %s\n", strerror (errno));
+              exit (1);
+            }
+
+          update_locations (addr, i);
+
+          /* Link entry at the end of the list.  */
+          entry->symfile_addr = (const char *)addr;
+          entry->symfile_size = st.st_size;
+          entry->prev_entry = __jit_debug_descriptor.relevant_entry;
+          __jit_debug_descriptor.relevant_entry = entry;
+
+          if (entry->prev_entry != NULL)
+            entry->prev_entry->next_entry = entry;
+          else
+            __jit_debug_descriptor.first_entry = entry;
+
+          /* Notify GDB.  */
+          __jit_debug_descriptor.action_flag = JIT_REGISTER_FN;
+          __jit_debug_register_code ();
+        }
+
+      i = 0;  /* gdb break here 1 */
+
+      /* Now unregister them all in reverse order.  */
+      while (__jit_debug_descriptor.relevant_entry != NULL)
+        {
+          struct jit_code_entry *const entry = __jit_debug_descriptor.relevant_entry;
+          struct jit_code_entry *const prev_entry = entry->prev_entry;
+
+          if (prev_entry != NULL)
+            {
+              prev_entry->next_entry = NULL;
+              entry->prev_entry = NULL;
+            }
+          else
+            __jit_debug_descriptor.first_entry = NULL;
+
+          /* Notify GDB.  */
+          __jit_debug_descriptor.action_flag = JIT_UNREGISTER_FN;
+          __jit_debug_register_code ();
+
+          __jit_debug_descriptor.relevant_entry = prev_entry;
+          free (entry);
+        }
+    }
+  return 0;  /* gdb break here 2  */
+}
Index: testsuite/gdb.base/jit-solib.c
===================================================================
RCS file: testsuite/gdb.base/jit-solib.c
diff -N testsuite/gdb.base/jit-solib.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/jit-solib.c	11 Jan 2011 23:17:54 -0000
@@ -0,0 +1,22 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.
+   */
+
+/* This simulates a JIT library.  The function is "renamed" after being
+   loaded into memory.  */
+
+int jit_function_XXXX() { return 42; }
Index: testsuite/gdb.base/jit.exp
===================================================================
RCS file: testsuite/gdb.base/jit.exp
diff -N testsuite/gdb.base/jit.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/jit.exp	11 Jan 2011 23:17:54 -0000
@@ -0,0 +1,83 @@
+# Copyright 2011 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 $tracelevel {
+    strace $tracelevel
+}
+
+if {[skip_shlib_tests]} {
+    untested jit.exp
+    return -1
+}
+
+if {[get_compiler_info not-used]} {
+    warning "Could not get compiler info"
+    untested jit.exp
+    return 1
+}
+
+#
+# test running programs
+#
+
+set testfile jit-main
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    untested jit.exp
+    return -1
+}
+
+set solib_testfile "jit-solib"
+set solib_srcfile "${srcdir}/${subdir}/${solib_testfile}.c"
+set solib_binfile "${objdir}/${subdir}/${solib_testfile}.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 ${solib_srcfile} ${solib_binfile} {-fPIC}] != "" } {
+    untested jit.exp
+    return -1
+}
+
+
+# Start with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load $binfile
+
+proc one_test {count match_str} {
+    global solib_binfile gdb_prompt
+
+    gdb_test_no_output "set args $solib_binfile $count" "set args"
+    if ![runto_main] then {
+	fail "Can't run to main"
+	return 0
+    }
+
+    gdb_breakpoint [gdb_get_line_number "break here 1"]
+    gdb_continue_to_breakpoint "break here 1"
+    gdb_test "info function jit_function" "$match_str"
+    gdb_breakpoint [gdb_get_line_number "break here 2"]
+    gdb_continue_to_breakpoint "break here 2"
+    # All jit librares must have been unregistered
+    gdb_test "info function jit_function" \
+	"All functions matching regular expression \"jit_function\":"
+}
+
+one_test 1 "${hex}  jit_function_0000"
+one_test 2 "${hex}  jit_function_0000\[\r\n\]+${hex}  jit_function_0001"

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch] Add tests for JIT debugging interface
  2011-01-11 23:28 [patch] Add tests for JIT debugging interface Paul Pluzhnikov
@ 2011-01-12  7:02 ` Yao Qi
  2011-01-12 10:44   ` Pedro Alves
  2011-01-12 10:03 ` Pedro Alves
  1 sibling, 1 reply; 13+ messages in thread
From: Yao Qi @ 2011-01-12  7:02 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: gdb-patches

On 01/11/2011 05:26 PM, Paul Pluzhnikov wrote:
> +
> +int main(int argc, char *argv[])
> +{
> +  if (argc<  2)
> +    usage(argv[0]);
> +  else
> +    {
> +      const char *const libname = argv[1];
> +      int i, fd, count = 1;
> +      struct stat st;
> +
> +      if (argc>  2)
> +        count = atoi (argv[2]);
> +
> +      if ((fd = open (libname, O_RDONLY)) == -1)
> +        {
> +          fprintf (stderr, "open (\"%s\", O_RDONLY): %s\n", libname, strerror (errno));

This line is too long.

> +          exit (1);
> +        }
> +
> +      if (fstat (fd,&st) != 0)
> +        {
> +          fprintf (stderr, "fstat (\"%d\"): %s\n", fd, strerror (errno));
> +          exit (1);
> +        }
> +
> +      for (i = 0; i<  count; ++i)
> +        {
> +          const void *const addr = mmap (0, st.st_size, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0);

This line is too long as well.

> +# Start with a fresh gdb.
> +
> +gdb_exit
> +gdb_start
> +gdb_reinitialize_dir $srcdir/$subdir
> +gdb_load $binfile
> +
We can use "clean_restart" here.

> +proc one_test {count match_str} {
> +    global solib_binfile gdb_prompt

I don't find any usage of "gdb_prompt", we probably remove it.


Test cases work well in native GDB.  Does gdbserver have JIT debugging 
interface also?  If not, we probably should skip this test for remote 
mode or kfail it.

-- 
Yao Qi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch] Add tests for JIT debugging interface
  2011-01-11 23:28 [patch] Add tests for JIT debugging interface Paul Pluzhnikov
  2011-01-12  7:02 ` Yao Qi
@ 2011-01-12 10:03 ` Pedro Alves
  1 sibling, 0 replies; 13+ messages in thread
From: Pedro Alves @ 2011-01-12 10:03 UTC (permalink / raw)
  To: gdb-patches; +Cc: Paul Pluzhnikov

On Tuesday 11 January 2011 23:26:41, Paul Pluzhnikov wrote:
> +int main(int argc, char *argv[])
> +{
> +  if (argc < 2)
> +    usage(argv[0]);
> +  else
> +    {

You're using GNU formatting throughout the file except here.


> +proc one_test {count match_str} {
> +    global solib_binfile gdb_prompt
> +
> +    gdb_test_no_output "set args $solib_binfile $count" "set args"
> +    if ![runto_main] then {
> +       fail "Can't run to main"
> +       return 0
> +    }
> +
> +    gdb_breakpoint [gdb_get_line_number "break here 1"]
> +    gdb_continue_to_breakpoint "break here 1"
> +    gdb_test "info function jit_function" "$match_str"
> +    gdb_breakpoint [gdb_get_line_number "break here 2"]
> +    gdb_continue_to_breakpoint "break here 2"
> +    # All jit librares must have been unregistered
> +    gdb_test "info function jit_function" \
> +       "All functions matching regular expression \"jit_function\":"

Tests should have distinct messages.  Since this procedure
is ran more than once, you'll have identical output more than
once in your gdb.sum.  I suggest making use of $pf_prefix (grep will
find uses).

This is okay with that, and Yao's comments addressed.

Thanks much!

> +}
> +
> +one_test 1 "${hex}  jit_function_0000"
> +one_test 2 "${hex}  jit_function_0000\[\r\n\]+${hex}  jit_function_0001"

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch] Add tests for JIT debugging interface
  2011-01-12  7:02 ` Yao Qi
@ 2011-01-12 10:44   ` Pedro Alves
  2011-01-12 15:19     ` Paul Pluzhnikov
  0 siblings, 1 reply; 13+ messages in thread
From: Pedro Alves @ 2011-01-12 10:44 UTC (permalink / raw)
  To: gdb-patches; +Cc: Yao Qi, Paul Pluzhnikov

On Wednesday 12 January 2011 05:39:32, Yao Qi wrote:
> Test cases work well in native GDB.  Does gdbserver have JIT debugging 
> interface also?  If not, we probably should skip this test for remote 
> mode or kfail it.

The JIT debugging interface is all implemented in common code,
so it should work against remote/gdbserver as well (though I haven't
tried it).  A gdb_load_shlibs call to copy the shlib to the
target appears to be missing though.  You want notice it missing
if you do the usual gdbserver running on the same machine as
gdb testing (*).

*)
http://sourceware.org/gdb/wiki/TestingGDB#Testing_gdbserver_in_a_native_configuration

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch] Add tests for JIT debugging interface
  2011-01-12 10:44   ` Pedro Alves
@ 2011-01-12 15:19     ` Paul Pluzhnikov
  2011-01-12 16:12       ` Pedro Alves
  2011-01-12 17:23       ` Yao Qi
  0 siblings, 2 replies; 13+ messages in thread
From: Paul Pluzhnikov @ 2011-01-12 15:19 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Yao Qi

On Wed, Jan 12, 2011 at 2:03 AM, Pedro Alves <pedro@codesourcery.com> wrote:
> On Wednesday 12 January 2011 05:39:32, Yao Qi wrote:
>> Test cases work well in native GDB.  Does gdbserver have JIT debugging
>> interface also?  If not, we probably should skip this test for remote
>> mode or kfail it.
>
> The JIT debugging interface is all implemented in common code,
> so it should work against remote/gdbserver as well (though I haven't
> tried it).  A gdb_load_shlibs call to copy the shlib to the
> target appears to be missing though.

I tried running the test with gdbserver-native, and it failed, because
'set args' doesn't work in server mode (AFAICT).

I grepped through *.exp in search of examples of how to run programs
that require args under gdbserver, but didn't find any.

I want something like

  if {[is_remote host]} {
    spawn gdbserver :2345 exe shlib 1
  } else {
    spawn gdb --args exe shlib 1
  }

Is there a "standard" way to achieve this?

> You want notice it missing
> if you do the usual gdbserver running on the same machine as
> gdb testing (*).

Right. I didn't get that far yet.

Are there instructions for running with gdbserver on different host?

Thanks,
-- 
Paul Pluzhnikov

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch] Add tests for JIT debugging interface
  2011-01-12 15:19     ` Paul Pluzhnikov
@ 2011-01-12 16:12       ` Pedro Alves
  2011-01-12 17:23       ` Yao Qi
  1 sibling, 0 replies; 13+ messages in thread
From: Pedro Alves @ 2011-01-12 16:12 UTC (permalink / raw)
  To: gdb-patches; +Cc: Paul Pluzhnikov, Yao Qi

On Wednesday 12 January 2011 15:08:00, Paul Pluzhnikov wrote:
> Is there a "standard" way to achieve this?

Run to main, and write whatever options you need to
(global) variables (e.g., "p count $count") instead of
using "set args".  You could leave the argc > 0 handling
in place if it makes it simpler to debug manually.
That, or compile the program with -DOPTION=FOO.  Some
tests do that instead.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch] Add tests for JIT debugging interface
  2011-01-12 15:19     ` Paul Pluzhnikov
  2011-01-12 16:12       ` Pedro Alves
@ 2011-01-12 17:23       ` Yao Qi
  2011-01-12 17:33         ` Paul Pluzhnikov
  1 sibling, 1 reply; 13+ messages in thread
From: Yao Qi @ 2011-01-12 17:23 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: Pedro Alves, gdb-patches

On 01/12/2011 09:08 AM, Paul Pluzhnikov wrote:
> On Wed, Jan 12, 2011 at 2:03 AM, Pedro Alves<pedro@codesourcery.com>  wrote:
>> On Wednesday 12 January 2011 05:39:32, Yao Qi wrote:
>>> Test cases work well in native GDB.  Does gdbserver have JIT debugging
>>> interface also?  If not, we probably should skip this test for remote
>>> mode or kfail it.
>>
>> The JIT debugging interface is all implemented in common code,
>> so it should work against remote/gdbserver as well (though I haven't
>> tried it).  A gdb_load_shlibs call to copy the shlib to the
>> target appears to be missing though.
>
> I tried running the test with gdbserver-native, and it failed, because
> 'set args' doesn't work in server mode (AFAICT).
>

Yes, I also got some failures due to this.  Then I hacked jit-main.c to 
set libname by an absolute path on my laptop, so no args are needed any 
more.  There are still two failures, which are not about 'set args' in 
gdbserver.

FAIL: gdb.base/jit.exp: info function jit_function
FAIL: gdb.base/jit.exp: info function jit_function

info function jit_function^M
All functions matching regular expression "jit_function":^M
(gdb) FAIL: gdb.base/jit.exp: info function jit_function

In gdb.log, I find something strange,

(gdb) continue^M
Continuing.^M
jit_inferior_init, registering_code = 0^M
jit_inferior_init, reg_addr = 0x80486c4^M
jit_inferior_init, jit_descriptor_addr = 0x804a040^M
Cannot remove breakpoints because program is no longer writable. <-- [1]

There is no such error [1] in native gdb test.  I have no clue on this 
so far.

> I grepped through *.exp in search of examples of how to run programs
> that require args under gdbserver, but didn't find any.
>
> I want something like
>
>    if {[is_remote host]} {
>      spawn gdbserver :2345 exe shlib 1
>    } else {
>      spawn gdb --args exe shlib 1
>    }
>
> Is there a "standard" way to achieve this?
>
>> You want notice it missing
>> if you do the usual gdbserver running on the same machine as
>> gdb testing (*).
>
> Right. I didn't get that far yet.
>
> Are there instructions for running with gdbserver on different host?

We may re-write jit-main.c a little bit to compute the location of 
jit-solib.so via getcwd() + argv[0], rather than passing arguments of 
its location.

-- 
Yao Qi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch] Add tests for JIT debugging interface
  2011-01-12 17:23       ` Yao Qi
@ 2011-01-12 17:33         ` Paul Pluzhnikov
  2011-01-18 17:07           ` Paul Pluzhnikov
  2011-01-19 19:27           ` Pedro Alves
  0 siblings, 2 replies; 13+ messages in thread
From: Paul Pluzhnikov @ 2011-01-12 17:33 UTC (permalink / raw)
  To: Yao Qi; +Cc: Pedro Alves, gdb-patches

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

On Wed, Jan 12, 2011 at 8:57 AM, Yao Qi <yao@codesourcery.com> wrote:

> There are still two failures, which are not about 'set args' in gdbserver.
>
> FAIL: gdb.base/jit.exp: info function jit_function
> FAIL: gdb.base/jit.exp: info function jit_function

Yes, I see that as well.

> info function jit_function^M
> All functions matching regular expression "jit_function":^M
> (gdb) FAIL: gdb.base/jit.exp: info function jit_function
>
> In gdb.log, I find something strange,
>
> (gdb) continue^M
> Continuing.^M
> jit_inferior_init, registering_code = 0^M
> jit_inferior_init, reg_addr = 0x80486c4^M
> jit_inferior_init, jit_descriptor_addr = 0x804a040^M
> Cannot remove breakpoints because program is no longer writable. <-- [1]

And that.

> There is no such error [1] in native gdb test.  I have no clue on this so
> far.

That's alright -- I was going to fix this area (see "JIT interface slowness"
in gdb@sourceware.org list), and this isn't the only problem -- I also
noticed that we leak several jit_breakpoints on rerun.

> We may re-write jit-main.c a little bit to compute the location of
> jit-solib.so via getcwd() + argv[0], rather than passing arguments of its
> location.

Done slightly differently.

I believe revised patch addresses all comments so far.

Thanks,
-- 
Paul Pluzhnikov


2011-01-12  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb.base/jit.exp: New file.
	* gdb.base/jit-main.c: New file.
	* gdb.base/jit-solib.c: New file.

[-- Attachment #2: gdb-jit-tests-20110112.txt --]
[-- Type: text/plain, Size: 11350 bytes --]

Index: testsuite/gdb.base/jit-main.c
===================================================================
RCS file: testsuite/gdb.base/jit-main.c
diff -N testsuite/gdb.base/jit-main.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/jit-main.c	12 Jan 2011 17:17:30 -0000
@@ -0,0 +1,202 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.
+   */
+
+/* Simulate loading of JIT code.  */
+
+#include <elf.h>
+#include <link.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/stat.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;
+  const char *symfile_addr;
+  uint64_t symfile_size;
+};
+
+struct jit_descriptor
+{
+  uint32_t version;
+  /* This type should be jit_actions_t, but we use uint32_t
+     to be explicit about the bitwidth.  */
+  uint32_t action_flag;
+  struct jit_code_entry *relevant_entry;
+  struct jit_code_entry *first_entry;
+};
+
+/* GDB puts a breakpoint in this function.  */
+void __attribute__((noinline)) __jit_debug_register_code () { }
+
+/* Make sure to specify the version statically, because the
+   debugger may check the version before we can set it.  */
+struct jit_descriptor __jit_debug_descriptor = { 1, 0, 0, 0 };
+
+static void
+usage (const char *const argv0)
+{
+  fprintf (stderr, "Usage: %s library [count]\n", argv0);
+  exit (1);
+}
+
+/* Update .p_vaddr and .sh_addr as if the code was JITted to ADDR.  */
+
+static void
+update_locations (const void *const addr, int idx)
+{
+  const ElfW (Ehdr) *const ehdr = (ElfW (Ehdr) *)addr;
+  ElfW (Shdr) *const shdr = (ElfW (Shdr) *)((char *)addr + ehdr->e_shoff);
+  ElfW (Phdr) *const phdr = (ElfW (Phdr) *)((char *)addr + ehdr->e_phoff);
+  int i;
+
+  for (i = 0; i < ehdr->e_phnum; ++i)
+    if (phdr[i].p_type == PT_LOAD)
+      phdr[i].p_vaddr += (ElfW (Addr))addr;
+
+  for (i = 0; i < ehdr->e_shnum; ++i)
+    {
+      if (shdr[i].sh_type == SHT_STRTAB)
+        {
+          /* Note: we update both .strtab and .dynstr.  The latter would
+             not be correct if this were a regular shared library (.hash
+             would be wrong), but this is a simulation -- the library is
+             never exposed to the dynamic loader, so it all ends up ok.  */
+          char *const strtab = (char *)((ElfW (Addr))addr + shdr[i].sh_offset);
+          char *const strtab_end = strtab + shdr[i].sh_size;
+          char *p;
+
+          for (p = strtab; p < strtab_end; p += strlen (p) + 1)
+            if (strcmp (p, "jit_function_XXXX") == 0)
+              sprintf (p, "jit_function_%04d", idx);
+        }
+
+      if (shdr[i].sh_flags & SHF_ALLOC)
+        shdr[i].sh_addr += (ElfW (Addr))addr;
+    }
+}
+
+int main (int argc, char *argv[])
+{
+  /* These variables are here so they can easily be set from jit.exp  */
+  const char *libname = NULL;
+  int count = 0;
+
+  count = count;  /* gdb break here 0  */
+
+  if (argc < 2)
+    usage (argv[0]);
+  else
+    {
+      int i, fd;
+      struct stat st;
+
+      if (libname == NULL)
+        /* Only set if not already set from GDB.  */
+        libname = argv[1];
+
+      if (argc > 2 && count == 0)
+        /* Only set if not already set from GDB.  */
+        count = atoi (argv[2]);
+
+      printf ("%s:%d: libname = %s, count = %d\n", __FILE__, __LINE__,
+              libname, count);
+
+      if ((fd = open (libname, O_RDONLY)) == -1)
+        {
+          fprintf (stderr, "open (\"%s\", O_RDONLY): %s\n", libname,
+                   strerror (errno));
+          exit (1);
+        }
+
+      if (fstat (fd, &st) != 0)
+        {
+          fprintf (stderr, "fstat (\"%d\"): %s\n", fd, strerror (errno));
+          exit (1);
+        }
+
+      for (i = 0; i < count; ++i)
+        {
+          const void *const addr = mmap (0, st.st_size, PROT_READ|PROT_WRITE,
+                                         MAP_PRIVATE, fd, 0);
+          struct jit_code_entry *const entry = calloc (1, sizeof (*entry));
+
+          if (addr == MAP_FAILED)
+            {
+              fprintf (stderr, "mmap: %s\n", strerror (errno));
+              exit (1);
+            }
+
+          update_locations (addr, i);
+
+          /* Link entry at the end of the list.  */
+          entry->symfile_addr = (const char *)addr;
+          entry->symfile_size = st.st_size;
+          entry->prev_entry = __jit_debug_descriptor.relevant_entry;
+          __jit_debug_descriptor.relevant_entry = entry;
+
+          if (entry->prev_entry != NULL)
+            entry->prev_entry->next_entry = entry;
+          else
+            __jit_debug_descriptor.first_entry = entry;
+
+          /* Notify GDB.  */
+          __jit_debug_descriptor.action_flag = JIT_REGISTER_FN;
+          __jit_debug_register_code ();
+        }
+
+      i = 0;  /* gdb break here 1 */
+
+      /* Now unregister them all in reverse order.  */
+      while (__jit_debug_descriptor.relevant_entry != NULL)
+        {
+          struct jit_code_entry *const entry =
+            __jit_debug_descriptor.relevant_entry;
+          struct jit_code_entry *const prev_entry = entry->prev_entry;
+
+          if (prev_entry != NULL)
+            {
+              prev_entry->next_entry = NULL;
+              entry->prev_entry = NULL;
+            }
+          else
+            __jit_debug_descriptor.first_entry = NULL;
+
+          /* Notify GDB.  */
+          __jit_debug_descriptor.action_flag = JIT_UNREGISTER_FN;
+          __jit_debug_register_code ();
+
+          __jit_debug_descriptor.relevant_entry = prev_entry;
+          free (entry);
+        }
+    }
+  return 0;  /* gdb break here 2  */
+}
Index: testsuite/gdb.base/jit-solib.c
===================================================================
RCS file: testsuite/gdb.base/jit-solib.c
diff -N testsuite/gdb.base/jit-solib.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/jit-solib.c	12 Jan 2011 17:17:30 -0000
@@ -0,0 +1,22 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.
+   */
+
+/* This simulates a JIT library.  The function is "renamed" after being
+   loaded into memory.  */
+
+int jit_function_XXXX() { return 42; }
Index: testsuite/gdb.base/jit.exp
===================================================================
RCS file: testsuite/gdb.base/jit.exp
diff -N testsuite/gdb.base/jit.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/jit.exp	12 Jan 2011 17:17:30 -0000
@@ -0,0 +1,109 @@
+# Copyright 2011 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 $tracelevel {
+    strace $tracelevel
+}
+
+if {[skip_shlib_tests]} {
+    untested jit.exp
+    return -1
+}
+
+if {[get_compiler_info not-used]} {
+    warning "Could not get compiler info"
+    untested jit.exp
+    return 1
+}
+
+#
+# test running programs
+#
+
+set testfile jit-main
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    untested jit.exp
+    return -1
+}
+
+set solib_testfile "jit-solib"
+set solib_srcfile "${srcdir}/${subdir}/${solib_testfile}.c"
+set solib_binfile "${objdir}/${subdir}/${solib_testfile}.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 ${solib_srcfile} ${solib_binfile} {-fPIC}] != "" } {
+    untested jit.exp
+    return -1
+}
+
+
+# Start with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load $binfile
+
+proc one_jit_test {count match_str} {
+    global verbose testfile solib_binfile pf_prefix
+
+    set old_pf_prefix $pf_prefix
+    set pf_prefix "one_jit_test-$count"
+
+    clean_restart $testfile
+
+    # This is just to help debugging when things fail
+    if {$verbose > 0} {
+	gdb_test "set debug jit 1"
+    }
+
+    if { ![runto_main] } {
+	fail "Can't run to main"
+	return
+    }
+
+    gdb_breakpoint [gdb_get_line_number "break here 0"]
+    gdb_continue_to_breakpoint "break here 0"
+
+    # Poke desired values directly into inferior instead of using "set args"
+    # because "set args" does not work under gdbserver.
+    gdb_test "set var argc = 2"
+    gdb_test "set var libname = \"$solib_binfile\""
+    gdb_test "set var count = $count"
+
+    gdb_breakpoint [gdb_get_line_number "break here 1"]
+    gdb_continue_to_breakpoint "break here 1"
+
+    gdb_test "info function jit_function" "$match_str"
+
+    # This is just to help debugging when things fail
+    if {$verbose > 0} {
+	gdb_test "maintenance print objfiles"
+    }
+
+    gdb_breakpoint [gdb_get_line_number "break here 2"]
+    gdb_continue_to_breakpoint "break here 2"
+    # All jit librares must have been unregistered
+    gdb_test "info function jit_function" \
+	"All functions matching regular expression \"jit_function\":" \
+    set pf_prefix $old_pf_prefix
+}
+
+one_jit_test 1 "${hex}  jit_function_0000"
+one_jit_test 2 "${hex}  jit_function_0000\[\r\n\]+${hex}  jit_function_0001"

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch] Add tests for JIT debugging interface
  2011-01-12 17:33         ` Paul Pluzhnikov
@ 2011-01-18 17:07           ` Paul Pluzhnikov
  2011-01-19 19:27           ` Pedro Alves
  1 sibling, 0 replies; 13+ messages in thread
From: Paul Pluzhnikov @ 2011-01-18 17:07 UTC (permalink / raw)
  To: Yao Qi; +Cc: Pedro Alves, gdb-patches

On Wed, Jan 12, 2011 at 9:22 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:

> I believe revised patch addresses all comments so far.

Ping?

-- 
Paul Pluzhnikov

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch] Add tests for JIT debugging interface
  2011-01-12 17:33         ` Paul Pluzhnikov
  2011-01-18 17:07           ` Paul Pluzhnikov
@ 2011-01-19 19:27           ` Pedro Alves
  2011-01-19 20:30             ` Paul Pluzhnikov
  1 sibling, 1 reply; 13+ messages in thread
From: Pedro Alves @ 2011-01-19 19:27 UTC (permalink / raw)
  To: gdb-patches; +Cc: Paul Pluzhnikov, Yao Qi

On Wednesday 19 January 2011 10:22:53, Paul Pluzhnikov wrote:
> On Wed, Jan 12, 2011 at 8:57 AM, Yao Qi <yao@codesourcery.com> wrote:
> > There are still two failures, which are not about 'set args' in
> > gdbserver.
> > 
> > FAIL: gdb.base/jit.exp: info function jit_function
> > FAIL: gdb.base/jit.exp: info function jit_function
> 
> Yes, I see that as well.
> 
> > info function jit_function^M
> > All functions matching regular expression "jit_function":^M
> > (gdb) FAIL: gdb.base/jit.exp: info function jit_function
> > 
> > In gdb.log, I find something strange,
> > 
> > (gdb) continue^M
> > Continuing.^M
> > jit_inferior_init, registering_code = 0^M
> > jit_inferior_init, reg_addr = 0x80486c4^M
> > jit_inferior_init, jit_descriptor_addr = 0x804a040^M
> > Cannot remove breakpoints because program is no longer writable. <-- [1]
> 
> And that.

It's output from infrun.c:normal_stop.  Sounds like a bug somewhere.

> 
> > There is no such error [1] in native gdb test.  I have no clue on this so
> > far.
> 
> That's alright -- I was going to fix this area (see "JIT interface
> slowness" in gdb@sourceware.org list), and this isn't the only problem --
> I also noticed that we leak several jit_breakpoints on rerun.
> 
> > We may re-write jit-main.c a little bit to compute the location of
> > jit-solib.so via getcwd() + argv[0], rather than passing arguments of its
> > location.
> 
> Done slightly differently.
> 
> I believe revised patch addresses all comments so far.
> 
> Thanks,

> +int main (int argc, char *argv[])
> +{
> +  /* These variables are here so they can easily be set from jit.exp  */

"main" at column 0.  Period, double space.

> +gdb_exit
> +gdb_start
> +gdb_reinitialize_dir $srcdir/$subdir
> +gdb_load $binfile

gdb_load_shlibs call missing.  This will probably need tweaking for
remote host testing, but we'll handle it when/if we stumble on it
on our testing, probably.

Did you try Yao's suggestion of using clean_restart/prepare_for_testing, etc.?
We're prefering using those whenever possible.

Other than that, this looks fine.  Please go ahead.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch] Add tests for JIT debugging interface
  2011-01-19 19:27           ` Pedro Alves
@ 2011-01-19 20:30             ` Paul Pluzhnikov
  2011-01-19 21:04               ` Pedro Alves
  2011-01-19 21:09               ` Yao Qi
  0 siblings, 2 replies; 13+ messages in thread
From: Paul Pluzhnikov @ 2011-01-19 20:30 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Yao Qi

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

On Wed, Jan 19, 2011 at 11:03 AM, Pedro Alves <pedro@codesourcery.com> wrote:

>> > Cannot remove breakpoints because program is no longer writable. <-- [1]
>>
>> And that.
>
> It's output from infrun.c:normal_stop.  Sounds like a bug somewhere.

It is a bug. Fix in progress...

>> That's alright -- I was going to fix this area (see "JIT interface
>> slowness" in gdb@sourceware.org list), and this isn't the only problem --
>> I also noticed that we leak several jit_breakpoints on rerun.

That leak is in fact causing the bug above ...

>> +int main (int argc, char *argv[])
>> +{
>> +  /* These variables are here so they can easily be set from jit.exp  */
>
> "main" at column 0.  Period, double space.

Fixed.

>> +gdb_exit
>> +gdb_start
>> +gdb_reinitialize_dir $srcdir/$subdir
>> +gdb_load $binfile
>
> gdb_load_shlibs call missing.  This will probably need tweaking for
> remote host testing, but we'll handle it when/if we stumble on it
> on our testing, probably.
>
> Did you try Yao's suggestion of using clean_restart/prepare_for_testing, etc.?
> We're prefering using those whenever possible.

I am already doing clean_restart inside one_jit_test.  Above were unnecessary
leftovers.  Deleted.

prepare_for_testing rebuilds the executable, which is not necessary here;
so not using that.


Thanks!
-- 
Paul Pluzhnikov

2011-01-19  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb.base/jit.exp: New file.
	* gdb.base/jit-main.c: New file.
	* gdb.base/jit-solib.c: New file.

[-- Attachment #2: gdb-jit-tests-20110119.txt --]
[-- Type: text/plain, Size: 11241 bytes --]

Index: testsuite/gdb.base/jit-main.c
===================================================================
RCS file: testsuite/gdb.base/jit-main.c
diff -N testsuite/gdb.base/jit-main.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/jit-main.c	19 Jan 2011 19:22:21 -0000
@@ -0,0 +1,203 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.
+   */
+
+/* Simulate loading of JIT code.  */
+
+#include <elf.h>
+#include <link.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/stat.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;
+  const char *symfile_addr;
+  uint64_t symfile_size;
+};
+
+struct jit_descriptor
+{
+  uint32_t version;
+  /* This type should be jit_actions_t, but we use uint32_t
+     to be explicit about the bitwidth.  */
+  uint32_t action_flag;
+  struct jit_code_entry *relevant_entry;
+  struct jit_code_entry *first_entry;
+};
+
+/* GDB puts a breakpoint in this function.  */
+void __attribute__((noinline)) __jit_debug_register_code () { }
+
+/* Make sure to specify the version statically, because the
+   debugger may check the version before we can set it.  */
+struct jit_descriptor __jit_debug_descriptor = { 1, 0, 0, 0 };
+
+static void
+usage (const char *const argv0)
+{
+  fprintf (stderr, "Usage: %s library [count]\n", argv0);
+  exit (1);
+}
+
+/* Update .p_vaddr and .sh_addr as if the code was JITted to ADDR.  */
+
+static void
+update_locations (const void *const addr, int idx)
+{
+  const ElfW (Ehdr) *const ehdr = (ElfW (Ehdr) *)addr;
+  ElfW (Shdr) *const shdr = (ElfW (Shdr) *)((char *)addr + ehdr->e_shoff);
+  ElfW (Phdr) *const phdr = (ElfW (Phdr) *)((char *)addr + ehdr->e_phoff);
+  int i;
+
+  for (i = 0; i < ehdr->e_phnum; ++i)
+    if (phdr[i].p_type == PT_LOAD)
+      phdr[i].p_vaddr += (ElfW (Addr))addr;
+
+  for (i = 0; i < ehdr->e_shnum; ++i)
+    {
+      if (shdr[i].sh_type == SHT_STRTAB)
+        {
+          /* Note: we update both .strtab and .dynstr.  The latter would
+             not be correct if this were a regular shared library (.hash
+             would be wrong), but this is a simulation -- the library is
+             never exposed to the dynamic loader, so it all ends up ok.  */
+          char *const strtab = (char *)((ElfW (Addr))addr + shdr[i].sh_offset);
+          char *const strtab_end = strtab + shdr[i].sh_size;
+          char *p;
+
+          for (p = strtab; p < strtab_end; p += strlen (p) + 1)
+            if (strcmp (p, "jit_function_XXXX") == 0)
+              sprintf (p, "jit_function_%04d", idx);
+        }
+
+      if (shdr[i].sh_flags & SHF_ALLOC)
+        shdr[i].sh_addr += (ElfW (Addr))addr;
+    }
+}
+
+int
+main (int argc, char *argv[])
+{
+  /* These variables are here so they can easily be set from jit.exp.  */
+  const char *libname = NULL;
+  int count = 0;
+
+  count = count;  /* gdb break here 0  */
+
+  if (argc < 2)
+    usage (argv[0]);
+  else
+    {
+      int i, fd;
+      struct stat st;
+
+      if (libname == NULL)
+        /* Only set if not already set from GDB.  */
+        libname = argv[1];
+
+      if (argc > 2 && count == 0)
+        /* Only set if not already set from GDB.  */
+        count = atoi (argv[2]);
+
+      printf ("%s:%d: libname = %s, count = %d\n", __FILE__, __LINE__,
+              libname, count);
+
+      if ((fd = open (libname, O_RDONLY)) == -1)
+        {
+          fprintf (stderr, "open (\"%s\", O_RDONLY): %s\n", libname,
+                   strerror (errno));
+          exit (1);
+        }
+
+      if (fstat (fd, &st) != 0)
+        {
+          fprintf (stderr, "fstat (\"%d\"): %s\n", fd, strerror (errno));
+          exit (1);
+        }
+
+      for (i = 0; i < count; ++i)
+        {
+          const void *const addr = mmap (0, st.st_size, PROT_READ|PROT_WRITE,
+                                         MAP_PRIVATE, fd, 0);
+          struct jit_code_entry *const entry = calloc (1, sizeof (*entry));
+
+          if (addr == MAP_FAILED)
+            {
+              fprintf (stderr, "mmap: %s\n", strerror (errno));
+              exit (1);
+            }
+
+          update_locations (addr, i);
+
+          /* Link entry at the end of the list.  */
+          entry->symfile_addr = (const char *)addr;
+          entry->symfile_size = st.st_size;
+          entry->prev_entry = __jit_debug_descriptor.relevant_entry;
+          __jit_debug_descriptor.relevant_entry = entry;
+
+          if (entry->prev_entry != NULL)
+            entry->prev_entry->next_entry = entry;
+          else
+            __jit_debug_descriptor.first_entry = entry;
+
+          /* Notify GDB.  */
+          __jit_debug_descriptor.action_flag = JIT_REGISTER_FN;
+          __jit_debug_register_code ();
+        }
+
+      i = 0;  /* gdb break here 1 */
+
+      /* Now unregister them all in reverse order.  */
+      while (__jit_debug_descriptor.relevant_entry != NULL)
+        {
+          struct jit_code_entry *const entry =
+            __jit_debug_descriptor.relevant_entry;
+          struct jit_code_entry *const prev_entry = entry->prev_entry;
+
+          if (prev_entry != NULL)
+            {
+              prev_entry->next_entry = NULL;
+              entry->prev_entry = NULL;
+            }
+          else
+            __jit_debug_descriptor.first_entry = NULL;
+
+          /* Notify GDB.  */
+          __jit_debug_descriptor.action_flag = JIT_UNREGISTER_FN;
+          __jit_debug_register_code ();
+
+          __jit_debug_descriptor.relevant_entry = prev_entry;
+          free (entry);
+        }
+    }
+  return 0;  /* gdb break here 2  */
+}
Index: testsuite/gdb.base/jit-solib.c
===================================================================
RCS file: testsuite/gdb.base/jit-solib.c
diff -N testsuite/gdb.base/jit-solib.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/jit-solib.c	19 Jan 2011 19:22:21 -0000
@@ -0,0 +1,22 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.
+   */
+
+/* This simulates a JIT library.  The function is "renamed" after being
+   loaded into memory.  */
+
+int jit_function_XXXX() { return 42; }
Index: testsuite/gdb.base/jit.exp
===================================================================
RCS file: testsuite/gdb.base/jit.exp
diff -N testsuite/gdb.base/jit.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/jit.exp	19 Jan 2011 19:22:21 -0000
@@ -0,0 +1,101 @@
+# Copyright 2011 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 $tracelevel {
+    strace $tracelevel
+}
+
+if {[skip_shlib_tests]} {
+    untested jit.exp
+    return -1
+}
+
+if {[get_compiler_info not-used]} {
+    warning "Could not get compiler info"
+    untested jit.exp
+    return 1
+}
+
+#
+# test running programs
+#
+
+set testfile jit-main
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    untested jit.exp
+    return -1
+}
+
+set solib_testfile "jit-solib"
+set solib_srcfile "${srcdir}/${subdir}/${solib_testfile}.c"
+set solib_binfile "${objdir}/${subdir}/${solib_testfile}.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 ${solib_srcfile} ${solib_binfile} {-fPIC}] != "" } {
+    untested jit.exp
+    return -1
+}
+
+proc one_jit_test {count match_str} {
+    global verbose testfile solib_binfile pf_prefix
+
+    set old_pf_prefix $pf_prefix
+    set pf_prefix "one_jit_test-$count"
+
+    clean_restart $testfile
+
+    # This is just to help debugging when things fail
+    if {$verbose > 0} {
+	gdb_test "set debug jit 1"
+    }
+
+    if { ![runto_main] } {
+	fail "Can't run to main"
+	return
+    }
+
+    gdb_breakpoint [gdb_get_line_number "break here 0"]
+    gdb_continue_to_breakpoint "break here 0"
+
+    # Poke desired values directly into inferior instead of using "set args"
+    # because "set args" does not work under gdbserver.
+    gdb_test "set var argc = 2"
+    gdb_test "set var libname = \"$solib_binfile\""
+    gdb_test "set var count = $count"
+
+    gdb_breakpoint [gdb_get_line_number "break here 1"]
+    gdb_continue_to_breakpoint "break here 1"
+
+    gdb_test "info function jit_function" "$match_str"
+
+    # This is just to help debugging when things fail
+    if {$verbose > 0} {
+	gdb_test "maintenance print objfiles"
+    }
+
+    gdb_breakpoint [gdb_get_line_number "break here 2"]
+    gdb_continue_to_breakpoint "break here 2"
+    # All jit librares must have been unregistered
+    gdb_test "info function jit_function" \
+	"All functions matching regular expression \"jit_function\":" \
+    set pf_prefix $old_pf_prefix
+}
+
+one_jit_test 1 "${hex}  jit_function_0000"
+one_jit_test 2 "${hex}  jit_function_0000\[\r\n\]+${hex}  jit_function_0001"

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch] Add tests for JIT debugging interface
  2011-01-19 20:30             ` Paul Pluzhnikov
@ 2011-01-19 21:04               ` Pedro Alves
  2011-01-19 21:09               ` Yao Qi
  1 sibling, 0 replies; 13+ messages in thread
From: Pedro Alves @ 2011-01-19 21:04 UTC (permalink / raw)
  To: gdb-patches; +Cc: Paul Pluzhnikov, Yao Qi

On Wednesday 19 January 2011 12:26:34, Paul Pluzhnikov wrote:
> On Wed, Jan 19, 2011 at 11:03 AM, Pedro Alves <pedro@codesourcery.com> 

> > It's output from infrun.c:normal_stop.  Sounds like a bug somewhere.
> 
> It is a bug. Fix in progress...
> 
> >> That's alright -- I was going to fix this area (see "JIT interface
> >> slowness" in gdb@sourceware.org list), and this isn't the only problem
> >> -- I also noticed that we leak several jit_breakpoints on rerun.
> 
> That leak is in fact causing the bug above ...

Ah.

Patch is okay.  As general principle, I'd rather fix the leak first and
avoid introducing new failures against gdbserver if possible and
easy, but if you want to check this in already, that'll  be okay too.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [patch] Add tests for JIT debugging interface
  2011-01-19 20:30             ` Paul Pluzhnikov
  2011-01-19 21:04               ` Pedro Alves
@ 2011-01-19 21:09               ` Yao Qi
  1 sibling, 0 replies; 13+ messages in thread
From: Yao Qi @ 2011-01-19 21:09 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: Pedro Alves, gdb-patches

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

On 01/19/2011 12:26 PM, Paul Pluzhnikov wrote:
>>>> Cannot remove breakpoints because program is no longer writable.<-- [1]
>>> >>
>>> >>  And that.
>> >
>> >  It's output from infrun.c:normal_stop.  Sounds like a bug somewhere.
> It is a bug. Fix in progress...
>

FYI, I looked at this problem a little bit last week, and found this 
failure is caused by inserting multiple breakpoints at the same location 
in create_jit_event_breakpoint().

Patch attached can fix this failure, but I am not 100% clear why 
inserting multiple breakpoints causes this failure, and still lack of 
big picture of breakpoint.  If this patch helps, take it away.  If this 
patch is wrong, please ignore it. :-)

-- 
Yao Qi

[-- Attachment #2: many_jit_breakpoint.patch --]
[-- Type: text/x-patch, Size: 745 bytes --]

gdb/
	* breakpoint.c (create_jit_event_breakpoint): Only set one
	breakpoint on an address for bp_jit_event.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 8d0692b..b89118f 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5906,6 +5907,16 @@ create_jit_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address)
 {
   struct breakpoint *b;
 
+  /* Look for jit event breakpoint first.  If there is one set on ADDRESS,
+     don't create new one and return it directly.  */
+
+  ALL_BREAKPOINTS (b)
+  {
+    if (b->type == bp_jit_event && b->loc->requested_address == address)
+      return b;
+  }
+
   b = create_internal_breakpoint (gdbarch, address, bp_jit_event);
   update_global_location_list_nothrow (1);
   return b;

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2011-01-19 21:04 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-11 23:28 [patch] Add tests for JIT debugging interface Paul Pluzhnikov
2011-01-12  7:02 ` Yao Qi
2011-01-12 10:44   ` Pedro Alves
2011-01-12 15:19     ` Paul Pluzhnikov
2011-01-12 16:12       ` Pedro Alves
2011-01-12 17:23       ` Yao Qi
2011-01-12 17:33         ` Paul Pluzhnikov
2011-01-18 17:07           ` Paul Pluzhnikov
2011-01-19 19:27           ` Pedro Alves
2011-01-19 20:30             ` Paul Pluzhnikov
2011-01-19 21:04               ` Pedro Alves
2011-01-19 21:09               ` Yao Qi
2011-01-12 10:03 ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).