[12/24] gdb: make solib-svr4 not use so_list internally
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
Commit Message
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<svr4_so>. 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(-)
Comments
Hi Simon,
When applying this patch, I have:
Applying: gdb: make solib-svr4 not use so_list internally
.git/rebase-apply/patch:182: trailing whitespace.
sizeof (newobj->so_name) - 1);
.git/rebase-apply/patch:183: trailing whitespace.
newobj->so_name[sizeof (newobj->so_name) - 1] = 0;
.git/rebase-apply/patch:184: trailing whitespace.
strcpy (newobj->so_original_name, newobj->so_name);
warning: 3 lines add whitespace errors.
I only skimmed through the patch, I have not payed detailed attention to
details so far.
Best,
Lancelot.
On Tue, Oct 10, 2023 at 04:40:07PM -0400, Simon Marchi wrote:
> 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<svr4_so>. 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<const lm_info_svr4 *> (gdb.lm_info);
> auto *lmi = gdb::checked_static_cast<const lm_info_svr4 *> (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<lm_info_svr4>
> +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_svr4> 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_svr4> ();
> 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<CORE_ADDR, so_list *> solib_lists;
> + std::map<CORE_ADDR, std::vector<svr4_so>> 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<CORE_ADDR, so_list *> 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<svr4_so> *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<CORE_ADDR, so_list *> solib_lists;
> + std::map<CORE_ADDR, std::vector<svr4_so>> 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<svr4_so> &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<lm_info_svr4 *> (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<lm_info_svr4> ();
> 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<svr4_so> *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 <library-list-svr4> 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<CORE_ADDR, so_list *> 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<svr4_so> &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<char> buffer
> + gdb::unique_xmalloc_ptr<char> 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<CORE_ADDR, so_list *> tuple
> - : info->solib_lists)
> + for (const auto &tuple : info->solib_lists)
> {
> - so_list *solist = tuple.second;
> + const std::vector<svr4_so> &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<svr4_so> &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<lm_info_svr4 *> (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<svr4_so> 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<CORE_ADDR, so_list *> 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<CORE_ADDR, so_list *> 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<svr4_so> &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<lm_info_svr4>;
> +
> /* 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
>
On 10/13/23 13:52, Lancelot SIX wrote:
> Hi Simon,
>
> When applying this patch, I have:
>
> Applying: gdb: make solib-svr4 not use so_list internally
> .git/rebase-apply/patch:182: trailing whitespace.
> sizeof (newobj->so_name) - 1);
> .git/rebase-apply/patch:183: trailing whitespace.
> newobj->so_name[sizeof (newobj->so_name) - 1] = 0;
> .git/rebase-apply/patch:184: trailing whitespace.
> strcpy (newobj->so_original_name, newobj->so_name);
> warning: 3 lines add whitespace errors.
>
> I only skimmed through the patch, I have not payed detailed attention to
> details so far.
Thanks, fixed that locally.
Simon
Hi Simon,
> @@ -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<lm_info_svr4> ();
> li->lm_addr = *lmp;
> li->l_addr_inferior = *l_addrp;
> li->l_ld = *l_ldp;
Just a thought for future changes to continue the c++-ification, adding
a proper ctor (or ctors) to lm_info_svr4 would probably make things
cleaner.
Otherwise, things looks reasonable to me.
Best,
Lancelot.
>
> - 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<svr4_so> *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 <library-list-svr4> 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<CORE_ADDR, so_list *> 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<svr4_so> &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<char> buffer
> + gdb::unique_xmalloc_ptr<char> 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<CORE_ADDR, so_list *> tuple
> - : info->solib_lists)
> + for (const auto &tuple : info->solib_lists)
> {
> - so_list *solist = tuple.second;
> + const std::vector<svr4_so> &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<svr4_so> &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<lm_info_svr4 *> (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<svr4_so> 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<CORE_ADDR, so_list *> 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<CORE_ADDR, so_list *> 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<svr4_so> &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<lm_info_svr4>;
> +
> /* 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
>
On 10/19/23 07:08, Lancelot SIX wrote:
> Hi Simon,
>
>> @@ -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<lm_info_svr4> ();
>> li->lm_addr = *lmp;
>> li->l_addr_inferior = *l_addrp;
>> li->l_ld = *l_ldp;
>
> Just a thought for future changes to continue the c++-ification, adding
> a proper ctor (or ctors) to lm_info_svr4 would probably make things
> cleaner.
>
> Otherwise, things looks reasonable to me.
Ok, I'll look into doing a patch on top instead of inserting one in the
middle, since I don't want to deal with conflicts again.
Simon
@@ -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<const lm_info_svr4 *> (gdb.lm_info);
auto *lmi = gdb::checked_static_cast<const lm_info_svr4 *> (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<lm_info_svr4>
+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_svr4> 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_svr4> ();
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<CORE_ADDR, so_list *> solib_lists;
+ std::map<CORE_ADDR, std::vector<svr4_so>> 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<CORE_ADDR, so_list *> 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<svr4_so> *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<CORE_ADDR, so_list *> solib_lists;
+ std::map<CORE_ADDR, std::vector<svr4_so>> 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<svr4_so> &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<lm_info_svr4 *> (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<lm_info_svr4> ();
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<svr4_so> *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 <library-list-svr4> 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<CORE_ADDR, so_list *> 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<svr4_so> &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<char> buffer
+ gdb::unique_xmalloc_ptr<char> 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<CORE_ADDR, so_list *> tuple
- : info->solib_lists)
+ for (const auto &tuple : info->solib_lists)
{
- so_list *solist = tuple.second;
+ const std::vector<svr4_so> &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<svr4_so> &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<lm_info_svr4 *> (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<svr4_so> 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<CORE_ADDR, so_list *> 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<CORE_ADDR, so_list *> 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<svr4_so> &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;
}
@@ -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<lm_info_svr4>;
+
/* 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. */