From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id eJwdGc2LEmDraQAAWB0awg (envelope-from ) for ; Thu, 28 Jan 2021 05:02:53 -0500 Received: by simark.ca (Postfix, from userid 112) id 5FEB41EF80; Thu, 28 Jan 2021 05:02:53 -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 0444D1E945 for ; Thu, 28 Jan 2021 05:02:52 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id AD5683857025; Thu, 28 Jan 2021 10:02:51 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AD5683857025 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1611828171; bh=w/xb9tASmIdDmr666qN0v5Eny7FM0yvUzuQibOlXmkc=; 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=tJGWjYr7rxFSvxVb6/6xS5NeqnqNetil2dtksdgcxDJMcgitxv41uDsfX8jq3hr/w LTPpCNnS1zWDOi/WjfZF8tHBly9DslscffV6nOE4c7xaeBbqTl/Rr/3m9TXm12XC4G QDaWtkbW0t8+pUmMeFE+5hAsZ5J4dyqNyM37lJcc= Received: from mail-qk1-x729.google.com (mail-qk1-x729.google.com [IPv6:2607:f8b0:4864:20::729]) by sourceware.org (Postfix) with ESMTPS id 95E863857025 for ; Thu, 28 Jan 2021 10:02:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 95E863857025 Received: by mail-qk1-x729.google.com with SMTP id n15so4671938qkh.8 for ; Thu, 28 Jan 2021 02:02:48 -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=w/xb9tASmIdDmr666qN0v5Eny7FM0yvUzuQibOlXmkc=; b=ZxfyYvjcs5Y+B9cvLynzl1sxdZEHnLbsPyf7vNeNGF+RNjJGgIVKkkj8w3xCLE7t8Z VGlh3lgfLSm0pEFr+Ywp39hoiavJUeIcA5pUFyjpXxLUZ+r7UgMvtafDj0hr0nAlbvb3 R4O+11wKJUW7coKp/Qvpvv+JMn1PGW4QGZEmlhzxoQjAHtxyD2/5odXYUgbh49BjjxvQ oHXsLMrKnrBEViHpg2GbGZG4LeGb1ebM+4yZ7Tlwu8fNWT6xGdtD+SPNazBnxthpyrTK wSFOTZHv9FS5fMESvyhED8d3kbaUowSpKrZ0t1tnTc7zKb5DZNYkA4wjQSf0v5D/h5dL ONqA== X-Gm-Message-State: AOAM532pvw3KhFwxOW/77/yakE9/4/+CiQb8/Gbmt0oixJAqZRr6dprA +XTaa0vO+eeGlYChdI2fTYpm9ZY6zyciVA== X-Google-Smtp-Source: ABdhPJxFXHdtX722FfaNmxlSPh80tfkfK6Stl+WF/KJmwrCoSBoChg5nOjjSbV5atVVIVQqaPdXdMQ== X-Received: by 2002:a05:620a:5aa:: with SMTP id q10mr14041374qkq.103.1611828167995; Thu, 28 Jan 2021 02:02:47 -0800 (PST) Received: from ?IPv6:2804:7f0:8284:874d:b82c:87fc:4324:adab? ([2804:7f0:8284:874d:b82c:87fc:4324:adab]) by smtp.gmail.com with ESMTPSA id p26sm3127264qkk.6.2021.01.28.02.02.46 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 28 Jan 2021 02:02:47 -0800 (PST) Subject: Re: [PATCH v5 01/25] New target methods for memory tagging support To: Lancelot SIX References: <20210127202112.2485702-1-luis.machado@linaro.org> <20210127202112.2485702-2-luis.machado@linaro.org> Message-ID: <89b73bed-694b-f61c-b377-a2dd0c944984@linaro.org> Date: Thu, 28 Jan 2021 07:02: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: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" On 1/27/21 8:26 PM, Lancelot SIX wrote: > On Wed, Jan 27, 2021 at 05:20:48PM -0300, Luis Machado via Gdb-patches wrote: >> Updates on v5: >> >> - Remove the memory_tagging global (in favor of setting a specific print >> option) and related functions. >> >> Updates on v4: >> >> - Updated the return types of fetch/store member functions from int to bool. >> - Implemented target-debug type print helpers. >> - Renamed global memtag to memory_tagging. >> >> 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. >> >> gdb/ChangeLog: >> >> YYYY-MM-DD Luis Machado >> >> * 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: Regenerate. >> * target.h (struct target_ops) : New virtual >> method. >> : New virtual method. >> : New virtual method. >> (target_supports_memory_tagging): Define. >> (target_fetch_memtags): Define. >> (target_store_memtags): Define. >> * target-debug.h (target_debug_print_size_t) >> (target_debug_print_const_gdb_byte_vector_r) >> (target_debug_print_gdb_byte_vector_r): New functions. >> --- >> gdb/remote.c | 34 +++++++++++++++ >> gdb/target-debug.h | 24 +++++++++++ >> gdb/target-delegates.c | 95 ++++++++++++++++++++++++++++++++++++++++++ >> gdb/target.h | 41 ++++++++++++++++++ >> 4 files changed, 194 insertions(+) >> >> diff --git a/gdb/remote.c b/gdb/remote.c >> index bc995edc53..b130f1ddae 100644 >> --- a/gdb/remote.c >> +++ b/gdb/remote.c >> @@ -690,6 +690,14 @@ class remote_target : public process_stratum_target >> int remove_exec_catchpoint (int) override; >> enum exec_direction_kind execution_direction () override; >> >> + bool supports_memory_tagging () override; >> + >> + bool fetch_memtags (CORE_ADDR address, size_t len, >> + gdb::byte_vector &tags, int type) override; >> + >> + bool store_memtags (CORE_ADDR address, size_t len, >> + const gdb::byte_vector &tags, int type) override; >> + >> public: /* Remote specific methods. */ >> >> void remote_download_command_source (int num, ULONGEST addr, >> @@ -14472,6 +14480,32 @@ show_remote_timeout (struct ui_file *file, int from_tty, >> value); >> } >> >> +/* Implement the "supports_memory_tagging" target_ops method. */ >> + >> +bool >> +remote_target::supports_memory_tagging () >> +{ >> + return false; >> +} >> + >> +/* Implement the "fetch_memtags" target_ops method. */ >> + >> +bool >> +remote_target::fetch_memtags (CORE_ADDR address, size_t len, >> + gdb::byte_vector &tags, int type) >> +{ >> + return 0; > > If you return a bool, you should probably return false. > >> +} >> + >> +/* Implement the "store_memtags" target_ops method. */ >> + >> +bool >> +remote_target::store_memtags (CORE_ADDR address, size_t len, >> + const gdb::byte_vector &tags, int type) >> +{ >> + return 0; > > Similarly, return false. > Missed it, since this gets replaced by the full implementation later. I'll address it locally. Thanks for spotting that. >> +} >> + >> void _initialize_remote (); >> void >> _initialize_remote () >> diff --git a/gdb/target-debug.h b/gdb/target-debug.h >> index 6910338865..5bc384a39a 100644 >> --- a/gdb/target-debug.h >> +++ b/gdb/target-debug.h >> @@ -212,4 +212,28 @@ target_debug_print_signals (gdb::array_view sigs) >> fputs_unfiltered (" }", gdb_stdlog); >> } >> >> +static void >> +target_debug_print_size_t (size_t size) >> +{ >> + fprintf_unfiltered (gdb_stdlog, "%s", pulongest (size)); >> +} >> + >> +static void >> +target_debug_print_const_gdb_byte_vector_r (const gdb::byte_vector &vector) >> +{ >> + fputs_unfiltered ("{", gdb_stdlog); >> + >> + for (size_t i = 0; i < vector.size (); i++) >> + { >> + fprintf_unfiltered (gdb_stdlog, " %s", >> + phex_nz (vector[i], 1)); >> + } >> + fputs_unfiltered (" }", gdb_stdlog); >> +} >> + >> +static void >> +target_debug_print_gdb_byte_vector_r (gdb::byte_vector &vector) >> +{ >> + target_debug_print_gdb_byte_vector_r (vector); > > I think you meant target_debug_print_const_gdb_byte_vector_r here. > That's correct. I'll fix this locally. Thanks! > Lancelot. > >> +} >> #endif /* TARGET_DEBUG_H */ >> diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c >> index 437b19b858..e4563e4dac 100644 >> --- a/gdb/target-delegates.c >> +++ b/gdb/target-delegates.c >> @@ -173,6 +173,9 @@ struct dummy_target : public target_ops >> const struct frame_unwind *get_tailcall_unwinder () override; >> void prepare_to_generate_core () override; >> void done_generating_core () override; >> + bool supports_memory_tagging () override; >> + bool fetch_memtags (CORE_ADDR arg0, size_t arg1, gdb::byte_vector &arg2, int arg3) override; >> + bool store_memtags (CORE_ADDR arg0, size_t arg1, const gdb::byte_vector &arg2, int arg3) override; >> }; >> >> struct debug_target : public target_ops >> @@ -344,6 +347,9 @@ struct debug_target : public target_ops >> const struct frame_unwind *get_tailcall_unwinder () override; >> void prepare_to_generate_core () override; >> void done_generating_core () override; >> + bool supports_memory_tagging () override; >> + bool fetch_memtags (CORE_ADDR arg0, size_t arg1, gdb::byte_vector &arg2, int arg3) override; >> + bool store_memtags (CORE_ADDR arg0, size_t arg1, const gdb::byte_vector &arg2, int arg3) override; >> }; >> >> void >> @@ -4413,3 +4419,92 @@ debug_target::done_generating_core () >> fputs_unfiltered (")\n", gdb_stdlog); >> } >> >> +bool >> +target_ops::supports_memory_tagging () >> +{ >> + return this->beneath ()->supports_memory_tagging (); >> +} >> + >> +bool >> +dummy_target::supports_memory_tagging () >> +{ >> + return false; >> +} >> + >> +bool >> +debug_target::supports_memory_tagging () >> +{ >> + bool result; >> + fprintf_unfiltered (gdb_stdlog, "-> %s->supports_memory_tagging (...)\n", this->beneath ()->shortname ()); >> + result = this->beneath ()->supports_memory_tagging (); >> + fprintf_unfiltered (gdb_stdlog, "<- %s->supports_memory_tagging (", this->beneath ()->shortname ()); >> + fputs_unfiltered (") = ", gdb_stdlog); >> + target_debug_print_bool (result); >> + fputs_unfiltered ("\n", gdb_stdlog); >> + return result; >> +} >> + >> +bool >> +target_ops::fetch_memtags (CORE_ADDR arg0, size_t arg1, gdb::byte_vector &arg2, int arg3) >> +{ >> + return this->beneath ()->fetch_memtags (arg0, arg1, arg2, arg3); >> +} >> + >> +bool >> +dummy_target::fetch_memtags (CORE_ADDR arg0, size_t arg1, gdb::byte_vector &arg2, int arg3) >> +{ >> + tcomplain (); >> +} >> + >> +bool >> +debug_target::fetch_memtags (CORE_ADDR arg0, size_t arg1, gdb::byte_vector &arg2, int arg3) >> +{ >> + bool result; >> + fprintf_unfiltered (gdb_stdlog, "-> %s->fetch_memtags (...)\n", this->beneath ()->shortname ()); >> + result = this->beneath ()->fetch_memtags (arg0, arg1, arg2, arg3); >> + fprintf_unfiltered (gdb_stdlog, "<- %s->fetch_memtags (", this->beneath ()->shortname ()); >> + target_debug_print_CORE_ADDR (arg0); >> + fputs_unfiltered (", ", gdb_stdlog); >> + target_debug_print_size_t (arg1); >> + fputs_unfiltered (", ", gdb_stdlog); >> + target_debug_print_gdb_byte_vector_r (arg2); >> + fputs_unfiltered (", ", gdb_stdlog); >> + target_debug_print_int (arg3); >> + fputs_unfiltered (") = ", gdb_stdlog); >> + target_debug_print_bool (result); >> + fputs_unfiltered ("\n", gdb_stdlog); >> + return result; >> +} >> + >> +bool >> +target_ops::store_memtags (CORE_ADDR arg0, size_t arg1, const gdb::byte_vector &arg2, int arg3) >> +{ >> + return this->beneath ()->store_memtags (arg0, arg1, arg2, arg3); >> +} >> + >> +bool >> +dummy_target::store_memtags (CORE_ADDR arg0, size_t arg1, const gdb::byte_vector &arg2, int arg3) >> +{ >> + tcomplain (); >> +} >> + >> +bool >> +debug_target::store_memtags (CORE_ADDR arg0, size_t arg1, const gdb::byte_vector &arg2, int arg3) >> +{ >> + bool result; >> + fprintf_unfiltered (gdb_stdlog, "-> %s->store_memtags (...)\n", this->beneath ()->shortname ()); >> + result = this->beneath ()->store_memtags (arg0, arg1, arg2, arg3); >> + fprintf_unfiltered (gdb_stdlog, "<- %s->store_memtags (", this->beneath ()->shortname ()); >> + target_debug_print_CORE_ADDR (arg0); >> + fputs_unfiltered (", ", gdb_stdlog); >> + target_debug_print_size_t (arg1); >> + fputs_unfiltered (", ", gdb_stdlog); >> + target_debug_print_const_gdb_byte_vector_r (arg2); >> + fputs_unfiltered (", ", gdb_stdlog); >> + target_debug_print_int (arg3); >> + fputs_unfiltered (") = ", gdb_stdlog); >> + target_debug_print_bool (result); >> + fputs_unfiltered ("\n", gdb_stdlog); >> + return result; >> +} >> + >> diff --git a/gdb/target.h b/gdb/target.h >> index 917476d16a..19395f1258 100644 >> --- a/gdb/target.h >> +++ b/gdb/target.h >> @@ -1260,6 +1260,38 @@ struct target_ops >> /* Cleanup after generating a core file. */ >> virtual void done_generating_core () >> TARGET_DEFAULT_IGNORE (); >> + >> + /* Returns true if the target supports memory tagging, false otherwise. */ >> + virtual bool supports_memory_tagging () >> + TARGET_DEFAULT_RETURN (false); >> + >> + /* Return the allocated memory tags of type TYPE associated with >> + [ADDRESS, ADDRESS + LEN) in TAGS. >> + >> + LEN is the number of bytes in the memory range. TAGS is a vector of >> + bytes containing the tags found in the above memory range. >> + >> + It is up to the architecture/target to interpret the bytes in the TAGS >> + vector and read the tags appropriately. >> + >> + Returns true if fetching the tags succeeded and false otherwise. */ >> + virtual bool fetch_memtags (CORE_ADDR address, size_t len, >> + gdb::byte_vector &tags, int type) >> + TARGET_DEFAULT_NORETURN (tcomplain ()); >> + >> + /* Write the allocation tags of type TYPE contained in TAGS to the memory >> + range [ADDRESS, ADDRESS + LEN). >> + >> + LEN is the number of bytes in the memory range. TAGS is a vector of >> + bytes containing the tags to be stored to the memory range. >> + >> + It is up to the architecture/target to interpret the bytes in the TAGS >> + vector and store them appropriately. >> + >> + Returns true if storing the tags succeeded and false otherwise. */ >> + virtual bool store_memtags (CORE_ADDR address, size_t len, >> + const gdb::byte_vector &tags, int type) >> + TARGET_DEFAULT_NORETURN (tcomplain ()); >> }; >> >> /* Deleter for std::unique_ptr. See comments in >> @@ -2312,6 +2344,15 @@ extern gdb::unique_xmalloc_ptr target_fileio_read_stralloc >> #define target_augmented_libraries_svr4_read() \ >> (current_top_target ()->augmented_libraries_svr4_read) () >> >> +#define target_supports_memory_tagging() \ >> + ((current_top_target ()->supports_memory_tagging) ()) >> + >> +#define target_fetch_memtags(address, len, tags, type) \ >> + (current_top_target ()->fetch_memtags) ((address), (len), (tags), (type)) >> + >> +#define target_store_memtags(address, len, tags, type) \ >> + (current_top_target ()->store_memtags) ((address), (len), (tags), (type)) >> + >> /* Command logging facility. */ >> >> #define target_log_command(p) \ >> -- >> 2.25.1 >>