[patch,7.3] Fix JIT clang-lli gdb-7.3 regression
Jan Kratochvil
jan.kratochvil@redhat.com
Wed Jul 6 12:10:00 GMT 2011
On Tue, 05 Jul 2011 23:06:47 +0200, Paul Pluzhnikov wrote:
> Another option is to apply a better fix (provided it is deemed safe) ...
It is still regressing against 739571cc3151651f49f7171cfd98275d983bfaaa^ with
the attached testcase:
FAIL: gdb.base/jit-so.exp: one_jit_test-1 run-2 info function jit_function
FAIL: gdb.base/jit-so.exp: one_jit_test-2 run-2 info function jit_function
It is also regressing on the former fix by me but it is no longer regressing
on the fix attached here.
That is on unload+reload of the JIT engine. OTOH (a) this case is probably
not faced by normal users and (b) it needs more work for better performance
and (c) in this case maybe the overall multi-JIT rework would be better.
So I am fine with the Paul's fix for 7.3; just the testcase there does not
work for me, I had to change:
- void *h = dlopen ("jit-dlmain-so.so", RTLD_LAZY);
+ void *h = dlopen ("gdb.base/jit-main.so", RTLD_LAZY);
And jit-dlmain.c was missing the GPL comment.
> It took me much longer to create a test case (I don't have 'lli'), then
> it did to fix the problem once I had the repro.
I agree that testcase of yours is based on better principle.
Thanks,
Jan
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -38,6 +38,15 @@ static const char *const jit_break_name = "__jit_debug_register_code";
static const char *const jit_descriptor_name = "__jit_debug_descriptor";
+/* This is a boolean indicating whether we're currently registering code. This
+ is used to avoid re-entering the registration code. We want to check for
+ new JITed every time a new object file is loaded, but we want to avoid
+ checking for new code while we're registering object files for JITed code.
+ Therefore, we flip this variable to 1 before registering new object files,
+ and set it to 0 before returning. */
+
+static int registering_code = 0;
+
static const struct inferior_data *jit_inferior_data = NULL;
/* Non-zero if we want to see trace of jit level stuff. */
@@ -295,9 +304,18 @@ JITed symbol file is not an object file, ignoring it.\n"));
++i;
}
+ /* Raise this flag while we register code so we won't trigger any
+ re-registration. */
+ gdb_assert (registering_code == 0);
+ my_cleanups = make_cleanup_restore_integer (®istering_code);
+ registering_code = 1;
+
/* This call takes ownership of sai. */
objfile = symbol_file_add_from_bfd (nbfd, 0, sai, OBJF_SHARED, NULL);
+ /* Clear the registering_code flag. */
+ do_cleanups (my_cleanups);
+
/* Remember a mapping from entry_addr to objfile. */
entry_addr_ptr = xmalloc (sizeof (CORE_ADDR));
*entry_addr_ptr = entry_addr;
@@ -332,41 +350,6 @@ jit_find_objf_with_entry_addr (CORE_ADDR entry_addr)
return NULL;
}
-/* (Re-)Initialize the jit breakpoint if necessary.
- Return 0 on success. */
-
-static int
-jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
- struct jit_inferior_data *inf_data)
-{
- if (inf_data->breakpoint_addr == 0)
- {
- struct minimal_symbol *reg_symbol;
-
- /* Lookup the registration symbol. If it is missing, then we assume
- we are not attached to a JIT. */
- reg_symbol = lookup_minimal_symbol (jit_break_name, NULL, NULL);
- if (reg_symbol == NULL)
- return 1;
- inf_data->breakpoint_addr = SYMBOL_VALUE_ADDRESS (reg_symbol);
- if (inf_data->breakpoint_addr == 0)
- return 2;
- }
- else
- return 0;
-
- if (jit_debug)
- fprintf_unfiltered (gdb_stdlog,
- "jit_breakpoint_re_set_internal, "
- "breakpoint_addr = %s\n",
- paddress (gdbarch, inf_data->breakpoint_addr));
-
- /* Put a breakpoint in the registration symbol. */
- create_jit_event_breakpoint (gdbarch, inf_data->breakpoint_addr);
-
- return 0;
-}
-
/* Register any already created translations. */
static void
@@ -375,29 +358,54 @@ jit_inferior_init (struct gdbarch *gdbarch)
struct jit_descriptor descriptor;
struct jit_code_entry cur_entry;
struct jit_inferior_data *inf_data;
- CORE_ADDR cur_entry_addr;
+ CORE_ADDR cur_entry_addr, breakpoint_addr;
+ struct minimal_symbol *reg_symbol, *desc_symbol;
if (jit_debug)
- fprintf_unfiltered (gdb_stdlog, "jit_inferior_init\n");
+ fprintf_unfiltered (gdb_stdlog,
+ "jit_inferior_init, registering_code = %d\n",
+ registering_code);
- inf_data = get_jit_inferior_data ();
- if (jit_breakpoint_re_set_internal (gdbarch, inf_data) != 0)
+ /* When we register code, GDB resets its breakpoints in case symbols have
+ changed. That in turn calls this handler, which makes us look for new
+ code again. To avoid being re-entered, we check this flag. */
+ if (registering_code)
return;
- if (inf_data->descriptor_addr == 0)
- {
- struct minimal_symbol *desc_symbol;
+ inf_data = get_jit_inferior_data ();
- /* Lookup the descriptor symbol and cache the addr. If it is
- missing, we assume we are not attached to a JIT and return early. */
- desc_symbol = lookup_minimal_symbol (jit_descriptor_name, NULL, NULL);
- if (desc_symbol == NULL)
- return;
+ /* Lookup the registration symbol. If it is missing, then we assume
+ we are not attached to a JIT. */
+ breakpoint_addr = 0;
+ reg_symbol = lookup_minimal_symbol (jit_break_name, NULL, NULL);
+ if (reg_symbol != NULL)
+ breakpoint_addr = SYMBOL_VALUE_ADDRESS (reg_symbol);
+
+ if (jit_debug)
+ fprintf_unfiltered (gdb_stdlog,
+ "jit_inferior_init, new breakpoint_addr = %s, "
+ "old breakpoint_addr = %s\n",
+ paddress (gdbarch, breakpoint_addr),
+ paddress (gdbarch, inf_data->breakpoint_addr));
- inf_data->descriptor_addr = SYMBOL_VALUE_ADDRESS (desc_symbol);
- if (inf_data->descriptor_addr == 0)
- return;
+ remove_jit_event_breakpoints ();
+ if (breakpoint_addr != 0)
+ {
+ /* Put a breakpoint in the registration symbol. */
+ create_jit_event_breakpoint (gdbarch, breakpoint_addr);
}
+ inf_data->breakpoint_addr = breakpoint_addr;
+
+ /* Lookup the descriptor symbol and cache the addr. If it is
+ missing, we assume we are not attached to a JIT and return early. */
+ inf_data->descriptor_addr = 0;
+ desc_symbol = lookup_minimal_symbol (jit_descriptor_name, NULL, NULL);
+ if (desc_symbol == NULL)
+ return;
+
+ inf_data->descriptor_addr = SYMBOL_VALUE_ADDRESS (desc_symbol);
+ if (inf_data->descriptor_addr == 0)
+ return;
if (jit_debug)
fprintf_unfiltered (gdb_stdlog,
@@ -443,8 +451,7 @@ jit_inferior_created_hook (void)
void
jit_breakpoint_re_set (void)
{
- jit_breakpoint_re_set_internal (target_gdbarch,
- get_jit_inferior_data ());
+ jit_inferior_init (target_gdbarch);
}
/* Reset inferior_data, so sybols will be looked up again, and jit_breakpoint
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-clang.c
@@ -0,0 +1,30 @@
+/* This testcase 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/>. */
+
+#include <stdlib.h>
+
+static void
+jit_clang_function_name (void)
+{
+ abort ();
+}
+
+int
+main (void)
+{
+ jit_clang_function_name ();
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-clang.exp
@@ -0,0 +1,52 @@
+# Copyright (C) 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/>.
+
+# gdbserver configuration does not support passing arguments to `lli'.
+if { ![isnative] || [is_remote target] } {
+ return
+}
+
+set testfile "jit-clang"
+set srcfile ${testfile}.c
+set binfile $objdir/$subdir/${testfile}.bc
+
+if {[catch "system \"clang -fexceptions $srcdir/$subdir/$srcfile -c -emit-llvm -o ${binfile}\""] != 0} {
+ verbose -log "clang compilation failed"
+ untested ${testfile}.exp
+ return -1
+}
+if {[catch "system \"which lli &>/dev/null\""] != 0} {
+ verbose -log "lli not found"
+ untested ${testfile}.exp
+ return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+
+if [gdb_load "lli"] {
+ untested ${testfile}.exp
+ return -1
+}
+
+gdb_test_no_output "set args -jit-emit-debug ${binfile}" "set args"
+gdb_run_cmd
+
+# The error was:
+# Unable to read JIT descriptor from remote memory!
+gdb_test "" "\r\nProgram received signal SIGABRT.*" "run"
+
+gdb_test "bt" " jit_clang_function_name .*" "jit registration succeeded"
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-dlmain.c
@@ -0,0 +1,40 @@
+#include <dlfcn.h>
+#include <stdio.h>
+
+static int
+run (int argc, char *argv[])
+{
+ void *h = dlopen ("gdb.base/jit-main.so", RTLD_LAZY);
+ int (*p_main) (int, char **);
+ int rc;
+
+ if (h == NULL) return 1;
+
+ p_main = dlsym (h, "jit_dl_main");
+ if (p_main == NULL) return 2;
+
+ h = h; /* break here after-dlopen */
+ rc = (*p_main) (argc, argv);
+ if (rc != 0)
+ return rc;
+
+ if (dlclose (h) != 0)
+ return 3;
+
+ return 0;
+}
+
+int main (int argc, char *argv[])
+{
+ int rc;
+
+ rc = run (argc, argv);
+ if (rc != 0)
+ return rc;
+
+ rc = run (argc, argv);
+ if (rc != 0)
+ return rc + 10;
+
+ return 0;
+}
--- a/gdb/testsuite/gdb.base/jit-main.c
+++ b/gdb/testsuite/gdb.base/jit-main.c
@@ -117,8 +117,12 @@ update_locations (const void *const addr, int idx)
}
}
+#ifndef MAIN
+#define MAIN main
+#endif
+
int
-main (int argc, char *argv[])
+MAIN (int argc, char *argv[])
{
/* These variables are here so they can easily be set from jit.exp. */
const char *libname = NULL;
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-so.exp
@@ -0,0 +1,137 @@
+# 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/>.
+
+# The same tests as in jit.exp, but loading JITer itself from a shared
+# library.
+
+if $tracelevel {
+ strace $tracelevel
+}
+
+if {[skip_shlib_tests]} {
+ untested jit-so.exp
+ return -1
+}
+
+if {[get_compiler_info not-used]} {
+ warning "Could not get compiler info"
+ untested jit-so.exp
+ return 1
+}
+
+#
+# test running programs
+#
+
+set testfile jit-dlmain
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug shlib_load}] != "" } {
+ untested jit-so.exp
+ return -1
+}
+
+set testfile2 jit-main
+set srcfile2 ${testfile2}.c
+set binfile2 ${objdir}/${subdir}/${testfile2}.so
+if { [gdb_compile_shlib "${srcdir}/${subdir}/${srcfile2}" ${binfile2} {debug additional_flags="-DMAIN=jit_dl_main"}] != "" } {
+ 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"
+set solib_binfile_test_msg "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} {}] != "" } {
+ untested jit-so.exp
+ return -1
+}
+
+proc run {run count match_str} {
+ global verbose testfile srcfile2 solib_binfile solib_binfile_test_msg pf_prefix srcfile
+
+ set old_pf_prefix $pf_prefix
+ lappend pf_prefix "run-$run"
+
+ gdb_breakpoint $srcfile:[gdb_get_line_number "break here after-dlopen" ]
+ gdb_continue_to_breakpoint "break here after-dlopen"
+
+ gdb_breakpoint "$srcfile2:[gdb_get_line_number {break here 0} $srcfile2]"
+ 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_no_output "set var argc = 2"
+ gdb_test_no_output "set var libname = \"$solib_binfile\"" "set var libname = \"$solib_binfile_test_msg\""
+ gdb_test_no_output "set var count = $count"
+
+ gdb_breakpoint "$srcfile2:[gdb_get_line_number {break here 1} $srcfile2]"
+ 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_test "maintenance info break"
+ }
+
+ gdb_breakpoint "$srcfile2:[gdb_get_line_number {break here 2} $srcfile2]"
+ 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
+}
+
+proc one_jit_test {count match_str} {
+ global verbose testfile pf_prefix
+
+ set old_pf_prefix $pf_prefix
+ lappend pf_prefix "one_jit_test-$count"
+
+ clean_restart $testfile
+
+ # Error is OK if not supported by the platform.
+ gdb_test "set disable-randomization off"
+
+ # 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
+ }
+
+ run 1 $count $match_str
+
+ # Second run with reloaded JITer.
+ gdb_test_no_output "delete breakpoints" "delete breakpoints" \
+ {Delete all breakpoints\? \(y or n\) } "y"
+ run 2 $count $match_str
+
+ 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"
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -923,6 +923,13 @@ proc gdb_test_no_output { args } {
set message $command
}
+ if [llength $args]==4 {
+ set question_string [lindex $args 2];
+ set response_string [lindex $args 3];
+ } else {
+ set question_string "^FOOBAR$"
+ }
+
set command_regex [string_to_regexp $command]
gdb_test_multiple $command $message {
-re "^$command_regex\r\n$gdb_prompt $" {
@@ -930,6 +937,16 @@ proc gdb_test_no_output { args } {
pass "$message"
}
}
+ -re "^$command_regex\r\n${question_string}$" {
+ # $command_regex is already precompiled above.
+ gdb_test_multiple $response_string $message {
+ -re "^[string_to_regexp $response_string]\r\n$gdb_prompt $" {
+ if ![string match "" $message] then {
+ pass "$message"
+ }
+ }
+ }
+ }
}
}
More information about the Gdb-patches
mailing list