Re: [PATCH v3 16/24] AArch64: Report tag violation error information

David Spickett via Gdb-patches
2020-11-17T09:36:51Z

> This is a GDB-ism. The hook itself is named like that. I don't like the name, but I thought I'd leave it the same just to keep it standard.

Fair enough.

> We could print it without leading zeroes. That may make things a bit more compact. Would that work?

Sounds good to me.

On Mon, 16 Nov 2020 at 16:46, Luis Machado <luis.machado@linaro.org> wrote:
>
> On 11/16/20 12:43 PM, David Spickett wrote:
> >> +aarch64_linux_report_signal_info (struct gdbarch *gdbarch,
> >> + struct ui_out *uiout,
> >> + enum gdb_signal siggnal)
> >
> > siggnal -> signal
> >
>
> This is a GDB-ism. The hook itself is named like that. I don't like the
> name, but I thought I'd leave it the same just to keep it standard.
>
> > Also a nit about the formatting.
> >
> > Copying from 20/24 of v3:
> >> +Memory tag violation while accessing address 0x0000fffff7ff8000
> >> +Allocation tag 0x0000000000000001.
> >
> > (I'm guessing the code to print the "Allocation tag" line lives here
> > in some part)
> >
> > Would it make more sense to either not align the tag value to
> > anything, or to align it to the size of the architecture's tag? Which
> > would be 4 bits in this case so the same either way.
>
> We could print it without leading zeroes. That may make things a bit
> more compact. Would that work?
>
> >
> > On Mon, 9 Nov 2020 at 17:05, Luis Machado <luis.machado@linaro.org> wrote:
> >>
> >> Whenever a memory tag violation occurs, we get a SIGSEGV. Additional
> >> information can be obtained through the siginfo data structure.
> >>
> >> For AArch64 the Linux kernel may expose the fault address and tag
> >> information, if we have a synchronous event. Otherwise there is
> >> no fault address available.
> >>
> >> gdb/ChangeLog:
> >>
> >> YYYY-MM-DD Luis Machado <luis.machado@linaro.org>
> >>
> >> * aarch64-linux-tdep.c
> >> (aarch64_linux_report_signal_info): New function.
> >> (aarch64_linux_init_abi): Register
> >> aarch64_linux_report_signal_info as the report_signal_info hook.
> >> * arch/aarch64-linux.h (SEGV_MTEAERR): Define.
> >> (SEGV_MTESERR): Define.
> >> ---
> >> gdb/aarch64-linux-tdep.c | 64 ++++++++++++++++++++++++++++++++++++
> >> gdb/arch/aarch64-mte-linux.h | 6 ++++
> >> 2 files changed, 70 insertions(+)
> >>
> >> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> >> index 39b1790263..70e180e1cb 100644
> >> --- a/gdb/aarch64-linux-tdep.c
> >> +++ b/gdb/aarch64-linux-tdep.c
> >> @@ -1626,6 +1626,67 @@ 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\n");
> >> +
> >> + fault_addr
> >> + = parse_and_eval_long ("$_siginfo._sifields._sigfault.si_addr");
> >> + }
> >> + catch (const gdb_exception &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");
> >> +
> >> + CORE_ADDR atag;
> >> + if (aarch64_linux_get_atag (fault_addr, &atag) != 0)
> >> + uiout->text (_("Allocation tag unavailable"));
> >> + else
> >> + {
> >> + uiout->text (_("Allocation tag "));
> >> + uiout->field_core_addr ("allocation-tag", gdbarch, atag);
> >> + }
> >> + }
> >> + else
> >> + {
> >> + uiout->text ("\n");
> >> + uiout->text (_("Fault address unavailable"));
> >> + }
> >> +}
> >> +
> >> static void
> >> aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> >> {
> >> @@ -1706,6 +1767,9 @@ aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> >>
> >> /* Register a hook for converting a memory tag to a string. */
> >> set_gdbarch_memtag_to_string (gdbarch, aarch64_linux_memtag_to_string);
> >> +
> >> + set_gdbarch_report_signal_info (gdbarch,
> >> + aarch64_linux_report_signal_info);
> >> }
> >>
> >> /* Initialize the aarch64_linux_record_tdep. */
> >> diff --git a/gdb/arch/aarch64-mte-linux.h b/gdb/arch/aarch64-mte-linux.h
> >> index 5c5783f28b..a5a980ed49 100644
> >> --- a/gdb/arch/aarch64-mte-linux.h
> >> +++ b/gdb/arch/aarch64-mte-linux.h
> >> @@ -35,6 +35,12 @@
> >> #define MTE_LOGICAL_TAG_START_BIT 56
> >> #define MTE_LOGICAL_MAX_VALUE 0xf
> >>
> >> +/* Memory tagging definitions. */
> >> +#ifndef SEGV_MTEAERR
> >> +# define SEGV_MTEAERR 8
> >> +# define SEGV_MTESERR 9
> >> +#endif
> >> +
> >> /* Return the number of tag granules in the memory range
> >> [ADDR, ADDR + LEN) given GRANULE_SIZE. */
> >> extern size_t get_tag_granules (CORE_ADDR addr, size_t len,
> >> --
> >> 2.17.1
> >>