From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x72b.google.com (mail-qk1-x72b.google.com [IPv6:2607:f8b0:4864:20::72b]) by sourceware.org (Postfix) with ESMTPS id 20D5A3857817 for ; Fri, 5 Feb 2021 14:59:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 20D5A3857817 Received: by mail-qk1-x72b.google.com with SMTP id n15so7092909qkh.8 for ; Fri, 05 Feb 2021 06:59:52 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=oo1ZUpsw7AjS3OsC4j09CceMt8ZRUXLVC+5rNkstsxo=; b=SPNR3Xg5NJuBMQS5cFnI4unokyPnxgRUx4dSZG9W3TghvbTUxeolRLX6hUcfPLQobQ +BQEnlWj7vBjCRWDdMx7dus5+L7u4QQfuOeCYg92eMaZquOM90FZCCso6cUlnqyHh/fJ AlbFF0/lrNC1hR957INY7IGEYT2oNLLpkdtXMTIsgqGHLzWVWgKTLysqreafhF3NTh5X ecXhytg8RH7adzEhbEN8nD9W8vj0CVqOnzS99/GK3ESytlIuofhh2+lMcVYowN6bOVuS 3E0vWEySP7vXBeqJwMQ9a7zvPdL30b4nbl0okjNtjIB5y3eTP/fLTy9GK+ILo+frCQ/G H6ig== X-Gm-Message-State: AOAM5331WiY9VSj1JsRznf2/tBAMstoYcGyXGB2yJ6YHfh4U3kPgmpmS hnyNbUEWFp+Ezq2UVPj+A1kQga6efwczmQ== X-Google-Smtp-Source: ABdhPJzN5qsY2IKsl4Jj/9kUkDvSxJ98Cz5A9SLoBLrKYjkMa+qKj04sx9iFXYv7oyLZwIuut4mcJQ== X-Received: by 2002:ae9:ef0f:: with SMTP id d15mr4526808qkg.292.1612537191572; Fri, 05 Feb 2021 06:59:51 -0800 (PST) Received: from ?IPv6:2804:7f0:8284:848d:5d42:306c:a572:c6bb? ([2804:7f0:8284:848d:5d42:306c:a572:c6bb]) by smtp.gmail.com with ESMTPSA id v135sm9921983qka.98.2021.02.05.06.59.50 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 05 Feb 2021 06:59:51 -0800 (PST) Subject: Re: [PATCH v5 17/25] AArch64: Report tag violation error information To: Simon Marchi , gdb-patches@sourceware.org References: <20210127202112.2485702-1-luis.machado@linaro.org> <20210127202112.2485702-18-luis.machado@linaro.org> <1bd853e9-00ce-9856-7bd6-f5ab0af88475@polymtl.ca> From: Luis Machado Message-ID: <8cd308f0-8a31-43d5-d540-e5315a25f41b@linaro.org> Date: Fri, 5 Feb 2021 11:59:48 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <1bd853e9-00ce-9856-7bd6-f5ab0af88475@polymtl.ca> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 05 Feb 2021 14:59:53 -0000 On 2/5/21 1:22 AM, Simon Marchi wrote: >> 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. > Yeah. I raised this last year with the kernel folks. It was a possibility back then, but not the way it was designed at first. I checked again and it isn't clear if this is exposed in a convenient way, as it was enabled via a sigaction argument. GDB may need special handling to filter out the top byte from siginfo's si_addr. I'll check with the latest RC. >> + >> + gdb::optional 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)? Right. This is being exposed so MI can potentially use the information (as is done for i386-linux). Though I wouldn't say it is guaranteed to be kept the same. Honestly, the goal is not to enable an MI MTE API right now. When the functionality is pushed and working well, then we can focus on making the MI layer MTE-aware. The new commands, for example, don't have MI counterparts at this point. Given only a handful of architectures are using tagged memory, I'm not even sure if it is worth coming up with an MI API yet. > > 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. I have no problem documenting the current ones. > > But otherwise, that LGTM. Thanks!