From patchwork Thu Dec 19 10:28:23 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Simon Marchi (Code Review)" X-Patchwork-Id: 36959 Received: (qmail 33504 invoked by alias); 19 Dec 2019 10:28:40 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 33433 invoked by uid 89); 19 Dec 2019 10:28:40 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT autolearn=ham version=3.3.1 spammy=objfile* X-HELO: mx1.osci.io Received: from polly.osci.io (HELO mx1.osci.io) (8.43.85.229) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 19 Dec 2019 10:28:36 +0000 Received: by mx1.osci.io (Postfix, from userid 994) id 1423F2032B; Thu, 19 Dec 2019 05:28:35 -0500 (EST) Received: from gnutoolchain-gerrit.osci.io (gnutoolchain-gerrit.osci.io [IPv6:2620:52:3:1:5054:ff:fe06:16ca]) by mx1.osci.io (Postfix) with ESMTP id 367082032B; Thu, 19 Dec 2019 05:28:24 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by gnutoolchain-gerrit.osci.io (Postfix) with ESMTP id 0D8142816F; Thu, 19 Dec 2019 05:28:24 -0500 (EST) X-Gerrit-PatchSet: 3 Date: Thu, 19 Dec 2019 05:28:23 -0500 From: "Mihails Strasuns (Code Review)" To: Pedro Alves , Tom Tromey , gdb-patches@sourceware.org Cc: Luis Machado , Simon Marchi Auto-Submitted: auto-generated X-Gerrit-MessageType: newpatchset Subject: [review v3] jit: remove bp locations when unregistering jit code X-Gerrit-Change-Id: Id9133540d67fa0c4619ac88324b0349b89e4b2b1 X-Gerrit-Change-Number: 704 X-Gerrit-ChangeURL: X-Gerrit-Commit: 6b4a06b02cab47cc41c011341f7a34f809bc6603 In-Reply-To: References: Reply-To: mihails.strasuns@intel.com, simon.marchi@polymtl.ca, tromey@sourceware.org, palves@redhat.com, luis.machado@linaro.org, gdb-patches@sourceware.org MIME-Version: 1.0 Content-Disposition: inline User-Agent: Gerrit/3.0.3-79-g83ff7f88f1 Message-Id: <20191219102824.0D8142816F@gnutoolchain-gerrit.osci.io> Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/704 ...................................................................... jit: remove bp locations when unregistering jit code Fixes the rare problem when identical JIT code is registered and unregistered several times and its symbols happen to be relocated to the exactly the same addresses as previous time. In such situation gdb would still think that old breakpoint locations are still inserted (because `inserted` flag is set and objfile/address is the same) despite the fact that code memory was overwritten and does not contain a breakpoint trap anymore. This solution removes such breakpoint location from the respective breakpoint location list. That way next time `update_breakpoint_locations` is called, it will re-create a new bp_location instance if that JIT code was loaded again, and won't try reusing the previous one. It is a conservative solution which only applies to object files coming from JIT. Added test case tries to reproduce the problem using the opencl test suite (because current jit test suite doesn't actually execute the code). Without this patch the breakpoint would only trigger during the first kernel run and get completely ignored second run. gdb/ChangeLog: 2019-11-14 Mihails Strasuns * breakpoint.h, breakpoint.c (forget_breakpoint_locations_obj): new function to forget all breakpoint locations for a given objfile. * jit.c (jit_unregister_code): call `forget_breakpoint_locations_obj`. gdb/testsuite/ChangeLog: 2019-11-14 Mihails Strasuns * jit-reregister.exp, jit-reregister.c: new test case to verify that breakpoint is hit even when JIT code is unregistered and registered again. Signed-off-by: Mihails Strasuns Change-Id: Id9133540d67fa0c4619ac88324b0349b89e4b2b1 --- M gdb/breakpoint.c M gdb/breakpoint.h M gdb/jit.c A gdb/testsuite/gdb.base/jit-reregister.c A gdb/testsuite/gdb.base/jit-reregister.exp 5 files changed, 181 insertions(+), 9 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 904abda..ab1139d 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -3066,6 +3066,50 @@ } } +/* Tries to find `bp` in the linked list of `b` locations + and if found, removes it. In practice that means that breakpoint + will forget about this location and won't try to re-insert it. */ + +static bool +remove_location_from_bp (breakpoint *b, bp_location *bp) +{ + if (b->loc == bp) + { + b->loc = nullptr; + return true; + } + + bp_location *i; + for (i = b->loc; i != nullptr && i->next != bp; i = i->next) + ; + if (i == nullptr) + return false; + i->next = i->next->next; + return true; +} + +/* See breakpoint.h. */ + +int +forget_breakpoint_locations_obj (objfile *obj) +{ + struct bp_location **blp, *bl; + + int count = 0; + + ALL_BP_LOCATIONS (bl, blp) + { + if (is_addr_in_objfile (bl->address, obj)) + { + bool ret = remove_location_from_bp (bl->owner, bl); + gdb_assert (ret); + ++count; + } + } + + return count; +} + static int internal_breakpoint_number = -1; /* Set the breakpoint number of B, depending on the value of INTERNAL. diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 3197854..9105970 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -1411,6 +1411,13 @@ extern void remove_breakpoints_inf (inferior *inf); +/* Forgets all breakpoint locations that are inserted + for a given objfile by removing them from the owning breakpoint + linked list. Will not affect actual breakpoints and + if the object file remains these may be re-created during next + `update_breakpoint_locations` call. */ +extern int forget_breakpoint_locations_obj (objfile* obj); + /* This function can be used to update the breakpoint package's state after an exec() system call has been executed. diff --git a/gdb/jit.c b/gdb/jit.c index 0dd11e1..6ed5f51 100644 --- a/gdb/jit.c +++ b/gdb/jit.c @@ -923,6 +923,32 @@ return NULL; } +/* This function unregisters JITed code and does the necessary cleanup. */ + +static void +jit_unregister_code (struct gdbarch *gdbarch, CORE_ADDR entry_addr) +{ + struct objfile *objfile = nullptr; + if (jit_debug) + fprintf_unfiltered (gdb_stdlog, "jit_unregister_code (%s)\n", + host_address_to_string (objfile)); + objfile = jit_find_objf_with_entry_addr (entry_addr); + if (objfile == NULL) + printf_unfiltered (_ ("Unable to find JITed code " + "entry at address: %s\n"), + paddress (gdbarch, entry_addr)); + else + { + int count = forget_breakpoint_locations_obj (objfile); + if (jit_debug && count > 0) + fprintf_unfiltered (gdb_stdlog, + "jit_unregister_code, ignoring %d breakpoint" + "locations in the unregistered code\n", + count); + objfile->unlink (); + } +} + /* This is called when a breakpoint is deleted. It updates the inferior's cache, if needed. */ @@ -1333,7 +1359,6 @@ struct jit_descriptor descriptor; struct jit_code_entry code_entry; CORE_ADDR entry_addr; - struct objfile *objf; /* Read the descriptor from remote memory. */ if (!jit_read_descriptor (gdbarch, &descriptor, @@ -1351,14 +1376,7 @@ jit_register_code (gdbarch, entry_addr, &code_entry); break; case JIT_UNREGISTER: - objf = jit_find_objf_with_entry_addr (entry_addr); - if (objf == NULL) - printf_unfiltered (_("Unable to find JITed code " - "entry at address: %s\n"), - paddress (gdbarch, entry_addr)); - else - objf->unlink (); - + jit_unregister_code (gdbarch, entry_addr); break; default: error (_("Unknown action_flag value in JIT descriptor!")); diff --git a/gdb/testsuite/gdb.base/jit-reregister.c b/gdb/testsuite/gdb.base/jit-reregister.c new file mode 100644 index 0000000..3d63f14 --- /dev/null +++ b/gdb/testsuite/gdb.base/jit-reregister.c @@ -0,0 +1,58 @@ +/* This test program is part of GDB, the GNU debugger. + + Copyright 2019 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 . */ + +#include "jit-defs.h" +#include "jit-elf.h" + +int +main (int argc, char *argv[]) +{ + size_t obj_size; + ElfW (Addr) addr = load_elf (argv[1], &obj_size, 0); + update_locations (addr, -1); + int (*jit_function) () + = (int (*) ()) load_symbol (addr, "jit_function_XXXX"); + + struct jit_code_entry *const entry + = (struct jit_code_entry *) calloc (1, sizeof (*entry)); + + entry->symfile_addr = (const char *) addr; + entry->symfile_size = obj_size; + entry->prev_entry = __jit_debug_descriptor.relevant_entry; + __jit_debug_descriptor.relevant_entry = entry; + __jit_debug_descriptor.first_entry = entry; + __jit_debug_descriptor.action_flag = JIT_REGISTER_FN; + __jit_debug_register_code (); + + jit_function (); /* first-call */ + + __jit_debug_descriptor.action_flag = JIT_UNREGISTER_FN; + __jit_debug_register_code (); + + addr = load_elf (argv[1], &obj_size, addr); + update_locations (addr, -1); + jit_function = (int (*) ()) load_symbol (addr, "jit_function_XXXX"); + + entry->symfile_addr = (const char *) addr; + entry->symfile_size = obj_size; + __jit_debug_descriptor.relevant_entry = entry; + __jit_debug_descriptor.first_entry = entry; + __jit_debug_descriptor.action_flag = JIT_REGISTER_FN; + __jit_debug_register_code (); + + jit_function (); +} diff --git a/gdb/testsuite/gdb.base/jit-reregister.exp b/gdb/testsuite/gdb.base/jit-reregister.exp new file mode 100644 index 0000000..23de587 --- /dev/null +++ b/gdb/testsuite/gdb.base/jit-reregister.exp @@ -0,0 +1,45 @@ +# Copyright 2019 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 . */ +# +# Tests global symbol lookup in case a symbol's name in the kernel +# coincides with another in the main executable. + +standard_testfile .c + +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } { + return -1 +} + +set solib "jit-solib" +set solib_src "${srcdir}/${subdir}/${solib}.c" +set solib_bin [standard_output_file ${solib}.so] + +if { [gdb_compile_shlib "${solib_src}" "${solib_bin}" {debug}] != "" } { + untested "failed to compile jit shared library" + return -1 +} + +set solib_bin_target [gdb_remote_download target ${solib_bin}] + +clean_restart ${binfile} + +gdb_test "start ${solib_bin_target}" +gdb_breakpoint [gdb_get_line_number "first-call"] {temporary} +gdb_test "set debug jit 1" +gdb_continue_to_breakpoint "first-call" + +gdb_breakpoint "jit_function_XXXX" +gdb_continue_to_breakpoint "jit_function_XXXX" +gdb_continue_to_breakpoint "jit_function_XXXX" \ No newline at end of file