Re: [PATCH v5 17/25] AArch64: Report tag violation error information

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index 1063dd7a82..4ccee9c34e 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -1700,6 +1700,69 @@ aarch64_linux_memtag_to_string (struct gdbarch *gdbarch,
>    return string_printf ("0x%s", phex_nz (tag, sizeof (tag)));
>  }
>  
> +/* AArch64 Linux implementation of the report_signal_info gdbarch
> +   hook.  Displays information about possible memory tag violations.  */
> +
> +static void
> +aarch64_linux_report_signal_info (struct gdbarch *gdbarch,
> +				  struct ui_out *uiout,
> +				  enum gdb_signal siggnal)
> +{
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +
> +  if (!tdep->has_mte () || siggnal != GDB_SIGNAL_SEGV)
> +    return;
> +
> +  CORE_ADDR fault_addr = 0;
> +  long si_code = 0;
> +
> +  try
> +    {
> +      /* Sigcode tells us if the segfault is actually a memory tag
> +	 violation.  */
> +      si_code = parse_and_eval_long ("$_siginfo.si_code");
> +
> +      fault_addr
> +	= parse_and_eval_long ("$_siginfo._sifields._sigfault.si_addr");
> +    }
> +  catch (const gdb_exception_error &exception)
> +    {
> +      exception_print (gdb_stderr, exception);
> +      return;
> +    }
> +
> +  /* If this is not a memory tag violation, just return.  */
> +  if (si_code != SEGV_MTEAERR && si_code != SEGV_MTESERR)
> +    return;
> +
> +  uiout->text ("\n");
> +
> +  uiout->field_string ("sigcode-meaning", _("Memory tag violation"));
> +
> +  /* For synchronous faults, show additional information.  */
> +  if (si_code == SEGV_MTESERR)
> +    {
> +      uiout->text (_(" while accessing address "));
> +      uiout->field_core_addr ("fault-addr", gdbarch, fault_addr);
> +      uiout->text ("\n");

I guess there's no easy way of getting the logical address that was used
to attempt the memory access?  I think it would be nice to say something
like "Your pointer had logical tag X, but the pointed memory has
allocation tag Y".  But if we can't, we can't.

> +
> +      gdb::optional<CORE_ADDR> atag = aarch64_mte_get_atag (fault_addr);
> +
> +      if (!atag.has_value ())
> +	uiout->text (_("Allocation tag unavailable"));
> +      else
> +	{
> +	  uiout->text (_("Allocation tag "));
> +	  uiout->field_string ("allocation-tag", hex_string (*atag));
> +	}
> +    }
> +  else
> +    {
> +      uiout->text ("\n");
> +      uiout->text (_("Fault address unavailable"));
> +    }
> +}

I guess all of the `uiout->field_*` will appear as the stop event in MI?
I suppose that's fine, but we need to be careful to treat it as API and
not change it in a backwards-incompatible way.  Can you give it a try
with MI to see the result, see if it makes sense (if you haven't
already)?

And if this indeed adds new MI fields, they should be documented, in
theory.  Although I realize that there are equivalent fields in
i386-linux-tdep.c and sparc64-linux-tdep.c which aren't.

But otherwise, that LGTM.

Simon