public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: "Schimpe, Christina" <christina.schimpe@intel.com>
To: Luis Machado <luis.machado@arm.com>,
	"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: RE: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
Date: Tue, 5 Nov 2024 14:07:03 +0000	[thread overview]
Message-ID: <PH0PR11MB7636E73937BA91CBDE2AEFC9F9522@PH0PR11MB7636.namprd11.prod.outlook.com> (raw)
In-Reply-To: <5b1646e6-4cfc-43e9-afc5-4a6cacb787bb@arm.com>

Hi Luis, 

Thanks a lot for the review. I have one questions to your comment, please see below.

> -----Original Message-----
> From: Luis Machado <luis.machado@arm.com>
> Sent: Monday, November 4, 2024 12:18 PM
> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH v6 1/2] gdb: Make tagged pointer support configurable.
> 
> Hi Christina,
> 
> On 10/28/24 08:46, Schimpe, Christina wrote:
> > From: Christina Schimpe <christina.schimpe@intel.com>
> >
> > The gdbarch function gdbarch_remove_non_address_bits adjusts addresses
> > to enable debugging of programs with tagged pointers on Linux, for
> > instance for ARM's feature top byte ignore (TBI).
> > Once the function is implemented for an architecture, it adjusts
> > addresses for memory access, breakpoints and watchpoints.
> >
> > Linear address masking (LAM) is Intel's (R) implementation of tagged
> > pointer support.  It requires certain adaptions to GDB's tagged
> > pointer support due to the following:
> > - LAM supports address tagging for data accesses only.  Thus, specifying
> >   breakpoints on tagged addresses is not a valid use case.
> > - In contrast to the implementation for ARM's TBI, the Linux kernel supports
> >   tagged pointers for memory access.
> >
> > This patch makes GDB's tagged pointer support configurable such that
> > it is possible to enable the address adjustment for a specific feature
> > only (e.g memory access, breakpoints or watchpoints).  This way, one
> > can make sure that addresses are only adjusted when necessary.  In
> > case of LAM, this avoids unnecessary parsing of the /proc/<pid>/status
> > file to get the untag mask.
> >
> > Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
> > ---
> >  gdb/aarch64-linux-nat.c   |  2 +-
> >  gdb/aarch64-linux-tdep.c  |  7 +++--
> >  gdb/aarch64-tdep.c        | 23 ++++++++------
> >  gdb/aarch64-tdep.h        |  6 ++++
> >  gdb/breakpoint.c          |  5 +--
> >  gdb/gdbarch-gen.c         | 66 ++++++++++++++++++++++++++++++++-------
> >  gdb/gdbarch-gen.h         | 49 ++++++++++++++++++++++-------
> >  gdb/gdbarch_components.py | 53 ++++++++++++++++++++++++++-----
> > gdb/loongarch-linux-nat.c |  2 +-
> >  gdb/target.c              |  3 +-
> >  10 files changed, 169 insertions(+), 47 deletions(-)
> >
> > diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c index
> > 0fa5bee500b..48ab765d880 100644
> > --- a/gdb/aarch64-linux-nat.c
> > +++ b/gdb/aarch64-linux-nat.c
> > @@ -943,7 +943,7 @@ aarch64_linux_nat_target::stopped_data_address
> (CORE_ADDR *addr_p)
> >       kernel can potentially be tagged addresses.  */
> >    struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
> >    const CORE_ADDR addr_trap
> > -    = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR)
> siginfo.si_addr);
> > +    = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR)
> > + siginfo.si_addr);
> >
> >    /* Check if the address matches any watched address.  */
> >    state = aarch64_get_debug_reg_state (inferior_ptid.pid ()); diff
> > --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c index
> > c608a84bc71..61c3be8b6f0 100644
> > --- a/gdb/aarch64-linux-tdep.c
> > +++ b/gdb/aarch64-linux-tdep.c
> > @@ -2433,7 +2433,7 @@ static bool
> >  aarch64_linux_tagged_address_p (struct gdbarch *gdbarch, CORE_ADDR
> > address)  {
> >    /* Remove the top byte for the memory range check.  */
> > -  address = gdbarch_remove_non_address_bits (gdbarch, address);
> > +  address = aarch64_remove_non_address_bits (gdbarch, address);
> >
> >    /* Check if the page that contains ADDRESS is mapped with PROT_MTE.  */
> >    if (!linux_address_in_memtag_page (address)) @@ -2491,8 +2491,9 @@
> > aarch64_linux_report_signal_info (struct gdbarch *gdbarch,
> >        uiout->text ("\n");
> >
> >        std::optional<CORE_ADDR> atag
> > -	= aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch,
> > -								 fault_addr));
> > +	= aarch64_mte_get_atag (
> > +	    aarch64_remove_non_address_bits (gdbarch, fault_addr));
> > +
> >        gdb_byte ltag = aarch64_mte_get_ltag (fault_addr);
> >
> >        if (!atag.has_value ())
> > diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index
> > 8a2a9b1e23c..91fec7879ed 100644
> > --- a/gdb/aarch64-tdep.c
> > +++ b/gdb/aarch64-tdep.c
> > @@ -4121,7 +4121,7 @@ aarch64_memtag_matches_p (struct gdbarch
> > *gdbarch,
> >
> >    /* Fetch the allocation tag for ADDRESS.  */
> >    std::optional<CORE_ADDR> atag
> > -    = aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch,
> addr));
> > +    = aarch64_mte_get_atag (aarch64_remove_non_address_bits (gdbarch,
> > + addr));
> >
> >    if (!atag.has_value ())
> >      return true;
> > @@ -4160,7 +4160,7 @@ aarch64_set_memtags (struct gdbarch *gdbarch,
> struct value *address,
> >    else
> >      {
> >        /* Remove the top byte.  */
> > -      addr = gdbarch_remove_non_address_bits (gdbarch, addr);
> > +      addr = aarch64_remove_non_address_bits (gdbarch, addr);
> >
> >        /* With G being the number of tag granules and N the number of tags
> >  	 passed in, we can have the following cases:
> > @@ -4209,7 +4209,7 @@ aarch64_get_memtag (struct gdbarch *gdbarch,
> struct value *address,
> >    else
> >      {
> >        /* Remove the top byte.  */
> > -      addr = gdbarch_remove_non_address_bits (gdbarch, addr);
> > +      addr = aarch64_remove_non_address_bits (gdbarch, addr);
> >        std::optional<CORE_ADDR> atag = aarch64_mte_get_atag (addr);
> >
> >        if (!atag.has_value ())
> > @@ -4236,10 +4236,9 @@ aarch64_memtag_to_string (struct gdbarch
> *gdbarch, struct value *tag_value)
> >    return string_printf ("0x%s", phex_nz (tag, sizeof (tag)));  }
> >
> > -/* AArch64 implementation of the remove_non_address_bits gdbarch hook.
> Remove
> > -   non address bits from a pointer value.  */
> > +/* See aarch64-tdep.h.  */
> >
> > -static CORE_ADDR
> > +CORE_ADDR
> >  aarch64_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR
> > pointer)  {
> >    /* By default, we assume TBI and discard the top 8 bits plus the VA
> > range @@ -4750,9 +4749,15 @@ aarch64_gdbarch_init (struct gdbarch_info
> info, struct gdbarch_list *arches)
> >      tdep->ra_sign_state_regnum = ra_sign_state_offset + num_regs;
> >
> >    /* Architecture hook to remove bits of a pointer that are not part of the
> > -     address, like memory tags (MTE) and pointer authentication signatures.  */
> > -  set_gdbarch_remove_non_address_bits (gdbarch,
> > -				       aarch64_remove_non_address_bits);
> > +     address, like memory tags (MTE) and pointer authentication signatures.
> > +     Configure address adjustment for watch-, breakpoints and memory
> 
> s/watch-/watchpoints

Fill fix, thanks.

> > +     transfer.  */
> > +  set_gdbarch_remove_non_address_bits_watchpoint
> > +    (gdbarch, aarch64_remove_non_address_bits);
> > + set_gdbarch_remove_non_address_bits_breakpoint
> > +    (gdbarch, aarch64_remove_non_address_bits);
> > + set_gdbarch_remove_non_address_bits_memory
> > +    (gdbarch, aarch64_remove_non_address_bits);
> >
> >    /* SME pseudo-registers.  */
> >    if (tdep->has_sme ())
> > diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h index
> > 50166fb4f24..13ea37de7a3 100644
> > --- a/gdb/aarch64-tdep.h
> > +++ b/gdb/aarch64-tdep.h
> > @@ -205,4 +205,10 @@ bool aarch64_displaced_step_hw_singlestep (struct
> > gdbarch *gdbarch);
> >
> >  std::optional<CORE_ADDR> aarch64_mte_get_atag (CORE_ADDR address);
> >
> > +/* AArch64 implementation of the remove_non_address_bits gdbarch hooks.
> > +   Remove non address bits from a pointer value.  */
> > +
> > +CORE_ADDR aarch64_remove_non_address_bits (struct gdbarch *gdbarch,
> > +					   CORE_ADDR pointer);
> > +
> >  #endif /* aarch64-tdep.h */
> > diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index
> > b7e4f5d0a45..9983e905e1b 100644
> > --- a/gdb/breakpoint.c
> > +++ b/gdb/breakpoint.c
> > @@ -2313,7 +2313,8 @@ update_watchpoint (struct watchpoint *b, bool
> reparse)
> >  		  loc->gdbarch = v->type ()->arch ();
> >  		  loc->pspace = wp_pspace;
> >  		  loc->address
> > -		    = gdbarch_remove_non_address_bits (loc->gdbarch, addr);
> > +		    = gdbarch_remove_non_address_bits_watchpoint (loc-
> >gdbarch,
> > +								  addr);
> >  		  b->add_location (*loc);
> >
> >  		  if (bitsize != 0)
> > @@ -7538,7 +7539,7 @@ adjust_breakpoint_address (struct gdbarch
> *gdbarch,
> >  	}
> >
> >        adjusted_bpaddr
> > -	= gdbarch_remove_non_address_bits (gdbarch, adjusted_bpaddr);
> > +	= gdbarch_remove_non_address_bits_breakpoint (gdbarch,
> > +adjusted_bpaddr);
> >
> >        /* An adjusted breakpoint address can significantly alter
> >  	 a user's expectations.  Print a warning if an adjustment diff --git
> > a/gdb/gdbarch-gen.c b/gdb/gdbarch-gen.c index 0d00cd7c993..d05c7a3cbdf
> > 100644
> > --- a/gdb/gdbarch-gen.c
> > +++ b/gdb/gdbarch-gen.c
> > @@ -143,7 +143,9 @@ struct gdbarch
> >    int frame_red_zone_size = 0;
> >    gdbarch_convert_from_func_ptr_addr_ftype *convert_from_func_ptr_addr =
> convert_from_func_ptr_addr_identity;
> >    gdbarch_addr_bits_remove_ftype *addr_bits_remove =
> > core_addr_identity;
> > -  gdbarch_remove_non_address_bits_ftype *remove_non_address_bits =
> > default_remove_non_address_bits;
> > +  gdbarch_remove_non_address_bits_watchpoint_ftype
> > + *remove_non_address_bits_watchpoint =
> > + default_remove_non_address_bits;
> > + gdbarch_remove_non_address_bits_breakpoint_ftype
> > + *remove_non_address_bits_breakpoint =
> > + default_remove_non_address_bits;
> > + gdbarch_remove_non_address_bits_memory_ftype
> > + *remove_non_address_bits_memory = default_remove_non_address_bits;
> >    gdbarch_memtag_to_string_ftype *memtag_to_string =
> default_memtag_to_string;
> >    gdbarch_tagged_address_p_ftype *tagged_address_p =
> default_tagged_address_p;
> >    gdbarch_memtag_matches_p_ftype *memtag_matches_p =
> > default_memtag_matches_p; @@ -407,7 +409,9 @@ verify_gdbarch (struct
> gdbarch *gdbarch)
> >    /* Skip verify of frame_red_zone_size, invalid_p == 0.  */
> >    /* Skip verify of convert_from_func_ptr_addr, invalid_p == 0.  */
> >    /* Skip verify of addr_bits_remove, invalid_p == 0.  */
> > -  /* Skip verify of remove_non_address_bits, invalid_p == 0.  */
> > +  /* Skip verify of remove_non_address_bits_watchpoint, invalid_p ==
> > + 0.  */
> > +  /* Skip verify of remove_non_address_bits_breakpoint, invalid_p ==
> > + 0.  */
> > +  /* Skip verify of remove_non_address_bits_memory, invalid_p == 0.
> > + */
> >    /* Skip verify of memtag_to_string, invalid_p == 0.  */
> >    /* Skip verify of tagged_address_p, invalid_p == 0.  */
> >    /* Skip verify of memtag_matches_p, invalid_p == 0.  */ @@ -910,8
> > +914,14 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
> >  	      "gdbarch_dump: addr_bits_remove = <%s>\n",
> >  	      host_address_to_string (gdbarch->addr_bits_remove));
> >    gdb_printf (file,
> > -	      "gdbarch_dump: remove_non_address_bits = <%s>\n",
> > -	      host_address_to_string (gdbarch->remove_non_address_bits));
> > +	      "gdbarch_dump: remove_non_address_bits_watchpoint = <%s>\n",
> > +	      host_address_to_string
> > +(gdbarch->remove_non_address_bits_watchpoint));
> > +  gdb_printf (file,
> > +	      "gdbarch_dump: remove_non_address_bits_breakpoint = <%s>\n",
> > +	      host_address_to_string
> > +(gdbarch->remove_non_address_bits_breakpoint));
> > +  gdb_printf (file,
> > +	      "gdbarch_dump: remove_non_address_bits_memory = <%s>\n",
> > +	      host_address_to_string
> > +(gdbarch->remove_non_address_bits_memory));
> >    gdb_printf (file,
> >  	      "gdbarch_dump: memtag_to_string = <%s>\n",
> >  	      host_address_to_string (gdbarch->memtag_to_string)); @@
> > -3198,20 +3208,54 @@ set_gdbarch_addr_bits_remove (struct gdbarch
> > *gdbarch,  }
> >
> >  CORE_ADDR
> > -gdbarch_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR
> > pointer)
> > +gdbarch_remove_non_address_bits_watchpoint (struct gdbarch *gdbarch,
> > +CORE_ADDR pointer) {
> > +  gdb_assert (gdbarch != NULL);
> > +  gdb_assert (gdbarch->remove_non_address_bits_watchpoint != NULL);
> > +  if (gdbarch_debug >= 2)
> > +    gdb_printf (gdb_stdlog,
> > +"gdbarch_remove_non_address_bits_watchpoint called\n");
> > +  return gdbarch->remove_non_address_bits_watchpoint (gdbarch,
> > +pointer); }
> > +
> > +void
> > +set_gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
> *gdbarch,
> > +
> 	gdbarch_remove_non_address_bits_watchpoint_ftype
> > +remove_non_address_bits_watchpoint)
> > +{
> > +  gdbarch->remove_non_address_bits_watchpoint =
> > +remove_non_address_bits_watchpoint;
> > +}
> > +
> > +CORE_ADDR
> > +gdbarch_remove_non_address_bits_breakpoint (struct gdbarch *gdbarch,
> > +CORE_ADDR pointer) {
> > +  gdb_assert (gdbarch != NULL);
> > +  gdb_assert (gdbarch->remove_non_address_bits_breakpoint != NULL);
> > +  if (gdbarch_debug >= 2)
> > +    gdb_printf (gdb_stdlog,
> > +"gdbarch_remove_non_address_bits_breakpoint called\n");
> > +  return gdbarch->remove_non_address_bits_breakpoint (gdbarch,
> > +pointer); }
> > +
> > +void
> > +set_gdbarch_remove_non_address_bits_breakpoint (struct gdbarch *gdbarch,
> > +
> 	gdbarch_remove_non_address_bits_breakpoint_ftype
> > +remove_non_address_bits_breakpoint)
> > +{
> > +  gdbarch->remove_non_address_bits_breakpoint =
> > +remove_non_address_bits_breakpoint;
> > +}
> > +
> > +CORE_ADDR
> > +gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
> > +CORE_ADDR pointer)
> >  {
> >    gdb_assert (gdbarch != NULL);
> > -  gdb_assert (gdbarch->remove_non_address_bits != NULL);
> > +  gdb_assert (gdbarch->remove_non_address_bits_memory != NULL);
> >    if (gdbarch_debug >= 2)
> > -    gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits called\n");
> > -  return gdbarch->remove_non_address_bits (gdbarch, pointer);
> > +    gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits_memory
> > + called\n");  return gdbarch->remove_non_address_bits_memory
> > + (gdbarch, pointer);
> >  }
> >
> >  void
> > -set_gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
> > -				     gdbarch_remove_non_address_bits_ftype
> remove_non_address_bits)
> > +set_gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
> > +
> gdbarch_remove_non_address_bits_memory_ftype
> > +remove_non_address_bits_memory)
> >  {
> > -  gdbarch->remove_non_address_bits = remove_non_address_bits;
> > +  gdbarch->remove_non_address_bits_memory =
> > + remove_non_address_bits_memory;
> >  }
> >
> >  std::string
> > diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h index
> > b982fd7cd09..9fda85f860f 100644
> > --- a/gdb/gdbarch-gen.h
> > +++ b/gdb/gdbarch-gen.h
> > @@ -684,19 +684,46 @@ extern CORE_ADDR gdbarch_addr_bits_remove
> > (struct gdbarch *gdbarch, CORE_ADDR ad  extern void
> > set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch,
> > gdbarch_addr_bits_remove_ftype *addr_bits_remove);
> >
> >  /* On some architectures, not all bits of a pointer are significant.
> > -   On AArch64, for example, the top bits of a pointer may carry a "tag", which
> > -   can be ignored by the kernel and the hardware.  The "tag" can be regarded as
> > -   additional data associated with the pointer, but it is not part of the address.
> > +   On AArch64 and amd64, for example, the top bits of a pointer may carry a
> > +   "tag", which can be ignored by the kernel and the hardware.  The "tag" can
> be
> > +   regarded as additional data associated with the pointer, but it is not part
> > +   of the address.
> >
> >     Given a pointer for the architecture, this hook removes all the
> > -   non-significant bits and sign-extends things as needed.  It gets used to
> remove
> > -   non-address bits from data pointers (for example, removing the AArch64 MTE
> tag
> > -   bits from a pointer) and from code pointers (removing the AArch64 PAC
> signature
> > -   from a pointer containing the return address). */
> > -
> > -typedef CORE_ADDR (gdbarch_remove_non_address_bits_ftype) (struct
> > gdbarch *gdbarch, CORE_ADDR pointer); -extern CORE_ADDR
> > gdbarch_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR
> > pointer); -extern void set_gdbarch_remove_non_address_bits (struct
> > gdbarch *gdbarch, gdbarch_remove_non_address_bits_ftype
> > *remove_non_address_bits);
> > +   non-significant bits and sign-extends things as needed.  It gets used to
> > +   remove non-address bits from pointers used for watchpoints. */
> > +
> > +typedef CORE_ADDR (gdbarch_remove_non_address_bits_watchpoint_ftype)
> > +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
> > +gdbarch_remove_non_address_bits_watchpoint (struct gdbarch *gdbarch,
> > +CORE_ADDR pointer); extern void
> > +set_gdbarch_remove_non_address_bits_watchpoint (struct gdbarch
> > +*gdbarch, gdbarch_remove_non_address_bits_watchpoint_ftype
> > +*remove_non_address_bits_watchpoint);
> > +
> > +/* On some architectures, not all bits of a pointer are significant.
> > +   On AArch64 and amd64, for example, the top bits of a pointer may carry a
> > +   "tag", which can be ignored by the kernel and the hardware.  The "tag" can
> be
> > +   regarded as additional data associated with the pointer, but it is not part
> > +   of the address.
> > +
> > +   Given a pointer for the architecture, this hook removes all the
> > +   non-significant bits and sign-extends things as needed.  It gets used to
> > +   remove non-address bits from pointers used for breakpoints. */
> > +
> > +typedef CORE_ADDR (gdbarch_remove_non_address_bits_breakpoint_ftype)
> > +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
> > +gdbarch_remove_non_address_bits_breakpoint (struct gdbarch *gdbarch,
> > +CORE_ADDR pointer); extern void
> > +set_gdbarch_remove_non_address_bits_breakpoint (struct gdbarch
> > +*gdbarch, gdbarch_remove_non_address_bits_breakpoint_ftype
> > +*remove_non_address_bits_breakpoint);
> > +
> > +/* On some architectures, not all bits of a pointer are significant.
> > +   On AArch64 and amd64, for example, the top bits of a pointer may carry a
> > +   "tag", which can be ignored by the kernel and the hardware.  The "tag" can
> be
> > +   regarded as additional data associated with the pointer, but it is not part
> > +   of the address.
> > +
> > +   Given a pointer for the architecture, this hook removes all the
> > +   non-significant bits and sign-extends things as needed.  It gets used to
> > +   remove non-address bits from any pointer used to access memory. */
> > +
> > +typedef CORE_ADDR (gdbarch_remove_non_address_bits_memory_ftype)
> > +(struct gdbarch *gdbarch, CORE_ADDR pointer); extern CORE_ADDR
> > +gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
> > +CORE_ADDR pointer); extern void
> > +set_gdbarch_remove_non_address_bits_memory (struct gdbarch *gdbarch,
> > +gdbarch_remove_non_address_bits_memory_ftype
> > +*remove_non_address_bits_memory);
> >
> >  /* Return a string representation of the memory tag TAG. */
> >
> > diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
> > index 4006380076d..cc7c6d8677b 100644
> > --- a/gdb/gdbarch_components.py
> > +++ b/gdb/gdbarch_components.py
> > @@ -1232,18 +1232,55 @@ possible it should be in TARGET_READ_PC
> instead).
> >  Method(
> >      comment="""
> >  On some architectures, not all bits of a pointer are significant.
> > -On AArch64, for example, the top bits of a pointer may carry a "tag",
> > which -can be ignored by the kernel and the hardware.  The "tag" can
> > be regarded as -additional data associated with the pointer, but it is not part of
> the address.
> > +On AArch64 and amd64, for example, the top bits of a pointer may
> > +carry a "tag", which can be ignored by the kernel and the hardware.
> > +The "tag" can be regarded as additional data associated with the
> > +pointer, but it is not part of the address.
> >
> >  Given a pointer for the architecture, this hook removes all the
> > -non-significant bits and sign-extends things as needed.  It gets used
> > to remove -non-address bits from data pointers (for example, removing
> > the AArch64 MTE tag -bits from a pointer) and from code pointers
> > (removing the AArch64 PAC signature -from a pointer containing the return
> address).
> > +non-significant bits and sign-extends things as needed.  It gets used
> > +to remove non-address bits from pointers used for watchpoints.
> >  """,
> >      type="CORE_ADDR",
> > -    name="remove_non_address_bits",
> > +    name="remove_non_address_bits_watchpoint",
> > +    params=[("CORE_ADDR", "pointer")],
> > +    predefault="default_remove_non_address_bits",
> > +    invalid=False,
> > +)
> > +
> > +Method(
> > +    comment="""
> > +On some architectures, not all bits of a pointer are significant.
> > +On AArch64 and amd64, for example, the top bits of a pointer may
> > +carry a "tag", which can be ignored by the kernel and the hardware.
> > +The "tag" can be regarded as additional data associated with the
> > +pointer, but it is not part of the address.
> > +
> > +Given a pointer for the architecture, this hook removes all the
> > +non-significant bits and sign-extends things as needed.  It gets used
> > +to remove non-address bits from pointers used for breakpoints.
> > +""",
> > +    type="CORE_ADDR",
> > +    name="remove_non_address_bits_breakpoint",
> > +    params=[("CORE_ADDR", "pointer")],
> > +    predefault="default_remove_non_address_bits",
> > +    invalid=False,
> > +)
> > +
> > +Method(
> > +    comment="""
> > +On some architectures, not all bits of a pointer are significant.
> > +On AArch64 and amd64, for example, the top bits of a pointer may
> > +carry a "tag", which can be ignored by the kernel and the hardware.
> > +The "tag" can be regarded as additional data associated with the
> > +pointer, but it is not part of the address.
> > +
> > +Given a pointer for the architecture, this hook removes all the
> > +non-significant bits and sign-extends things as needed.  It gets used
> > +to remove non-address bits from any pointer used to access memory.
> > +""",
> > +    type="CORE_ADDR",
> > +    name="remove_non_address_bits_memory",
> >      params=[("CORE_ADDR", "pointer")],
> >      predefault="default_remove_non_address_bits",
> >      invalid=False,
> > diff --git a/gdb/loongarch-linux-nat.c b/gdb/loongarch-linux-nat.c
> > index bc9927dd751..dc4a019614a 100644
> > --- a/gdb/loongarch-linux-nat.c
> > +++ b/gdb/loongarch-linux-nat.c
> > @@ -613,7 +613,7 @@ loongarch_linux_nat_target::stopped_data_address
> (CORE_ADDR *addr_p)
> >       kernel can potentially be tagged addresses.  */
> >    struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
> >    const CORE_ADDR addr_trap
> > -    = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR)
> siginfo.si_addr);
> > +    = aarch64_remove_non_address_bits (gdbarch, (CORE_ADDR)
> > + siginfo.si_addr);
> >
> >    /* Check if the address matches any watched address.  */
> >    state = loongarch_get_debug_reg_state (inferior_ptid.pid ());
> 
> I think the loongarch-linux-nat.c change is spurious. Could you please address
> that?

Why is it spurious? What do you mean exactly?

Thanks,
Christina
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

  reply	other threads:[~2024-11-05 14:07 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-28  8:46 [PATCH v6 0/2] Add amd64 LAM watchpoint support Schimpe, Christina
2024-10-28  8:46 ` [PATCH v6 1/2] gdb: Make tagged pointer support configurable Schimpe, Christina
2024-11-04 11:17   ` Luis Machado
2024-11-05 14:07     ` Schimpe, Christina [this message]
2024-11-05 14:22       ` Luis Machado
2024-11-05 14:40         ` Schimpe, Christina
2024-11-05 14:51           ` Luis Machado
2024-11-05 15:09             ` Schimpe, Christina
2024-11-05 15:14               ` Luis Machado
2024-11-05 15:36                 ` Schimpe, Christina
2024-10-28  8:46 ` [PATCH v6 2/2] LAM: Enable tagged pointer support for watchpoints Schimpe, Christina
2024-11-04  8:37 ` [PATCH v6 0/2] Add amd64 LAM watchpoint support Schimpe, Christina

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=PH0PR11MB7636E73937BA91CBDE2AEFC9F9522@PH0PR11MB7636.namprd11.prod.outlook.com \
    --to=christina.schimpe@intel.com \
    --cc=gdb-patches@sourceware.org \
    --cc=luis.machado@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).