From patchwork Fri Apr 19 15:03:47 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 32346 Received: (qmail 111984 invoked by alias); 19 Apr 2019 15:03:52 -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 111975 invoked by uid 89); 19 Apr 2019 15:03:52 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-19.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.1 spammy=Filter, pspace, sk:gdbarch, U*simon.marchi X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 19 Apr 2019 15:03:49 +0000 Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id D20B51E175; Fri, 19 Apr 2019 11:03:47 -0400 (EDT) Subject: [PATCH] solib-svr4: Pass down svr4_info as much as possible (was: [PATCH] Fix "nosharedlibrary + continue + shared lib event" crash) From: Simon Marchi To: Pedro Alves , gdb-patches@sourceware.org References: <20190409131410.10205-1-palves@redhat.com> Message-ID: Date: Fri, 19 Apr 2019 11:03:47 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.3 MIME-Version: 1.0 In-Reply-To: On 2019-04-10 10:49 p.m., Simon Marchi wrote: > I am not able to reproduce the problem, and the test doesn't fail here, without > the rest of the patch applied. I think it's because on my system gdb doesn't use > the probe based interface. > > Anyhow, the fix makes sense. Since the probe is deleted on objfile destruction, the > corresponding probe_and_action structures should too. > > Simon While reviewing this patch, I had written the patch below to experiment, and while it's not super important, I think it's a good cleanup. From aedf5f7d846672ba6edc2780853baa43f35dd3c4 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 10 Apr 2019 22:02:33 -0400 Subject: [PATCH] solib-svr4: Pass down svr4_info as much as possible While reviewing https://sourceware.org/ml/gdb-patches/2019-04/msg00141.html I noticed that we relied heavily on global state through the get_svr4_info function, which uses current_program_space. I thought we could improve this (make things more explicit and easier to follow) by - Making get_svr4_info accept a program_space parameter, making it return the SVR4 info for that program space. - Passing down the svr4_info object from callers as much as possible. This means looking up the svr4_info for the appropriate program space at the entry points of the solib-svr4.c file and passing it down. For now, these entry points (most of them are "methods" of svr4_so_ops) rely on current_program_space, but we can later try to change the target_so_ops interface to pass down the program space. gdb/ChangeLog: * solib-svr4.c (get_svr4_info): Add pspace parameter. (svr4_keep_data_in_core): Pass current_program_space to get_svr4_info. (open_symbol_file_object): Likewise. (svr4_default_sos): Add info parameter. (svr4_read_so_list): Likewise. (svr4_current_sos_direct): Adjust functions calls to pass down info. (svr4_current_sos_1): Add info parameter. (svr4_current_sos): Call get_svr4_info, pass info down to svr4_current_sos_1. (svr4_fetch_objfile_link_map): Pass objfile->pspace to get_svr4_info. (svr4_in_dynsym_resolve_code): Pass current_program_space to get_svr4_info. (probes_table_htab_remove_objfile_probes): Pass objfile->pspace to get_svr4_info. (probes_table_remove_objfile_probes): Likewise. (register_solib_event_probe): Add info parameter. (solist_update_incremental): Pass info parameter down to svr4_read_so_list. (disable_probes_interface): Add info parameter. (svr4_handle_solib_event): Pass current_program_space to get_svr4_info. Adjust disable_probes_interface cleanup. (svr4_create_probe_breakpoints): Add info parameter, pass it down to register_solib_event_probe. (svr4_create_solib_event_breakpoints): Add info parameter, pass it down to svr4_create_probe_breakpoints. (enable_break): Pass info down to svr4_create_solib_event_breakpoints. (svr4_solib_create_inferior_hook): Pass current_program_space to get_svr4_info. (svr4_clear_solib): Likewise. --- gdb/solib-svr4.c | 84 +++++++++++++++++++++++------------------------- 1 file changed, 41 insertions(+), 43 deletions(-) diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c index 2c79dfec2bb6..2aa7b95ce6c1 100644 --- a/gdb/solib-svr4.c +++ b/gdb/solib-svr4.c @@ -391,21 +391,21 @@ svr4_pspace_data_cleanup (struct program_space *pspace, void *arg) xfree (info); } -/* Get the current svr4 data. If none is found yet, add it now. This - function always returns a valid object. */ +/* Get the svr4 data for program space PSPACE. If none is found yet, add it now. + This function always returns a valid object. */ static struct svr4_info * -get_svr4_info (void) +get_svr4_info (program_space *pspace) { struct svr4_info *info; - info = (struct svr4_info *) program_space_data (current_program_space, + info = (struct svr4_info *) program_space_data (pspace, solib_svr4_pspace_data); if (info != NULL) return info; info = XCNEW (struct svr4_info); - set_program_space_data (current_program_space, solib_svr4_pspace_data, info); + set_program_space_data (pspace, solib_svr4_pspace_data, info); return info; } @@ -940,7 +940,7 @@ svr4_keep_data_in_core (CORE_ADDR vaddr, unsigned long size) CORE_ADDR ldsomap; CORE_ADDR name_lm; - info = get_svr4_info (); + info = get_svr4_info (current_program_space); info->debug_base = 0; locate_base (info); @@ -969,7 +969,7 @@ open_symbol_file_object (int from_tty) struct type *ptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr; int l_name_size = TYPE_LENGTH (ptr_type); gdb::byte_vector l_name_buf (l_name_size); - struct svr4_info *info = get_svr4_info (); + struct svr4_info *info = get_svr4_info (current_program_space); symfile_add_flags add_flags = 0; if (from_tty) @@ -1264,9 +1264,8 @@ svr4_current_sos_via_xfer_libraries (struct svr4_library_list *list, linker, build a fallback list from other sources. */ static struct so_list * -svr4_default_sos (void) +svr4_default_sos (svr4_info *info) { - struct svr4_info *info = get_svr4_info (); struct so_list *newobj; if (!info->debug_loader_offset_p) @@ -1296,7 +1295,7 @@ svr4_default_sos (void) represent only part of the inferior library list. */ static int -svr4_read_so_list (CORE_ADDR lm, CORE_ADDR prev_lm, +svr4_read_so_list (svr4_info *info, CORE_ADDR lm, CORE_ADDR prev_lm, struct so_list ***link_ptr_ptr, int ignore_first) { CORE_ADDR first_l_name = 0; @@ -1331,8 +1330,6 @@ svr4_read_so_list (CORE_ADDR lm, CORE_ADDR prev_lm, decide when to ignore it. */ if (ignore_first && li->l_prev == 0) { - struct svr4_info *info = get_svr4_info (); - first_l_name = li->l_name; info->main_lm_addr = li->lm_addr; continue; @@ -1400,7 +1397,7 @@ svr4_current_sos_direct (struct svr4_info *info) if (library_list.main_lm) info->main_lm_addr = library_list.main_lm; - return library_list.head ? library_list.head : svr4_default_sos (); + return library_list.head ? library_list.head : svr4_default_sos (info); } /* Always locate the debug struct, in case it has moved. */ @@ -1410,7 +1407,7 @@ svr4_current_sos_direct (struct svr4_info *info) /* If we can't find the dynamic linker's base structure, this must not be a dynamically linked executable. Hmm. */ if (! info->debug_base) - return svr4_default_sos (); + return svr4_default_sos (info); /* Assume that everything is a library if the dynamic loader was loaded late by a static executable. */ @@ -1428,7 +1425,7 @@ svr4_current_sos_direct (struct svr4_info *info) `struct so_list' nodes. */ lm = solib_svr4_r_map (info); if (lm) - svr4_read_so_list (lm, 0, &link_ptr, ignore_first); + svr4_read_so_list (info, lm, 0, &link_ptr, ignore_first); /* On Solaris, the dynamic linker is not in the normal list of shared objects, so make sure we pick it up too. Having @@ -1436,12 +1433,12 @@ svr4_current_sos_direct (struct svr4_info *info) for skipping dynamic linker resolver code. */ lm = solib_svr4_r_ldsomap (info); if (lm) - svr4_read_so_list (lm, 0, &link_ptr, 0); + svr4_read_so_list (info, lm, 0, &link_ptr, 0); cleanup.release (); if (head == NULL) - return svr4_default_sos (); + return svr4_default_sos (info); return head; } @@ -1450,10 +1447,8 @@ svr4_current_sos_direct (struct svr4_info *info) method. */ static struct so_list * -svr4_current_sos_1 (void) +svr4_current_sos_1 (svr4_info *info) { - struct svr4_info *info = get_svr4_info (); - /* If the solib list has been read and stored by the probes interface then we return a copy of the stored list. */ if (info->solib_list != NULL) @@ -1468,7 +1463,8 @@ svr4_current_sos_1 (void) static struct so_list * svr4_current_sos (void) { - struct so_list *so_head = svr4_current_sos_1 (); + svr4_info *info = get_svr4_info (current_program_space); + struct so_list *so_head = svr4_current_sos_1 (info); struct mem_range vsyscall_range; /* Filter out the vDSO module, if present. Its symbol file would @@ -1548,7 +1544,7 @@ CORE_ADDR svr4_fetch_objfile_link_map (struct objfile *objfile) { struct so_list *so; - struct svr4_info *info = get_svr4_info (); + struct svr4_info *info = get_svr4_info (objfile->pspace); /* Cause svr4_current_sos() to be run if it hasn't been already. */ if (info->main_lm_addr == 0) @@ -1601,7 +1597,7 @@ match_main (const char *soname) int svr4_in_dynsym_resolve_code (CORE_ADDR pc) { - struct svr4_info *info = get_svr4_info (); + struct svr4_info *info = get_svr4_info (current_program_space); return ((pc >= info->interp_text_sect_low && pc < info->interp_text_sect_high) @@ -1681,7 +1677,7 @@ probes_table_htab_remove_objfile_probes (void **slot, void *info) struct objfile *objfile = (struct objfile *) info; if (pa->objfile == objfile) - htab_clear_slot (get_svr4_info ()->probes_table, slot); + htab_clear_slot (get_svr4_info (objfile->pspace)->probes_table, slot); return 1; } @@ -1691,7 +1687,7 @@ probes_table_htab_remove_objfile_probes (void **slot, void *info) static void probes_table_remove_objfile_probes (struct objfile *objfile) { - svr4_info *info = get_svr4_info (); + svr4_info *info = get_svr4_info (objfile->pspace); if (info->probes_table != nullptr) htab_traverse_noresize (info->probes_table, probes_table_htab_remove_objfile_probes, objfile); @@ -1701,11 +1697,10 @@ probes_table_remove_objfile_probes (struct objfile *objfile) probes table. */ static void -register_solib_event_probe (struct objfile *objfile, +register_solib_event_probe (svr4_info *info, struct objfile *objfile, probe *prob, CORE_ADDR address, enum probe_action action) { - struct svr4_info *info = get_svr4_info (); struct probe_and_action lookup, *pa; void **slot; @@ -1857,7 +1852,7 @@ solist_update_incremental (struct svr4_info *info, CORE_ADDR lm) above check and deferral to solist_update_full ensures that this call to svr4_read_so_list will never see the first element. */ - if (!svr4_read_so_list (lm, prev_lm, &link, 0)) + if (!svr4_read_so_list (info, lm, prev_lm, &link, 0)) return 0; } @@ -1869,10 +1864,8 @@ solist_update_incremental (struct svr4_info *info, CORE_ADDR lm) ones set up for the probes-based interface are adequate. */ static void -disable_probes_interface () +disable_probes_interface (svr4_info *info) { - struct svr4_info *info = get_svr4_info (); - warning (_("Probes-based dynamic linker interface failed.\n" "Reverting to original interface.\n")); @@ -1887,7 +1880,7 @@ disable_probes_interface () static void svr4_handle_solib_event (void) { - struct svr4_info *info = get_svr4_info (); + struct svr4_info *info = get_svr4_info (current_program_space); struct probe_and_action *pa; enum probe_action action; struct value *val = NULL; @@ -1900,7 +1893,10 @@ svr4_handle_solib_event (void) /* If anything goes wrong we revert to the original linker interface. */ - auto cleanup = make_scope_exit (disable_probes_interface); + auto cleanup = make_scope_exit ([info] () + { + disable_probes_interface (info); + }); pc = regcache_read_pc (get_current_regcache ()); pa = solib_event_probe_at (info, pc); @@ -2055,7 +2051,7 @@ svr4_update_solib_event_breakpoints (void) probe. */ static void -svr4_create_probe_breakpoints (struct gdbarch *gdbarch, +svr4_create_probe_breakpoints (svr4_info *info, struct gdbarch *gdbarch, const std::vector *probes, struct objfile *objfile) { @@ -2068,7 +2064,7 @@ svr4_create_probe_breakpoints (struct gdbarch *gdbarch, CORE_ADDR address = p->get_relocated_address (objfile); create_solib_event_breakpoint (gdbarch, address); - register_solib_event_probe (objfile, p, address, action); + register_solib_event_probe (info, objfile, p, address, action); } } @@ -2088,7 +2084,7 @@ svr4_create_probe_breakpoints (struct gdbarch *gdbarch, marker function. */ static void -svr4_create_solib_event_breakpoints (struct gdbarch *gdbarch, +svr4_create_solib_event_breakpoints (svr4_info *info, struct gdbarch *gdbarch, CORE_ADDR address) { struct obj_section *os; @@ -2151,7 +2147,7 @@ svr4_create_solib_event_breakpoints (struct gdbarch *gdbarch, } if (all_probes_found) - svr4_create_probe_breakpoints (gdbarch, probes, os->objfile); + svr4_create_probe_breakpoints (info, gdbarch, probes, os->objfile); if (all_probes_found) return; @@ -2282,7 +2278,7 @@ enable_break (struct svr4_info *info, int from_tty) + bfd_section_size (tmp_bfd, interp_sect); } - svr4_create_solib_event_breakpoints (target_gdbarch (), sym_addr); + svr4_create_solib_event_breakpoints (info, target_gdbarch (), sym_addr); return 1; } } @@ -2444,7 +2440,7 @@ enable_break (struct svr4_info *info, int from_tty) if (sym_addr != 0) { - svr4_create_solib_event_breakpoints (target_gdbarch (), + svr4_create_solib_event_breakpoints (info, target_gdbarch (), load_addr + sym_addr); return 1; } @@ -2470,7 +2466,8 @@ enable_break (struct svr4_info *info, int from_tty) sym_addr = gdbarch_convert_from_func_ptr_addr (target_gdbarch (), sym_addr, current_top_target ()); - svr4_create_solib_event_breakpoints (target_gdbarch (), sym_addr); + svr4_create_solib_event_breakpoints (info, target_gdbarch (), + sym_addr); return 1; } } @@ -2487,7 +2484,8 @@ enable_break (struct svr4_info *info, int from_tty) sym_addr = gdbarch_convert_from_func_ptr_addr (target_gdbarch (), sym_addr, current_top_target ()); - svr4_create_solib_event_breakpoints (target_gdbarch (), sym_addr); + svr4_create_solib_event_breakpoints (info, target_gdbarch (), + sym_addr); return 1; } } @@ -3010,7 +3008,7 @@ svr4_solib_create_inferior_hook (int from_tty) { struct svr4_info *info; - info = get_svr4_info (); + info = get_svr4_info (current_program_space); /* Clear the probes-based interface's state. */ free_probes_table (info); @@ -3036,7 +3034,7 @@ svr4_clear_solib (void) { struct svr4_info *info; - info = get_svr4_info (); + info = get_svr4_info (current_program_space); info->debug_base = 0; info->debug_loader_offset_p = 0; info->debug_loader_offset = 0;