Re: [PATCH v4 1/2] [gdb] Fix segfault in for_each_block, part 1

[Tom de Vries <tdevries@suse.de>]

On 11/10/23 09:09, Tom de Vries wrote:
> When running test-case gdb.base/vfork-follow-parent.exp on powerpc64 (likewise
> on s390x), I run into:
> ...
> (gdb) PASS: gdb.base/vfork-follow-parent.exp: \
>    exec_file=vfork-follow-parent-exit: target-non-stop=on: non-stop=off: \
>    resolution_method=schedule-multiple: print unblock_parent = 1
> continue^M
> Continuing.^M
> Reading symbols from vfork-follow-parent-exit...^M
> ^M
> ^M
> Fatal signal: Segmentation fault^M
> ----- Backtrace -----^M
> 0x1027d3e7 gdb_internal_backtrace_1^M
>          src/gdb/bt-utils.c:122^M
> 0x1027d54f _Z22gdb_internal_backtracev^M
>          src/gdb/bt-utils.c:168^M
> 0x1057643f handle_fatal_signal^M
>          src/gdb/event-top.c:889^M
> 0x10576677 handle_sigsegv^M
>          src/gdb/event-top.c:962^M
> 0x3fffa7610477 ???^M
> 0x103f2144 for_each_block^M
>          src/gdb/dcache.c:199^M
> 0x103f235b _Z17dcache_invalidateP13dcache_struct^M
>          src/gdb/dcache.c:251^M
> 0x10bde8c7 _Z24target_dcache_invalidatev^M
>          src/gdb/target-dcache.c:50^M
> ...
> or similar.
> 
> The root cause for the segmentation fault is that linux_is_uclinux gives an
> incorrect result: it should always return false, given that we're running on a
> regular linux system, but instead it returns first true, then false.
> 
> In more detail, the segmentation fault happens as follows:
> - a program space with an address space is created
> - a second program space is about to be created. maybe_new_address_space
>    is called, and because linux_is_uclinux returns true, maybe_new_address_space
>    returns false, and no new address space is created
> - a second program space with the same address space is created
> - a program space is deleted. Because linux_is_uclinux now returns false,
>    gdbarch_has_shared_address_space (current_inferior ()->arch ()) returns
>    false, and the address space is deleted
> - when gdb uses the address space of the remaining program space, we run into
>    the segfault, because the address space is deleted.
> 
> Hardcoding linux_is_uclinux to false makes the test-case pass.
> 
> We leave addressing the root cause for the following commit in this series.
> 
> For now, prevent the segmentation fault by making the address space a refcounted
> object.
> 
> This was already suggested here [1]:
> ...
> A better solution might be to have the address spaces be reference counted
> ...
> 
> Tested on top of trunk on x86_64-linux and ppc64le-linux.
> Tested on top of gdb-14-branch on ppc64-linux.
> 
> Co-Authored-By: Simon Marchi <simon.marchi@polymtl.ca>
> 

With no further comments, I decided to commit.  The rebase was not 
entirely trivial, so I posted this as v5 ( 
https://sourceware.org/pipermail/gdb-patches/2023-November/204570.html ) 
before pushing.

Thanks,
- Tom

> PR gdb/30547
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30547
> 
> [1] https://sourceware.org/pipermail/gdb-patches/2023-October/202928.html
> ---
>   gdb/breakpoint.c               | 29 ++++++++-------
>   gdb/inferior.c                 |  8 ++---
>   gdb/inferior.h                 |  2 +-
>   gdb/infrun.c                   | 18 +++++-----
>   gdb/linux-nat.c                |  2 +-
>   gdb/process-stratum-target.c   |  2 +-
>   gdb/progspace.c                | 22 +++++-------
>   gdb/progspace.h                | 64 +++++++++++++++++++++-------------
>   gdb/record-btrace.c            |  2 +-
>   gdb/regcache.c                 |  2 +-
>   gdb/scoped-mock-context.h      |  2 +-
>   gdb/target-dcache.c            | 11 +++---
>   gdbsupport/refcounted-object.h | 17 +++++++++
>   13 files changed, 102 insertions(+), 79 deletions(-)
> 
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index fe09dbcbeee..0b086fbeab3 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -1723,7 +1723,7 @@ one_breakpoint_xfer_memory (gdb_byte *readbuf, gdb_byte *writebuf,
>     int bptoffset = 0;
>   
>     if (!breakpoint_address_match (target_info->placed_address_space, 0,
> -				 current_program_space->aspace, 0))
> +				 current_program_space->aspace.get (), 0))
>       {
>         /* The breakpoint is inserted in a different address space.  */
>         return;
> @@ -2409,7 +2409,7 @@ should_be_inserted (struct bp_location *bl)
>        a breakpoint.  */
>     if ((bl->loc_type == bp_loc_software_breakpoint
>          || bl->loc_type == bp_loc_hardware_breakpoint)
> -      && stepping_past_instruction_at (bl->pspace->aspace,
> +      && stepping_past_instruction_at (bl->pspace->aspace.get (),
>   				       bl->address)
>         /* The single-step breakpoint may be inserted at the location
>   	 we're trying to step if the instruction branches to itself.
> @@ -2847,7 +2847,7 @@ insert_bp_location (struct bp_location *bl,
>        read the breakpoint instead of returning the data saved in
>        the breakpoint location's shadow contents.  */
>     bl->target_info.reqstd_address = bl->address;
> -  bl->target_info.placed_address_space = bl->pspace->aspace;
> +  bl->target_info.placed_address_space = bl->pspace->aspace.get ();
>     bl->target_info.length = bl->length;
>   
>     /* When working with target-side conditions, we must pass all the conditions
> @@ -4429,7 +4429,7 @@ bp_location_inserted_here_p (const struct bp_location *bl,
>   			     const address_space *aspace, CORE_ADDR pc)
>   {
>     if (bl->inserted
> -      && breakpoint_address_match (bl->pspace->aspace, bl->address,
> +      && breakpoint_address_match (bl->pspace->aspace.get (), bl->address,
>   				   aspace, pc))
>       {
>         /* An unmapped overlay can't be a match.  */
> @@ -4508,7 +4508,7 @@ hardware_watchpoint_inserted_in_range (const address_space *aspace,
>   	continue;
>   
>         for (bp_location &loc : bpt.locations ())
> -	if (loc.pspace->aspace == aspace && loc.inserted)
> +	if (loc.pspace->aspace.get () == aspace && loc.inserted)
>   	  {
>   	    CORE_ADDR l, h;
>   
> @@ -7330,10 +7330,10 @@ breakpoint_location_address_match (struct bp_location *bl,
>   				   const address_space *aspace,
>   				   CORE_ADDR addr)
>   {
> -  return (breakpoint_address_match (bl->pspace->aspace, bl->address,
> +  return (breakpoint_address_match (bl->pspace->aspace.get (), bl->address,
>   				    aspace, addr)
>   	  || (bl->length
> -	      && breakpoint_address_match_range (bl->pspace->aspace,
> +	      && breakpoint_address_match_range (bl->pspace->aspace.get (),
>   						 bl->address, bl->length,
>   						 aspace, addr)));
>   }
> @@ -7350,7 +7350,7 @@ breakpoint_location_address_range_overlap (struct bp_location *bl,
>   					   CORE_ADDR addr, int len)
>   {
>     if (gdbarch_has_global_breakpoints (current_inferior ()->arch ())
> -      || bl->pspace->aspace == aspace)
> +      || bl->pspace->aspace.get () == aspace)
>       {
>         int bl_len = bl->length != 0 ? bl->length : 1;
>   
> @@ -7407,8 +7407,10 @@ breakpoint_locations_match (const struct bp_location *loc1,
>       /* We compare bp_location.length in order to cover ranged
>          breakpoints.  Keep this in sync with
>          bp_location_is_less_than.  */
> -    return (breakpoint_address_match (loc1->pspace->aspace, loc1->address,
> -				     loc2->pspace->aspace, loc2->address)
> +    return (breakpoint_address_match (loc1->pspace->aspace.get (),
> +				      loc1->address,
> +				      loc2->pspace->aspace.get (),
> +				      loc2->address)
>   	    && (loc1->loc_type == loc2->loc_type || sw_hw_bps_match)
>   	    && loc1->length == loc2->length);
>   }
> @@ -9568,8 +9570,9 @@ ranged_breakpoint::breakpoint_hit (const struct bp_location *bl,
>         || ws.sig () != GDB_SIGNAL_TRAP)
>       return 0;
>   
> -  return breakpoint_address_match_range (bl->pspace->aspace, bl->address,
> -					 bl->length, aspace, bp_addr);
> +  return breakpoint_address_match_range (bl->pspace->aspace.get (),
> +					 bl->address, bl->length, aspace,
> +					 bp_addr);
>   }
>   
>   /* Implement the "resources_needed" method for ranged breakpoints.  */
> @@ -12015,7 +12018,7 @@ code_breakpoint::breakpoint_hit (const struct bp_location *bl,
>         || ws.sig () != GDB_SIGNAL_TRAP)
>       return 0;
>   
> -  if (!breakpoint_address_match (bl->pspace->aspace, bl->address,
> +  if (!breakpoint_address_match (bl->pspace->aspace.get (), bl->address,
>   				 aspace, bp_addr))
>       return 0;
>   
> diff --git a/gdb/inferior.c b/gdb/inferior.c
> index 1778723863e..37b4fad6e44 100644
> --- a/gdb/inferior.c
> +++ b/gdb/inferior.c
> @@ -843,15 +843,13 @@ remove_inferior_command (const char *args, int from_tty)
>   struct inferior *
>   add_inferior_with_spaces (void)
>   {
> -  struct address_space *aspace;
>     struct program_space *pspace;
>     struct inferior *inf;
>   
>     /* If all inferiors share an address space on this system, this
>        doesn't really return a new address space; otherwise, it
>        really does.  */
> -  aspace = maybe_new_address_space ();
> -  pspace = new program_space (aspace);
> +  pspace = new program_space (maybe_new_address_space ());
>     inf = add_inferior (0);
>     inf->pspace = pspace;
>     inf->aspace = pspace->aspace;
> @@ -1014,15 +1012,13 @@ clone_inferior_command (const char *args, int from_tty)
>   
>     for (i = 0; i < copies; ++i)
>       {
> -      struct address_space *aspace;
>         struct program_space *pspace;
>         struct inferior *inf;
>   
>         /* If all inferiors share an address space on this system, this
>   	 doesn't really return a new address space; otherwise, it
>   	 really does.  */
> -      aspace = maybe_new_address_space ();
> -      pspace = new program_space (aspace);
> +      pspace = new program_space (maybe_new_address_space ());
>         inf = add_inferior (0);
>         inf->pspace = pspace;
>         inf->aspace = pspace->aspace;
> diff --git a/gdb/inferior.h b/gdb/inferior.h
> index 33eff7a9141..3f74cb5d8d4 100644
> --- a/gdb/inferior.h
> +++ b/gdb/inferior.h
> @@ -584,7 +584,7 @@ class inferior : public refcounted_object,
>     bool removable = false;
>   
>     /* The address space bound to this inferior.  */
> -  struct address_space *aspace = NULL;
> +  address_space_ref_ptr aspace;
>   
>     /* The program space bound to this inferior.  */
>     struct program_space *pspace = NULL;
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 4c7eb9be792..95620d479d8 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -529,8 +529,8 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
>   	    }
>   	  else
>   	    {
> -	      child_inf->aspace = new address_space ();
> -	      child_inf->pspace = new program_space (child_inf->aspace);
> +	      child_inf->pspace = new program_space (new_address_space ());
> +	      child_inf->aspace = child_inf->pspace->aspace;
>   	      child_inf->removable = true;
>   	      clone_program_space (child_inf->pspace, parent_inf->pspace);
>   	    }
> @@ -608,8 +608,8 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
>   
>   	  child_inf->aspace = parent_inf->aspace;
>   	  child_inf->pspace = parent_inf->pspace;
> -	  parent_inf->aspace = new address_space ();
> -	  parent_inf->pspace = new program_space (parent_inf->aspace);
> +	  parent_inf->pspace = new program_space (new_address_space ());
> +	  parent_inf->aspace = parent_inf->pspace->aspace;
>   	  clone_program_space (parent_inf->pspace, child_inf->pspace);
>   
>   	  /* The parent inferior is still the current one, so keep things
> @@ -618,8 +618,8 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
>   	}
>         else
>   	{
> -	  child_inf->aspace = new address_space ();
> -	  child_inf->pspace = new program_space (child_inf->aspace);
> +	  child_inf->pspace = new program_space (new_address_space ());
> +	  child_inf->aspace = child_inf->pspace->aspace;
>   	  child_inf->removable = true;
>   	  child_inf->symfile_flags = SYMFILE_NO_READ;
>   	  clone_program_space (child_inf->pspace, parent_inf->pspace);
> @@ -1029,7 +1029,6 @@ handle_vfork_child_exec_or_exit (int exec)
>         if (vfork_parent->pending_detach)
>   	{
>   	  struct program_space *pspace;
> -	  struct address_space *aspace;
>   
>   	  /* follow-fork child, detach-on-fork on.  */
>   
> @@ -1054,9 +1053,8 @@ handle_vfork_child_exec_or_exit (int exec)
>   	     of" a hack.  */
>   
>   	  pspace = inf->pspace;
> -	  aspace = inf->aspace;
> -	  inf->aspace = nullptr;
>   	  inf->pspace = nullptr;
> +	  address_space_ref_ptr aspace = std::move (inf->aspace);
>   
>   	  if (print_inferior_events)
>   	    {
> @@ -5905,7 +5903,7 @@ handle_inferior_event (struct execution_control_state *ecs)
>   	      = get_thread_arch_aspace_regcache (parent_inf,
>   						 ecs->ws.child_ptid (),
>   						 gdbarch,
> -						 parent_inf->aspace);
> +						 parent_inf->aspace.get ());
>   	    /* Read PC value of parent process.  */
>   	    parent_pc = regcache_read_pc (regcache);
>   
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index 1c9756c18bd..ca6e57cdaf9 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -4361,7 +4361,7 @@ linux_nat_target::thread_address_space (ptid_t ptid)
>   
>     inf = find_inferior_pid (this, pid);
>     gdb_assert (inf != NULL);
> -  return inf->aspace;
> +  return inf->aspace.get ();
>   }
>   
>   /* Return the cached value of the processor core for thread PTID.  */
> diff --git a/gdb/process-stratum-target.c b/gdb/process-stratum-target.c
> index 173c3ecad13..ebd47bb9282 100644
> --- a/gdb/process-stratum-target.c
> +++ b/gdb/process-stratum-target.c
> @@ -37,7 +37,7 @@ process_stratum_target::thread_address_space (ptid_t ptid)
>   		      "address space of thread %s\n"),
>   		    target_pid_to_str (ptid).c_str ());
>   
> -  return inf->aspace;
> +  return inf->aspace.get ();
>   }
>   
>   struct gdbarch *
> diff --git a/gdb/progspace.c b/gdb/progspace.c
> index 839707e9d71..0fc1fdd57ab 100644
> --- a/gdb/progspace.c
> +++ b/gdb/progspace.c
> @@ -55,8 +55,8 @@ address_space::address_space ()
>      return a pointer to an existing address space, in case inferiors
>      share an address space on this target system.  */
>   
> -struct address_space *
> -maybe_new_address_space (void)
> +address_space_ref_ptr
> +maybe_new_address_space ()
>   {
>     int shared_aspace
>       = gdbarch_has_shared_address_space (current_inferior ()->arch ());
> @@ -67,7 +67,7 @@ maybe_new_address_space (void)
>         return program_spaces[0]->aspace;
>       }
>   
> -  return new address_space ();
> +  return new_address_space ();
>   }
>   
>   /* Start counting over from scratch.  */
> @@ -95,9 +95,9 @@ remove_program_space (program_space *pspace)
>   
>   /* See progspace.h.  */
>   
> -program_space::program_space (address_space *aspace_)
> +program_space::program_space (address_space_ref_ptr aspace_)
>     : num (++last_program_space_num),
> -    aspace (aspace_)
> +    aspace (std::move (aspace_))
>   {
>     program_spaces.push_back (this);
>     gdb::observers::new_program_space.notify (this);
> @@ -122,8 +122,6 @@ program_space::~program_space ()
>     /* Defer breakpoint re-set because we don't want to create new
>        locations for this pspace which we're tearing down.  */
>     clear_symtab_users (SYMFILE_DEFER_BP_RESET);
> -  if (!gdbarch_has_shared_address_space (current_inferior ()->arch ()))
> -    delete this->aspace;
>   }
>   
>   /* See progspace.h.  */
> @@ -411,18 +409,14 @@ update_address_spaces (void)
>   
>     if (shared_aspace)
>       {
> -      struct address_space *aspace = new address_space ();
> +      address_space_ref_ptr aspace = new_address_space ();
>   
> -      delete current_program_space->aspace;
>         for (struct program_space *pspace : program_spaces)
>   	pspace->aspace = aspace;
>       }
>     else
>       for (struct program_space *pspace : program_spaces)
> -      {
> -	delete pspace->aspace;
> -	pspace->aspace = new address_space ();
> -      }
> +      pspace->aspace = new_address_space ();
>   
>     for (inferior *inf : all_inferiors ())
>       if (gdbarch_has_global_solist (current_inferior ()->arch ()))
> @@ -459,5 +453,5 @@ initialize_progspace (void)
>        modules have done that.  Do this before
>        initialize_current_architecture, because that accesses the ebfd
>        of current_program_space.  */
> -  current_program_space = new program_space (new address_space ());
> +  current_program_space = new program_space (new_address_space ());
>   }
> diff --git a/gdb/progspace.h b/gdb/progspace.h
> index a22e427400e..163cbf6a0f8 100644
> --- a/gdb/progspace.h
> +++ b/gdb/progspace.h
> @@ -29,6 +29,8 @@
>   #include "gdbsupport/next-iterator.h"
>   #include "gdbsupport/safe-iterator.h"
>   #include "gdbsupport/intrusive_list.h"
> +#include "gdbsupport/refcounted-object.h"
> +#include "gdbsupport/gdb_ref_ptr.h"
>   #include <list>
>   #include <vector>
>   
> @@ -43,6 +45,40 @@ struct shobj;
>   
>   typedef std::list<std::unique_ptr<objfile>> objfile_list;
>   
> +/* An address space.  It is used for comparing if
> +   pspaces/inferior/threads see the same address space and for
> +   associating caches to each address space.  */
> +struct address_space : public refcounted_object
> +{
> +  /* Create a new address space object, and add it to the list.  */
> +  address_space ();
> +  DISABLE_COPY_AND_ASSIGN (address_space);
> +
> +  /* Returns the integer address space id of this address space.  */
> +  int num () const
> +  {
> +    return m_num;
> +  }
> +
> +  /* Per aspace data-pointers required by other GDB modules.  */
> +  registry<address_space> registry_fields;
> +
> +private:
> +  int m_num;
> +};
> +
> +using address_space_ref_ptr
> +  = gdb::ref_ptr<address_space,
> +		 refcounted_object_delete_ref_policy<address_space>>;
> +
> +/* Create a new address space.  */
> +
> +static inline address_space_ref_ptr
> +new_address_space ()
> +{
> +  return address_space_ref_ptr::new_reference (new address_space);
> +}
> +
>   /* An iterator that wraps an iterator over std::unique_ptr<objfile>,
>      and dereferences the returned object.  This is useful for iterating
>      over a list of shared pointers and returning raw pointers -- which
> @@ -192,7 +228,7 @@ struct program_space
>   {
>     /* Constructs a new empty program space, binds it to ASPACE, and
>        adds it to the program space list.  */
> -  explicit program_space (address_space *aspace);
> +  explicit program_space (address_space_ref_ptr aspace);
>   
>     /* Releases a program space, and all its contents (shared libraries,
>        objfiles, and any other references to the program space in other
> @@ -334,7 +370,7 @@ struct program_space
>        are global, then this field is ignored (we don't currently
>        support inferiors sharing a program space if the target doesn't
>        make breakpoints global).  */
> -  struct address_space *aspace = NULL;
> +  address_space_ref_ptr aspace;
>   
>     /* True if this program space's section offsets don't yet represent
>        the final offsets of the "live" address space (that is, the
> @@ -381,28 +417,6 @@ struct program_space
>     std::vector<target_section> m_target_sections;
>   };
>   
> -/* An address space.  It is used for comparing if
> -   pspaces/inferior/threads see the same address space and for
> -   associating caches to each address space.  */
> -struct address_space
> -{
> -  /* Create a new address space object, and add it to the list.  */
> -  address_space ();
> -  DISABLE_COPY_AND_ASSIGN (address_space);
> -
> -  /* Returns the integer address space id of this address space.  */
> -  int num () const
> -  {
> -    return m_num;
> -  }
> -
> -  /* Per aspace data-pointers required by other GDB modules.  */
> -  registry<address_space> registry_fields;
> -
> -private:
> -  int m_num;
> -};
> -
>   /* The list of all program spaces.  There's always at least one.  */
>   extern std::vector<struct program_space *>program_spaces;
>   
> @@ -445,7 +459,7 @@ class scoped_restore_current_program_space
>   /* Maybe create a new address space object, and add it to the list, or
>      return a pointer to an existing address space, in case inferiors
>      share an address space.  */
> -extern struct address_space *maybe_new_address_space (void);
> +extern address_space_ref_ptr maybe_new_address_space ();
>   
>   /* Update all program spaces matching to address spaces.  The user may
>      have created several program spaces, and loaded executables into
> diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
> index abab79f3132..4a1ecdd1ad6 100644
> --- a/gdb/record-btrace.c
> +++ b/gdb/record-btrace.c
> @@ -2321,7 +2321,7 @@ record_btrace_replay_at_breakpoint (struct thread_info *tp)
>     if (insn == NULL)
>       return 0;
>   
> -  return record_check_stopped_by_breakpoint (tp->inf->aspace, insn->pc,
> +  return record_check_stopped_by_breakpoint (tp->inf->aspace.get (), insn->pc,
>   					     &btinfo->stop_reason);
>   }
>   
> diff --git a/gdb/regcache.c b/gdb/regcache.c
> index 2e48c025809..239656d1987 100644
> --- a/gdb/regcache.c
> +++ b/gdb/regcache.c
> @@ -1628,7 +1628,7 @@ get_thread_arch_aspace_regcache_and_check (inferior *inf_for_target_calls,
>        the current inferior's gdbarch.  Also use the current inferior's address
>        space.  */
>     gdbarch *arch = inf_for_target_calls->arch ();
> -  address_space *aspace = inf_for_target_calls->aspace;
> +  address_space *aspace = inf_for_target_calls->aspace.get ();
>     regcache *regcache = get_thread_arch_aspace_regcache (inf_for_target_calls,
>   							ptid, arch, aspace);
>   
> diff --git a/gdb/scoped-mock-context.h b/gdb/scoped-mock-context.h
> index 5b5f46df7ba..70b913b949b 100644
> --- a/gdb/scoped-mock-context.h
> +++ b/gdb/scoped-mock-context.h
> @@ -38,7 +38,7 @@ struct scoped_mock_context
>   
>     Target mock_target;
>     ptid_t mock_ptid {1, 1};
> -  program_space mock_pspace {new address_space ()};
> +  program_space mock_pspace {new_address_space ()};
>     inferior mock_inferior {mock_ptid.pid ()};
>     thread_info mock_thread {&mock_inferior, mock_ptid};
>   
> diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
> index 13c2888e7ea..b1b772ab76e 100644
> --- a/gdb/target-dcache.c
> +++ b/gdb/target-dcache.c
> @@ -33,7 +33,7 @@ int
>   target_dcache_init_p (void)
>   {
>     DCACHE *dcache
> -    = target_dcache_aspace_key.get (current_program_space->aspace);
> +    = target_dcache_aspace_key.get (current_program_space->aspace.get ());
>   
>     return (dcache != NULL);
>   }
> @@ -44,7 +44,7 @@ void
>   target_dcache_invalidate (void)
>   {
>     DCACHE *dcache
> -    = target_dcache_aspace_key.get (current_program_space->aspace);
> +    = target_dcache_aspace_key.get (current_program_space->aspace.get ());
>   
>     if (dcache != NULL)
>       dcache_invalidate (dcache);
> @@ -56,7 +56,7 @@ target_dcache_invalidate (void)
>   DCACHE *
>   target_dcache_get (void)
>   {
> -  return target_dcache_aspace_key.get (current_program_space->aspace);
> +  return target_dcache_aspace_key.get (current_program_space->aspace.get ());
>   }
>   
>   /* Return the target dcache.  If it is not initialized yet, initialize
> @@ -66,12 +66,13 @@ DCACHE *
>   target_dcache_get_or_init (void)
>   {
>     DCACHE *dcache
> -    = target_dcache_aspace_key.get (current_program_space->aspace);
> +    = target_dcache_aspace_key.get (current_program_space->aspace.get ());
>   
>     if (dcache == NULL)
>       {
>         dcache = dcache_init ();
> -      target_dcache_aspace_key.set (current_program_space->aspace, dcache);
> +      target_dcache_aspace_key.set (current_program_space->aspace.get (),
> +				    dcache);
>       }
>   
>     return dcache;
> diff --git a/gdbsupport/refcounted-object.h b/gdbsupport/refcounted-object.h
> index d8fdb950043..294fd873df1 100644
> --- a/gdbsupport/refcounted-object.h
> +++ b/gdbsupport/refcounted-object.h
> @@ -67,4 +67,21 @@ struct refcounted_object_ref_policy
>     }
>   };
>   
> +/* A policy class to interface gdb::ref_ptr with a refcounted_object, that
> +   deletes the object once the refcount reaches 0..  */
> +
> +template<typename T>
> +struct refcounted_object_delete_ref_policy
> +{
> +  static void incref (T *obj)
> +  { obj->incref (); }
> +
> +  static void decref (T *obj)
> +  {
> +    obj->decref ();
> +    if (obj->refcount () == 0)
> +      delete obj;
> +  }
> +};
> +
>   #endif /* COMMON_REFCOUNTED_OBJECT_H */