From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 69C303858C5E for ; Tue, 10 Oct 2023 20:44:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 69C303858C5E Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=efficios.com Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=efficios.com Received: from smarchi-efficios.internal.efficios.com (192-222-143-198.qc.cable.ebox.net [192.222.143.198]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 0F7701E11B; Tue, 10 Oct 2023 16:44:18 -0400 (EDT) From: Simon Marchi To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH 12/24] gdb: make solib-svr4 not use so_list internally Date: Tue, 10 Oct 2023 16:40:07 -0400 Message-ID: <20231010204213.111285-13-simon.marchi@efficios.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231010204213.111285-1-simon.marchi@efficios.com> References: <20231010204213.111285-1-simon.marchi@efficios.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3496.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_NONE,KAM_DMARC_STATUS,SPF_HELO_PASS,SPF_SOFTFAIL,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: A subsequent patch makes use of non-trivial types in struct so_list. This trips on the fact that svr4_copy_library_list uses memcpy to copy so_list objects: so_list *newobj = new so_list; memcpy (newobj, src, sizeof (struct so_list)); solib-svr4 maintains lists of so_list objects in its own internal data structures. When requested to return a list of so_list objects (through target_so_ops::current_sos), it duplicates the internal so_list lists, using memcpy. When changing so_list to make it non-trivial, we would need to replace this use of memcpy somehow. That would mean making so_list copyable, with all the complexity that entails, just to satisfy this internal usage of solib-svr4 (and solib-rocm, which does the same). Change solib-svr4 to use its own data type for its internal lists. The use of so_list is a bit overkill anyway, as most fields of so_list are irrelevant for this internal use. - Introduce svr4_so, which contains just an std::string for the name and a unique_ptr for the lm_info. - Change the internal so_list lists to be std::vector. Vector seems like a good choice for this, we don't need to insert/remove elements in the middle of these internal lists. - Remove svr4_free_library_list, free_solib_lists and ~svr4_info, as everything is managed automatically now. - Replace svr4_copy_library_list (which duplicated internal lists in order to return them to the core) with so_list_from_svr4_sos, which creates an so_list list from a vector of svr4_so. - Generalize svr4_same a bit, because find_debug_base_for_solib now needs to compare an so_list and an svr4_so to see if they are the same. Change-Id: I6012e48e07aace2a8172b74b389f9547ce777877 --- gdb/solib-svr4.c | 275 +++++++++++++++++------------------------------ gdb/solib-svr4.h | 2 + 2 files changed, 100 insertions(+), 177 deletions(-) diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c index 7d247fa455e9..9266929d2895 100644 --- a/gdb/solib-svr4.c +++ b/gdb/solib-svr4.c @@ -50,7 +50,6 @@ static struct link_map_offsets *svr4_fetch_link_map_offsets (void); static int svr4_have_link_map_offsets (void); static void svr4_relocate_main_executable (void); -static void svr4_free_library_list (so_list *solist); static void probes_table_remove_objfile_probes (struct objfile *objfile); static void svr4_iterate_over_objfiles_in_search_order (gdbarch *gdbarch, iterate_over_objfiles_in_search_order_cb_ftype cb, @@ -172,26 +171,35 @@ svr4_same_1 (const char *gdb_so_name, const char *inferior_so_name) return 0; } -static int -svr4_same (const so_list &gdb, const so_list &inferior) +static bool +svr4_same (const char *gdb_name, const char *inferior_name, + const lm_info_svr4 &gdb_lm_info, + const lm_info_svr4 &inferior_lm_info) { - if (!svr4_same_1 (gdb.so_original_name, inferior.so_original_name)) + if (!svr4_same_1 (gdb_name, inferior_name)) return false; /* There may be different instances of the same library, in different namespaces. Each instance, however, must have been loaded at a different address so its relocation offset would be different. */ + return gdb_lm_info.l_addr_inferior == inferior_lm_info.l_addr_inferior; +} + +static int +svr4_same (const so_list &gdb, const so_list &inferior) +{ auto *lmg = gdb::checked_static_cast (gdb.lm_info); auto *lmi = gdb::checked_static_cast (inferior.lm_info); - return (lmg->l_addr_inferior == lmi->l_addr_inferior); + return svr4_same (gdb.so_original_name, inferior.so_original_name, + *lmg, *lmi); } -static std::unique_ptr +static lm_info_svr4_up lm_info_read (CORE_ADDR lm_addr) { struct link_map_offsets *lmo = svr4_fetch_link_map_offsets (); - std::unique_ptr lm_info; + lm_info_svr4_up lm_info; gdb::byte_vector lm (lmo->link_map_size); @@ -203,7 +211,7 @@ lm_info_read (CORE_ADDR lm_addr) type *ptr_type = builtin_type (current_inferior ()->arch ())->builtin_data_ptr; - lm_info.reset (new lm_info_svr4); + lm_info = gdb::make_unique (); lm_info->lm_addr = lm_addr; lm_info->l_addr_inferior = extract_typed_address (&lm[lmo->l_addr_offset], @@ -333,13 +341,20 @@ lm_addr_check (const so_list &so, bfd *abfd) return li->l_addr; } +struct svr4_so +{ + svr4_so (const char *name, lm_info_svr4_up lm_info) + : name (name), lm_info (std::move (lm_info)) + {} + + std::string name; + lm_info_svr4_up lm_info; +}; + /* Per pspace SVR4 specific data. */ struct svr4_info { - svr4_info () = default; - ~svr4_info (); - /* Base of dynamic linker structures in default namespace. */ CORE_ADDR debug_base = 0; @@ -385,7 +400,7 @@ struct svr4_info The special entry zero is reserved for a linear list to support gdbstubs that do not support namespaces. */ - std::map solib_lists; + std::map> solib_lists; }; /* Per-program-space data key. */ @@ -407,23 +422,6 @@ free_probes_table (struct svr4_info *info) info->probes_table.reset (nullptr); } -/* Free the solib lists for all namespaces. */ - -static void -free_solib_lists (svr4_info *info) -{ - for (const std::pair tuple - : info->solib_lists) - svr4_free_library_list (tuple.second); - - info->solib_lists.clear (); -} - -svr4_info::~svr4_info () -{ - free_solib_lists (this); -} - /* Get the svr4 data for program space PSPACE. If none is found yet, add it now. This function always returns a valid object. */ @@ -955,7 +953,7 @@ struct svr4_library_list { /* The tail pointer of the current namespace. This is internal to XML parsing. */ - so_list **tailp; + std::vector *cur_list; /* Inferior address of struct link_map used for the main executable. It is NULL if not known. */ @@ -965,7 +963,7 @@ struct svr4_library_list not include any default sos. See comment on struct svr4_info.solib_lists. */ - std::map solib_lists; + std::map> solib_lists; }; /* This module's 'free_objfile' observer. */ @@ -987,41 +985,27 @@ svr4_clear_so (const so_list &so) li->l_addr_p = 0; } -/* Free so_list built so far. */ - -static void -svr4_free_library_list (so_list *list) -{ - while (list != NULL) - { - struct so_list *next = list->next; +/* Create the so_list objects equivalent to the svr4_sos in SOS. */ - free_so (*list); - list = next; - } -} - -/* Copy library list. */ - -static struct so_list * -svr4_copy_library_list (struct so_list *src) +static so_list * +so_list_from_svr4_sos (const std::vector &sos) { struct so_list *dst = NULL; struct so_list **link = &dst; - while (src != NULL) + for (const svr4_so &so : sos) { so_list *newobj = new so_list; - memcpy (newobj, src, sizeof (struct so_list)); - auto *src_li = gdb::checked_static_cast (src->lm_info); - newobj->lm_info = new lm_info_svr4 (*src_li); + strncpy (newobj->so_name, so.name.c_str (), + sizeof (newobj->so_name) - 1); + newobj->so_name[sizeof (newobj->so_name) - 1] = 0; + strcpy (newobj->so_original_name, newobj->so_name); + newobj->lm_info = new lm_info_svr4 (*so.lm_info); newobj->next = NULL; *link = newobj; link = &newobj->next; - - src = src->next; } return dst; @@ -1050,48 +1034,25 @@ library_list_start_library (struct gdb_xml_parser *parser, ULONGEST *l_ldp = (ULONGEST *) xml_find_attribute (attributes, "l_ld")->value.get (); - so_list *new_elem = new so_list; - lm_info_svr4 *li = new lm_info_svr4; - new_elem->lm_info = li; + lm_info_svr4_up li = gdb::make_unique (); li->lm_addr = *lmp; li->l_addr_inferior = *l_addrp; li->l_ld = *l_ldp; - strncpy (new_elem->so_name, name, sizeof (new_elem->so_name) - 1); - new_elem->so_name[sizeof (new_elem->so_name) - 1] = 0; - strcpy (new_elem->so_original_name, new_elem->so_name); + std::vector *solist; /* Older versions did not supply lmid. Put the element into the flat list of the special namespace zero in that case. */ gdb_xml_value *at_lmid = xml_find_attribute (attributes, "lmid"); if (at_lmid == nullptr) - { - *list->tailp = new_elem; - list->tailp = &new_elem->next; - } + solist = list->cur_list; else { ULONGEST lmid = *(ULONGEST *) at_lmid->value.get (); - - /* Ensure that the element is actually initialized. */ - if (list->solib_lists.find (lmid) == list->solib_lists.end ()) - list->solib_lists[lmid] = nullptr; - - so_list **psolist = &list->solib_lists[lmid]; - so_list **pnext = psolist; - - /* Walk to the end of the list if we have one. */ - so_list *solist = *psolist; - if (solist != nullptr) - { - for (; solist->next != nullptr; solist = solist->next) - /* Nothing. */; - - pnext = &solist->next; - } - - *pnext = new_elem; + solist = &list->solib_lists[lmid]; } + + solist->emplace_back (name, std::move (li)); } /* Handle the start of a element. */ @@ -1117,9 +1078,7 @@ svr4_library_list_start_list (struct gdb_xml_parser *parser, /* Older gdbserver do not support namespaces. We use the special namespace zero for a linear list of libraries. */ - so_list **solist = &list->solib_lists[0]; - *solist = nullptr; - list->tailp = solist; + list->cur_list = &list->solib_lists[0]; } /* The allowed elements and attributes for an XML library list. @@ -1169,13 +1128,9 @@ static int svr4_parse_libraries (const char *document, struct svr4_library_list *list) { auto cleanup = make_scope_exit ([list] () - { - for (const std::pair tuple - : list->solib_lists) - svr4_free_library_list (tuple.second); - }); + { list->solib_lists.clear (); }); - list->tailp = nullptr; + list->cur_list = nullptr; list->main_lm = 0; list->solib_lists.clear (); if (gdb_xml_parse_quick (_("target library list"), "library-list-svr4.dtd", @@ -1253,25 +1208,21 @@ svr4_default_sos (svr4_info *info) /* Read the whole inferior libraries chain starting at address LM. Expect the first entry in the chain's previous entry to be PREV_LM. - Add the entries to the tail referenced by LINK_PTR_PTR. Ignore the - first entry if IGNORE_FIRST and set global MAIN_LM_ADDR according - to it. Returns nonzero upon success. If zero is returned the - entries stored to LINK_PTR_PTR are still valid although they may + Add the entries to SOS. Ignore the first entry if IGNORE_FIRST and set + global MAIN_LM_ADDR according to it. Returns nonzero upon success. If zero + is returned the entries stored to LINK_PTR_PTR are still valid although they may represent only part of the inferior library list. */ static int svr4_read_so_list (svr4_info *info, CORE_ADDR lm, CORE_ADDR prev_lm, - struct so_list ***link_ptr_ptr, int ignore_first) + std::vector &sos, int ignore_first) { CORE_ADDR first_l_name = 0; CORE_ADDR next_lm; for (; lm != 0; prev_lm = lm, lm = next_lm) { - so_list_up newobj (new so_list); - - lm_info_svr4 *li = lm_info_read (lm).release (); - newobj->lm_info = li; + lm_info_svr4_up li = lm_info_read (lm); if (li == NULL) return 0; @@ -1298,9 +1249,9 @@ svr4_read_so_list (svr4_info *info, CORE_ADDR lm, CORE_ADDR prev_lm, } /* Extract this shared object's name. */ - gdb::unique_xmalloc_ptr buffer + gdb::unique_xmalloc_ptr name = target_read_string (li->l_name, SO_NAME_MAX_PATH_SIZE - 1); - if (buffer == nullptr) + if (name == nullptr) { /* If this entry's l_name address matches that of the inferior executable, then this is not a normal shared @@ -1311,19 +1262,12 @@ svr4_read_so_list (svr4_info *info, CORE_ADDR lm, CORE_ADDR prev_lm, continue; } - strncpy (newobj->so_name, buffer.get (), SO_NAME_MAX_PATH_SIZE - 1); - newobj->so_name[SO_NAME_MAX_PATH_SIZE - 1] = '\0'; - strcpy (newobj->so_original_name, newobj->so_name); - /* If this entry has no name, or its name matches the name for the main executable, don't include it in the list. */ - if (! newobj->so_name[0] || match_main (newobj->so_name)) + if (*name == '\0' || match_main (name.get ())) continue; - newobj->next = 0; - /* Don't free it now. */ - **link_ptr_ptr = newobj.release (); - *link_ptr_ptr = &(**link_ptr_ptr)->next; + sos.emplace_back (name.get (), std::move (li)); } return 1; @@ -1342,7 +1286,7 @@ svr4_current_sos_direct (struct svr4_info *info) struct svr4_library_list library_list; /* Remove any old libraries. We're going to read them back in again. */ - free_solib_lists (info); + info->solib_lists.clear (); /* Fall back to manual examination of the target if the packet is not supported or gdbserver failed to find DT_DEBUG. gdb.server/solib-list.exp @@ -1362,10 +1306,10 @@ svr4_current_sos_direct (struct svr4_info *info) /* Remove an empty special zero namespace so we know that when there is one, it is actually used, and we have a flat list without namespace information. */ - if ((library_list.solib_lists.find (0) - != library_list.solib_lists.end ()) - && (library_list.solib_lists[0] == nullptr)) - library_list.solib_lists.erase (0); + auto it_0 = library_list.solib_lists.find (0); + if (it_0 != library_list.solib_lists.end () + && it_0->second.empty ()) + library_list.solib_lists.erase (it_0); /* Replace the (empty) solib_lists in INFO with the one generated from the target. We don't want to copy it on assignment and then @@ -1391,9 +1335,7 @@ svr4_current_sos_direct (struct svr4_info *info) ignore_first = true; auto cleanup = make_scope_exit ([info] () - { - free_solib_lists (info); - }); + { info->solib_lists.clear (); }); /* Collect the sos in each namespace. */ CORE_ADDR debug_base = info->debug_base; @@ -1403,12 +1345,8 @@ svr4_current_sos_direct (struct svr4_info *info) /* Walk the inferior's link map list, and build our so_list list. */ lm = solib_svr4_r_map (debug_base); if (lm != 0) - { - so_list **sos = &info->solib_lists[debug_base]; - *sos = nullptr; - - svr4_read_so_list (info, lm, 0, &sos, ignore_first); - } + svr4_read_so_list (info, lm, 0, info->solib_lists[debug_base], + ignore_first); } /* On Solaris, the dynamic linker is not in the normal list of @@ -1425,11 +1363,8 @@ svr4_current_sos_direct (struct svr4_info *info) { /* Add the dynamic linker's namespace unless we already did. */ if (info->solib_lists.find (debug_base) == info->solib_lists.end ()) - { - so_list **sos = &info->solib_lists[debug_base]; - *sos = nullptr; - svr4_read_so_list (info, debug_base, 0, &sos, 0); - } + svr4_read_so_list (info, debug_base, 0, info->solib_lists[debug_base], + 0); } cleanup.release (); @@ -1440,19 +1375,18 @@ svr4_current_sos_direct (struct svr4_info *info) static so_list * svr4_collect_probes_sos (svr4_info *info) { - so_list *sos = nullptr; - so_list **pnext = &sos; + so_list *res = nullptr; + so_list **pnext = &res; - for (const std::pair tuple - : info->solib_lists) + for (const auto &tuple : info->solib_lists) { - so_list *solist = tuple.second; + const std::vector &sos = tuple.second; /* Allow the linker to report empty namespaces. */ - if (solist == nullptr) + if (sos.empty ()) continue; - *pnext = svr4_copy_library_list (solist); + *pnext = so_list_from_svr4_sos (sos); /* Update PNEXT to point to the next member of the last element. */ gdb_assert (*pnext != nullptr); @@ -1466,7 +1400,7 @@ svr4_collect_probes_sos (svr4_info *info) } } - return sos; + return res; } /* Implement the main part of the "current_sos" target_so_ops @@ -1853,16 +1787,10 @@ solist_update_incremental (svr4_info *info, CORE_ADDR debug_base, if (info->solib_lists.find (0) != info->solib_lists.end ()) return 0; - /* Ensure that the element is actually initialized. */ - if (info->solib_lists.find (debug_base) == info->solib_lists.end ()) - info->solib_lists[debug_base] = nullptr; - - so_list **psolist = &info->solib_lists[debug_base]; - so_list **pnext = nullptr; - so_list *solist = *psolist; + std::vector &solist = info->solib_lists[debug_base]; CORE_ADDR prev_lm; - if (solist == nullptr) + if (solist.empty ()) { /* svr4_current_sos_direct contains logic to handle a number of special cases relating to the first elements of the list in @@ -1872,18 +1800,9 @@ solist_update_incremental (svr4_info *info, CORE_ADDR debug_base, return 0; prev_lm = 0; - pnext = psolist; } else - { - /* Walk to the end of the list. */ - for (; solist->next != nullptr; solist = solist->next) - /* Nothing. */; - - auto *li = gdb::checked_static_cast (solist->lm_info); - prev_lm = li->lm_addr; - pnext = &solist->next; - } + prev_lm = solist.back ().lm_info->lm_addr; /* Read the new objects. */ if (info->using_xfer) @@ -1905,24 +1824,23 @@ solist_update_incremental (svr4_info *info, CORE_ADDR debug_base, We expect gdbserver to provide updates for the namespace that contains LM, which would be this namespace... */ - so_list *sos = nullptr; - if (library_list.solib_lists.find (debug_base) - != library_list.solib_lists.end ()) - std::swap (sos, library_list.solib_lists[debug_base]); - if (sos == nullptr) + std::vector sos; + auto it_debug_base = library_list.solib_lists.find (debug_base); + if (it_debug_base != library_list.solib_lists.end ()) + std::swap (sos, it_debug_base->second); + else { /* ...or for the special zero namespace for earlier versions... */ - if (library_list.solib_lists.find (0) - != library_list.solib_lists.end ()) - std::swap (sos, library_list.solib_lists[0]); + auto it_0 = library_list.solib_lists.find (0); + if (it_0 != library_list.solib_lists.end ()) + std::swap (sos, it_0->second); } /* ...but nothing else. */ - for (const std::pair tuple - : library_list.solib_lists) - gdb_assert (tuple.second == nullptr); + for (const auto &tuple : library_list.solib_lists) + gdb_assert (tuple.second.empty ()); - *pnext = sos; + std::move (sos.begin (), sos.end (), std::back_inserter (solist)); } else { @@ -1930,7 +1848,7 @@ solist_update_incremental (svr4_info *info, CORE_ADDR debug_base, 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 (info, lm, prev_lm, &pnext, 0)) + if (!svr4_read_so_list (info, lm, prev_lm, solist, 0)) return 0; } @@ -1948,7 +1866,7 @@ disable_probes_interface (svr4_info *info) "Reverting to original interface.")); free_probes_table (info); - free_solib_lists (info); + info->solib_lists.clear (); } /* Update the solib list as appropriate when using the @@ -3145,7 +3063,7 @@ svr4_solib_create_inferior_hook (int from_tty) /* Clear the probes-based interface's state. */ free_probes_table (info); - free_solib_lists (info); + info->solib_lists.clear (); /* Relocate the main executable if necessary. */ svr4_relocate_main_executable (); @@ -3373,14 +3291,17 @@ find_debug_base_for_solib (so_list *solib) svr4_info *info = get_svr4_info (current_program_space); gdb_assert (info != nullptr); - for (const std::pair tuple - : info->solib_lists) + + const lm_info_svr4 *lm_info = (const lm_info_svr4 *) solib->lm_info; + + for (const auto &tuple : info->solib_lists) { CORE_ADDR debug_base = tuple.first; - so_list *solist = tuple.second; + const std::vector &sos = tuple.second; - for (; solist != nullptr; solist = solist->next) - if (svr4_same (*solib, *solist)) + for (const svr4_so &so : sos) + if (svr4_same (solib->so_original_name, so.name.c_str (), + *lm_info, *so.lm_info)) return debug_base; } diff --git a/gdb/solib-svr4.h b/gdb/solib-svr4.h index 1aff3b59da8c..7cb6cabb109f 100644 --- a/gdb/solib-svr4.h +++ b/gdb/solib-svr4.h @@ -48,6 +48,8 @@ struct lm_info_svr4 final : public lm_info CORE_ADDR l_ld = 0, l_next = 0, l_prev = 0, l_name = 0; }; +using lm_info_svr4_up = std::unique_ptr; + /* Critical offsets and sizes which describe struct r_debug and struct link_map on SVR4-like targets. All offsets and sizes are in bytes unless otherwise specified. */ -- 2.42.0