From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id HCnWB1P06V//RQAAWB0awg (envelope-from ) for ; Mon, 28 Dec 2020 10:05:55 -0500 Received: by simark.ca (Postfix, from userid 112) id 0DF5A1F0AA; Mon, 28 Dec 2020 10:05:55 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id C97A51E552 for ; Mon, 28 Dec 2020 10:05:53 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 50465385800C; Mon, 28 Dec 2020 15:05:53 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 50465385800C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1609167953; bh=dRCGp+qmtnT1gI3kr5VQOfwA3Rf3v/eRZ5xNngM5i38=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=fkVaBB2Pjpwj/GIhbvD7ZsfBo5kBswW2DNLZXV3l36fPlYYGOCPkptyQomgpnNnQn CrNQcgBJUxihMSgYyLdBUU8p3dq2RlNNv4k1AInQt+Tud7xpYoQIyXA2GlZsMhbiAM dQUrNIsW6IasDVUINCqQkDEuHrrykr7Qi9olh1PQ= Received: from mail-qk1-x72f.google.com (mail-qk1-x72f.google.com [IPv6:2607:f8b0:4864:20::72f]) by sourceware.org (Postfix) with ESMTPS id CE5A7385800C for ; Mon, 28 Dec 2020 15:05:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org CE5A7385800C Received: by mail-qk1-x72f.google.com with SMTP id 19so8969241qkm.8 for ; Mon, 28 Dec 2020 07:05:49 -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:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=dRCGp+qmtnT1gI3kr5VQOfwA3Rf3v/eRZ5xNngM5i38=; b=tfXRd48vc9pDY1HycnrzchxZ4C50+pa7rtP1WPa/wm5elpOLa1ndvhprs1vPbcTM2t 1/JAdkTAo6cB4kLuGu192um+Hy8/1yWX4xgL6OPqcrhKXEudLibWI8wudCY2HM6BZRsb 2IHCPqlhXBU9wc03nnNlBDOnjFj6zfcVvsp7GdKVoKJwnwHJLrTzN76f4RXxGKz0kMpZ Y+CLEP61H8GUTqNANaTkSr2gPbRz7aDDa4BQ+3fxe4kJFFM4OQu9k4d8B3KnQNBs/HFM pMoAUMTcqNgpNg948nN+CeuBmKho1gkNyRkw86djyveLUZS5d01GRVKYpF7fWK9TxkV1 kOvw== X-Gm-Message-State: AOAM533vPdIlFXubAo9JrlWKQJh79jZ5s67+VPgmov0lv/6RKa+LCO0v BhcaHyl0atRowH4Z9pCtKhdNSQ== X-Google-Smtp-Source: ABdhPJwi0SXTSr8H6zjzcBhbaXVaEl7qKIMcoZWimInJg4VrTjdj8PfWlr/gJRg31WMK8MJog0MLug== X-Received: by 2002:a37:aa94:: with SMTP id t142mr46188793qke.116.1609167949249; Mon, 28 Dec 2020 07:05:49 -0800 (PST) Received: from ?IPv6:2804:7f0:8284:370e:19e7:527f:e109:2734? ([2804:7f0:8284:370e:19e7:527f:e109:2734]) by smtp.gmail.com with ESMTPSA id f24sm23640212qkk.89.2020.12.28.07.05.47 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 28 Dec 2020 07:05:48 -0800 (PST) Subject: Re: [PATCH v3 01/24] New target methods for memory tagging support To: Simon Marchi , gdb-patches@sourceware.org References: <20201109170435.15766-1-luis.machado@linaro.org> <20201109170435.15766-2-luis.machado@linaro.org> Message-ID: Date: Mon, 28 Dec 2020 12:05:45 -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: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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: , From: Luis Machado via Gdb-patches Reply-To: Luis Machado Cc: david.spickett@linaro.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" On 12/25/20 1:26 AM, Simon Marchi wrote: > On 2020-11-09 12:04 p.m., Luis Machado via Gdb-patches wrote: >> Updates on v3: >> >> - Updated the code documentation for the fetch_memtags and store_memtags >> methods. >> >> Updates on v2: >> >> - Added type parameter to fetch_memtags/store_memtags hooks. >> >> -- >> >> This patch starts adding some of the generic pieces to accomodate memory >> tagging. >> >> We have three new target methods: >> >> - supports_memory_tagging: Checks if the target supports memory tagging. This >> defaults to false for targets that don't support memory tagging. >> >> - fetch_memtags: Fetches the allocation tags associated with a particular >> memory range [address, address + length). >> >> The default is to return 0 without returning any tags. This should only >> be called if memory tagging is supported. >> >> - store_memtags: Stores a set of allocation tags for a particular memory >> range [address, address + length). >> >> The default is to return 0. This should only >> be called if memory tagging is supported. > > If fetch/store should only be called on targets for which supports_memory_tagging > returns true, why is the default to return 0? The text tries to make it more obvious that the architecture needs to support memory tagging before invoking these functions. It isn't the case for native targets, but we will always have target implementations for the remote targets. There is a corner case for extended-remote where we connect to the target, but don't have any inferiors running. That means we don't get qSupported features and can't really be sure if the target supports memory tagging or not. I think it is unlikely someone will invoke these functions incorrectly though. > > It seems to me like it should be either: > > - it's ok to call fetch/store on any target, if the target doesn't implement memtags, > the default of returning 0 is used > - it's only ok to call fetch/store on targets that implement memtags, in which case > I would expect the default to maybe be tcomplain or even an assert > > Or is there something I don't see? > No. That makes sense. I've changed this to a tcomplain for both implementations. >> >> It also adds a control option for enabling/disabling memory tagging >> manually: set memory-tagging on/off. >> >> The default is "on", with GDB making its use conditional to the >> architecture supporting memory tagging. >> >> gdb/ChangeLog: >> >> YYYY-MM-DD Luis Machado >> >> * printcmd.c (memtag): New static global. >> (show_memtag): New function. >> (_initialize_printcmd): Add set/show memory-tagging command. >> * remote.c (remote_target) : New method >> override. >> : New method override. >> : New method override. >> (remote_target::supports_memory_tagging): New method. >> (remote_target::fetch_memtags): New method. >> (remote_target::store_memtags): New method. >> * target-delegates.c >> (dummy_target) : New method override. >> : New method override. >> : New method override. >> (debug_target) : New method override. >> : New method override. >> : New method override. >> (target_ops::supports_memory_tagging): New method. >> (target_ops::fetch_memtags): New method. >> (target_ops::store_memtags): New method. >> (dummy_target::supports_memory_tagging): New method. >> (dummy_target::fetch_memtags): New method. >> (dummy_target::store_memtags): New method. >> (debug_target::supports_memory_tagging): New method. >> (debug_target::fetch_memtags): New method. >> (debug_target::store_memtags): New method. > > Oh, you wrote this by hand :(. target-delegates.c is generated (see the > command at the top of the file). Yikes. I see that now. Glad to not have to write one more piece of ChangeLog! :-) > > Otherwise, that LGTM. > > Simon > About returning bool... I recall we touched this subject before. The integer return was supposed to cover cases where we wanted to return error codes. But, given we don't have any clearly defined error codes at the moment, I'll stick with bool for success/failure.