[PATCH] gdb/solib-frv: remove manual memory management

[PATCH] gdb/solib-frv: remove manual memory management

[simon.marchi]

From: Simon Marchi <simon.marchi@efficios.com>

 - Make fetch_loadmap return a unique pointer, adjust callers
 - Make fetch_loadmap use a unique pointer for ext_ldmbuf
 - Replace some fields of lm_info_frv to be unique pointers
 - Make main_executable_lm_info a unique pointer

I can only build-test this.

Change-Id: I5782ea31b5e30fef2c7468f0bc24d10ffae92669
---
 gdb/solib-frv.c | 105 +++++++++++++++++++++++-------------------------
 1 file changed, 50 insertions(+), 55 deletions(-)

diff --git a/gdb/solib-frv.c b/gdb/solib-frv.c
index 95f97dc11333..dedf26ae64d8 100644
--- a/gdb/solib-frv.c
+++ b/gdb/solib-frv.c
@@ -80,6 +80,9 @@ struct ext_elf32_fdpic_loadmap {
   struct ext_elf32_fdpic_loadseg segs[1 /* nsegs, actually */];
 };
 
+using ext_elf32_fdpic_loadmap_up
+  = gdb::unique_xmalloc_ptr<ext_elf32_fdpic_loadmap>;
+
 /* Internal versions; the types are GDB types and the data in each
    of the fields is (or will be) decoded from the external struct
    for ease of consumption.  */
@@ -102,19 +105,20 @@ struct int_elf32_fdpic_loadmap {
   struct int_elf32_fdpic_loadseg segs[1 /* nsegs, actually */];
 };
 
+using int_elf32_fdpic_loadmap_up
+  = gdb::unique_xmalloc_ptr<int_elf32_fdpic_loadmap>;
+
 /* Given address LDMADDR, fetch and decode the loadmap at that address.
    Return NULL if there is a problem reading the target memory or if
    there doesn't appear to be a loadmap at the given address.  The
    allocated space (representing the loadmap) returned by this
    function may be freed via a single call to xfree().  */
 
-static struct int_elf32_fdpic_loadmap *
+static int_elf32_fdpic_loadmap_up
 fetch_loadmap (CORE_ADDR ldmaddr)
 {
   bfd_endian byte_order = gdbarch_byte_order (current_inferior ()->arch ());
   struct ext_elf32_fdpic_loadmap ext_ldmbuf_partial;
-  struct ext_elf32_fdpic_loadmap *ext_ldmbuf;
-  struct int_elf32_fdpic_loadmap *int_ldmbuf;
   int ext_ldmbuf_size, int_ldmbuf_size;
   int version, seg, nsegs;
 
@@ -147,18 +151,19 @@ fetch_loadmap (CORE_ADDR ldmaddr)
   /* Allocate space for the complete (external) loadmap.  */
   ext_ldmbuf_size = sizeof (struct ext_elf32_fdpic_loadmap)
 	       + (nsegs - 1) * sizeof (struct ext_elf32_fdpic_loadseg);
-  ext_ldmbuf = (struct ext_elf32_fdpic_loadmap *) xmalloc (ext_ldmbuf_size);
+  ext_elf32_fdpic_loadmap_up ext_ldmbuf
+    (XNEWVAR (ext_elf32_fdpic_loadmap, ext_ldmbuf_size));
 
   /* Copy over the portion of the loadmap that's already been read.  */
-  memcpy (ext_ldmbuf, &ext_ldmbuf_partial, sizeof ext_ldmbuf_partial);
+  memcpy (ext_ldmbuf.get (), &ext_ldmbuf_partial, sizeof ext_ldmbuf_partial);
 
   /* Read the rest of the loadmap from the target.  */
   if (target_read_memory (ldmaddr + sizeof ext_ldmbuf_partial,
-			  (gdb_byte *) ext_ldmbuf + sizeof ext_ldmbuf_partial,
+			  ((gdb_byte *) ext_ldmbuf.get ()
+			   + sizeof ext_ldmbuf_partial),
 			  ext_ldmbuf_size - sizeof ext_ldmbuf_partial))
     {
       /* Couldn't read rest of the loadmap.  */
-      xfree (ext_ldmbuf);
       return NULL;
     }
 
@@ -166,7 +171,8 @@ fetch_loadmap (CORE_ADDR ldmaddr)
      external loadsegs.  I.e, allocate the internal loadsegs.  */
   int_ldmbuf_size = sizeof (struct int_elf32_fdpic_loadmap)
 	       + (nsegs - 1) * sizeof (struct int_elf32_fdpic_loadseg);
-  int_ldmbuf = (struct int_elf32_fdpic_loadmap *) xmalloc (int_ldmbuf_size);
+  int_elf32_fdpic_loadmap_up int_ldmbuf
+    (XNEWVAR (int_elf32_fdpic_loadmap, int_ldmbuf_size));
 
   /* Place extracted information in internal structs.  */
   int_ldmbuf->version = version;
@@ -187,7 +193,6 @@ fetch_loadmap (CORE_ADDR ldmaddr)
 				    byte_order);
     }
 
-  xfree (ext_ldmbuf);
   return int_ldmbuf;
 }
 
@@ -219,22 +224,15 @@ struct ext_link_map
 
 struct lm_info_frv final : public lm_info
 {
-  lm_info_frv (int_elf32_fdpic_loadmap *map, CORE_ADDR got_value,
+  lm_info_frv (int_elf32_fdpic_loadmap_up map, CORE_ADDR got_value,
 	       CORE_ADDR lm_addr)
-    : map (map), got_value (got_value), lm_addr (lm_addr)
+    : map (std::move (map)),
+      got_value (got_value),
+      lm_addr (lm_addr)
   {}
 
-  DISABLE_COPY_AND_ASSIGN (lm_info_frv);
-
-  ~lm_info_frv ()
-  {
-    xfree (this->map);
-    xfree (this->dyn_syms);
-    xfree (this->dyn_relocs);
-  }
-
   /* The loadmap, digested into an easier to use form.  */
-  int_elf32_fdpic_loadmap *map;
+  int_elf32_fdpic_loadmap_up map;
   /* The GOT address for this link map entry.  */
   CORE_ADDR got_value;
   /* The link map address, needed for frv_fetch_objfile_link_map().  */
@@ -250,16 +248,18 @@ struct lm_info_frv final : public lm_info
      supplied to the first call.  Thus the caching of the dynamic
      symbols (dyn_syms) is critical for correct operation.  The
      caching of the dynamic relocations could be dispensed with.  */
-  asymbol **dyn_syms = NULL;
-  arelent **dyn_relocs = NULL;
+  gdb::unique_xmalloc_ptr<asymbol *> dyn_syms;
+  gdb::unique_xmalloc_ptr<arelent *> dyn_relocs;
   int dyn_reloc_count = 0;	/* Number of dynamic relocs.  */
 };
 
+using lm_info_frv_up = std::unique_ptr<lm_info_frv>;
+
 /* The load map, got value, etc. are not available from the chain
    of loaded shared objects.  ``main_executable_lm_info'' provides
    a way to get at this information so that it doesn't need to be
    frequently recomputed.  Initialized by frv_relocate_main_executable().  */
-static lm_info_frv *main_executable_lm_info;
+static lm_info_frv_up main_executable_lm_info;
 
 static void frv_relocate_main_executable (void);
 static CORE_ADDR main_got (void);
@@ -378,14 +378,13 @@ frv_solib_ops::current_sos () const
 	 this in the list of shared objects.  */
       if (got_addr != mgot)
 	{
-	  struct int_elf32_fdpic_loadmap *loadmap;
 	  CORE_ADDR addr;
 
 	  /* Fetch the load map address.  */
 	  addr = extract_unsigned_integer (lm_buf.l_addr.map,
 					   sizeof lm_buf.l_addr.map,
 					   byte_order);
-	  loadmap = fetch_loadmap (addr);
+	  int_elf32_fdpic_loadmap_up loadmap = fetch_loadmap (addr);
 	  if (loadmap == NULL)
 	    {
 	      warning (_("frv_current_sos: Unable to fetch load map.  "
@@ -405,11 +404,10 @@ frv_solib_ops::current_sos () const
 	  if (name_buf == nullptr)
 	    warning (_("Can't read pathname for link map entry."));
 
-	  sos.emplace_back (std::make_unique<lm_info_frv> (loadmap, got_addr,
-							   lm_addr),
-			    name_buf != nullptr ? name_buf.get () : "",
+	  sos.emplace_back (std::make_unique<lm_info_frv> (std::move (loadmap),
+							   got_addr, lm_addr),
 			    name_buf != nullptr ? name_buf.get () : "",
-			    *this);
+			    name_buf != nullptr ? name_buf.get () : "", *this);
 	}
       else
 	{
@@ -521,7 +519,6 @@ enable_break2 (void)
       int status;
       CORE_ADDR addr, interp_loadmap_addr;
       gdb_byte addr_buf[FRV_PTR_SIZE];
-      struct int_elf32_fdpic_loadmap *ldm;
 
       /* Read the contents of the .interp section into a local buffer;
 	 the contents specify the dynamic linker this program uses.  */
@@ -566,7 +563,7 @@ enable_break2 (void)
       solib_debug_printf ("interp_loadmap_addr = %s",
 			  hex_string_custom (interp_loadmap_addr, 8));
 
-      ldm = fetch_loadmap (interp_loadmap_addr);
+      int_elf32_fdpic_loadmap_up ldm = fetch_loadmap (interp_loadmap_addr);
       if (ldm == NULL)
 	{
 	  warning (_("Unable to load dynamic linker loadmap at address %s."),
@@ -582,7 +579,7 @@ enable_break2 (void)
 	{
 	  interp_text_sect_low = bfd_section_vma (interp_sect);
 	  interp_text_sect_low
-	    += displacement_from_map (ldm, interp_text_sect_low);
+	    += displacement_from_map (ldm.get (), interp_text_sect_low);
 	  interp_text_sect_high
 	    = interp_text_sect_low + bfd_section_size (interp_sect);
 	}
@@ -591,7 +588,7 @@ enable_break2 (void)
 	{
 	  interp_plt_sect_low = bfd_section_vma (interp_sect);
 	  interp_plt_sect_low
-	    += displacement_from_map (ldm, interp_plt_sect_low);
+	    += displacement_from_map (ldm.get (), interp_plt_sect_low);
 	  interp_plt_sect_high =
 	    interp_plt_sect_low + bfd_section_size (interp_sect);
 	}
@@ -614,7 +611,7 @@ enable_break2 (void)
       solib_debug_printf ("_dl_debug_addr (prior to relocation) = %s",
 			  hex_string_custom (addr, 8));
 
-      addr += displacement_from_map (ldm, addr);
+      addr += displacement_from_map (ldm.get (), addr);
 
       solib_debug_printf ("_dl_debug_addr (after relocation) = %s",
 			  hex_string_custom (addr, 8));
@@ -663,9 +660,6 @@ enable_break2 (void)
 	}
       addr = extract_unsigned_integer (addr_buf, sizeof addr_buf, byte_order);
 
-      /* We're done with the loadmap.  */
-      xfree (ldm);
-
       /* Remove all the solib event breakpoints.  Their addresses
 	 may have changed since the last time we ran the program.  */
       remove_solib_event_breakpoints ();
@@ -728,7 +722,6 @@ frv_relocate_main_executable (void)
 {
   int status;
   CORE_ADDR exec_addr, interp_addr;
-  struct int_elf32_fdpic_loadmap *ldm;
   int changed;
 
   status = frv_fdpic_loadmap_addresses (current_inferior ()->arch (),
@@ -741,13 +734,14 @@ frv_relocate_main_executable (void)
     }
 
   /* Fetch the loadmap located at ``exec_addr''.  */
-  ldm = fetch_loadmap (exec_addr);
-  if (ldm == NULL)
+  int_elf32_fdpic_loadmap_up ldm_up = fetch_loadmap (exec_addr);
+  if (ldm_up == nullptr)
     error (_("Unable to load the executable's loadmap."));
 
-  delete main_executable_lm_info;
-  main_executable_lm_info = new lm_info_frv (ldm, 0, 0);
+  main_executable_lm_info
+    = std::make_unique<lm_info_frv> (std::move (ldm_up), 0, 0);
 
+  int_elf32_fdpic_loadmap *ldm = main_executable_lm_info->map.get ();
   objfile *objf = current_program_space->symfile_object_file;
   std::vector<CORE_ADDR> new_offsets (objf->section_offsets.size ());
   changed = 0;
@@ -816,9 +810,7 @@ frv_solib_ops::clear_solib (program_space *pspace) const
   lm_base_cache = 0;
   enable_break2_done = 0;
   main_lm_addr = 0;
-
-  delete main_executable_lm_info;
-  main_executable_lm_info = NULL;
+  main_executable_lm_info.reset ();
 }
 
 void
@@ -827,7 +819,7 @@ frv_solib_ops::relocate_section_addresses (solib &so,
 {
   int seg;
   auto *li = gdb::checked_static_cast<lm_info_frv *> (so.lm_info.get ());
-  int_elf32_fdpic_loadmap *map = li->map;
+  int_elf32_fdpic_loadmap *map = li->map.get ();
 
   for (seg = 0; seg < map->nsegs; seg++)
     {
@@ -868,7 +860,7 @@ frv_fdpic_find_global_pointer (CORE_ADDR addr)
     {
       int seg;
       auto *li = gdb::checked_static_cast<lm_info_frv *> (so.lm_info.get ());
-      int_elf32_fdpic_loadmap *map = li->map;
+      int_elf32_fdpic_loadmap *map = li->map.get ();
 
       for (seg = 0; seg < map->nsegs; seg++)
 	{
@@ -915,7 +907,7 @@ frv_fdpic_find_canonical_descriptor (CORE_ADDR entry_point)
   objfile *objf = current_program_space->symfile_object_file;
   addr = find_canonical_descriptor_in_load_object
 	   (entry_point, got_value, name, objf->obfd.get (),
-	    main_executable_lm_info);
+	    main_executable_lm_info.get ());
 
   /* If descriptor not found via main executable, check each load object
      in list of shared objects.  */
@@ -973,10 +965,11 @@ find_canonical_descriptor_in_load_object
 	return 0;
 
       /* Allocate space for the dynamic symbol table.  */
-      lm->dyn_syms = (asymbol **) xmalloc (storage_needed);
+      lm->dyn_syms.reset (XNEWVAR (asymbol *, storage_needed));
 
       /* Fetch the dynamic symbol table.  */
-      number_of_symbols = bfd_canonicalize_dynamic_symtab (abfd, lm->dyn_syms);
+      number_of_symbols
+	= bfd_canonicalize_dynamic_symtab (abfd, lm->dyn_syms.get ());
 
       if (number_of_symbols == 0)
 	return 0;
@@ -995,17 +988,18 @@ find_canonical_descriptor_in_load_object
 	return 0;
 
       /* Allocate space for the relocs.  */
-      lm->dyn_relocs = (arelent **) xmalloc (storage_needed);
+      lm->dyn_relocs.reset (XNEWVAR (arelent *, storage_needed));
 
       /* Fetch the dynamic relocs.  */
       lm->dyn_reloc_count
-	= bfd_canonicalize_dynamic_reloc (abfd, lm->dyn_relocs, lm->dyn_syms);
+	= bfd_canonicalize_dynamic_reloc (abfd, lm->dyn_relocs.get (),
+					  lm->dyn_syms.get ());
     }
 
   /* Search the dynamic relocs.  */
   for (i = 0; i < lm->dyn_reloc_count; i++)
     {
-      rel = lm->dyn_relocs[i];
+      rel = lm->dyn_relocs.get ()[i];
 
       /* Relocs of interest are those which meet the following
 	 criteria:
@@ -1029,7 +1023,8 @@ find_canonical_descriptor_in_load_object
 	  gdb_byte buf [FRV_PTR_SIZE];
 
 	  /* Compute address of address of candidate descriptor.  */
-	  addr = rel->address + displacement_from_map (lm->map, rel->address);
+	  addr = (rel->address
+		  + displacement_from_map (lm->map.get (), rel->address));
 
 	  /* Fetch address of candidate descriptor.  */
 	  if (target_read_memory (addr, buf, sizeof buf) != 0)

base-commit: 625b835f63153677bf614de266c7bc8963163ffc
-- 
2.53.0

Re: [PATCH] gdb/solib-frv: remove manual memory management

[Kevin Buettner]

On Tue, 17 Feb 2026 16:16:44 -0500
simon.marchi@polymtl.ca wrote:

> From: Simon Marchi <simon.marchi@efficios.com>
> 
>  - Make fetch_loadmap return a unique pointer, adjust callers
>  - Make fetch_loadmap use a unique pointer for ext_ldmbuf
>  - Replace some fields of lm_info_frv to be unique pointers
>  - Make main_executable_lm_info a unique pointer
> 
> I can only build-test this.

I haven't had access to an FR-V machine in a long, long time, so I can't
properly test it either.

I found a comment that should be updated...

> @@ -102,19 +105,20 @@ struct int_elf32_fdpic_loadmap {
>    struct int_elf32_fdpic_loadseg segs[1 /* nsegs, actually */];
>  };
>  
> +using int_elf32_fdpic_loadmap_up
> +  = gdb::unique_xmalloc_ptr<int_elf32_fdpic_loadmap>;
> +
>  /* Given address LDMADDR, fetch and decode the loadmap at that address.
>     Return NULL if there is a problem reading the target memory or if
>     there doesn't appear to be a loadmap at the given address.  The
>     allocated space (representing the loadmap) returned by this
>     function may be freed via a single call to xfree().  */

The remark about xfree should be removed and NULL should be replaced
with nullptr.

> -static struct int_elf32_fdpic_loadmap *
> +static int_elf32_fdpic_loadmap_up
>  fetch_loadmap (CORE_ADDR ldmaddr)

[Above lines included for context.]

Okay with that comment fixed...

Approved-by: Kevin Buettner <kevinb@redhat.com>

Re: [PATCH] gdb/solib-frv: remove manual memory management

[Tom Tromey]

>>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:

>> I can only build-test this.

Kevin> I haven't had access to an FR-V machine in a long, long time, so I can't
Kevin> properly test it either.

Do you know whether they still exist and/or are relevant?

I wonder if this is more obsolete code that we could remove.

Tom

Re: [PATCH] gdb/solib-frv: remove manual memory management

[Kevin Buettner]

On Wed, 18 Feb 2026 11:34:26 -0700
Tom Tromey <tom@tromey.com> wrote:

> >>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:  
> 
> >> I can only build-test this.  
> 
> Kevin> I haven't had access to an FR-V machine in a long, long time, so I
> Kevin> can't properly test it either.  
> 
> Do you know whether they still exist and/or are relevant?
> 
> I wonder if this is more obsolete code that we could remove.

I don't know whether FR-V chips are still a available and/or in use. 
But I do know that the FDPIC shared library ABI has been used for
other MMU-less linux architectures (including ARM) too.  I think it
might be a good idea to hang onto FDPIC support  for a while longer.

There's a somewhat recent write-up on these topics here:

https://maskray.me/blog/2024-02-20-mmu-less-systems-and-fdpic

Kevin

Re: [PATCH] gdb/solib-frv: remove manual memory management

[Simon Marchi]

On 2/17/26 6:32 PM, Kevin Buettner wrote:
> On Tue, 17 Feb 2026 16:16:44 -0500
> simon.marchi@polymtl.ca wrote:
> 
>> From: Simon Marchi <simon.marchi@efficios.com>
>>
>>  - Make fetch_loadmap return a unique pointer, adjust callers
>>  - Make fetch_loadmap use a unique pointer for ext_ldmbuf
>>  - Replace some fields of lm_info_frv to be unique pointers
>>  - Make main_executable_lm_info a unique pointer
>>
>> I can only build-test this.
> 
> I haven't had access to an FR-V machine in a long, long time, so I can't
> properly test it either.
> 
> I found a comment that should be updated...
> 
>> @@ -102,19 +105,20 @@ struct int_elf32_fdpic_loadmap {
>>    struct int_elf32_fdpic_loadseg segs[1 /* nsegs, actually */];
>>  };
>>  
>> +using int_elf32_fdpic_loadmap_up
>> +  = gdb::unique_xmalloc_ptr<int_elf32_fdpic_loadmap>;
>> +
>>  /* Given address LDMADDR, fetch and decode the loadmap at that address.
>>     Return NULL if there is a problem reading the target memory or if
>>     there doesn't appear to be a loadmap at the given address.  The
>>     allocated space (representing the loadmap) returned by this
>>     function may be freed via a single call to xfree().  */
> 
> The remark about xfree should be removed and NULL should be replaced
> with nullptr.

Fixed.

> 
>> -static struct int_elf32_fdpic_loadmap *
>> +static int_elf32_fdpic_loadmap_up
>>  fetch_loadmap (CORE_ADDR ldmaddr)
> 
> [Above lines included for context.]
> 
> Okay with that comment fixed...
> 
> Approved-by: Kevin Buettner <kevinb@redhat.com>

Thanks, pushed.

Simon