From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id gR8mKKqH9V9hVwAAWB0awg (envelope-from ) for ; Wed, 06 Jan 2021 04:49:30 -0500 Received: by simark.ca (Postfix, from userid 112) id 94E891F0AA; Wed, 6 Jan 2021 04:49:30 -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 A21D01E590 for ; Wed, 6 Jan 2021 04:49:28 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 0231038708A6; Wed, 6 Jan 2021 09:49:28 +0000 (GMT) Received: from mail-wr1-x42b.google.com (mail-wr1-x42b.google.com [IPv6:2a00:1450:4864:20::42b]) by sourceware.org (Postfix) with ESMTPS id 11E25385702C for ; Wed, 6 Jan 2021 09:49:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 11E25385702C Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wr1-x42b.google.com with SMTP id d26so1793841wrb.12 for ; Wed, 06 Jan 2021 01:49:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=CTyb0pjhvLo3l+NOF4OXH5NgUWxq4mvr95Jz8t7Gsgk=; b=XViymO8Lapa5wkyKNvdvrQ+mqBwtgXSWhF71vwjL17ht+lToKCaGFpgUAp9vmlKrNc B4SFuWAM1aGKVQSkLafGkznWFQEIw1+IR5s+NCnx8EtJUNHiLHzNZVjYnT4KI0iIbOdX QsWgoVjwPEVM6rSur5EfKFvI7JMn4/dNwwdolPqN0oZvjvS+x2+wj3sA7IxonLqFnMx6 j5D8MOmI2TEMI+qX4Az0vxxT9mGaifL8CB55/wAa+Du8iGwHONUzhVP8X+txltQ75E7B UADPLaERvbtWZgiDdFFLpItYvZhXDHzvYrM4QfsEJJpKvZ9VHZPl2MGGsuhoGqCoBFpj fFKw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=CTyb0pjhvLo3l+NOF4OXH5NgUWxq4mvr95Jz8t7Gsgk=; b=IsRTGO+3DFaH2NG/enMjsEGA+Z3K9Lq6Jw+aJroV9tcIhlirsjcrsRkbspf18+Mk4l 77bqPDaCYhhiAL8wMpS919X5pWRBhhgkXL0D2ufO5+u0HSSgI4By4pk3AsoBIqQ5KLuc I3vIlspdl9+Hs8bWl+yC4l7j+7RYP2BzOn6Q6WP9BnIq2sfr1Iu0m+28cryWBdDkj6en CLqe50tLGlO9Pt1HtNstVVZKOuFAPjX/rtdoBIvOM95RLJuWgrHZoEYF+NEg0s5XR80n gXrL/TE4sZULWo06tzf+nFu98Api5ZGZCwno/+kQOAOwIJO0qdX+qKLl1vkpvjJCG3gm oIVQ== X-Gm-Message-State: AOAM533LlJZt4DvYXZ+xMXeBFjuwOrih5yYudRNujIzhgTq/mGDFbbOw d8w76oieWu1QHJFCQwE+3PhUXzXawwNxow== X-Google-Smtp-Source: ABdhPJxjWX0+vwdS5G7JLLiG6g5x9tp8Nsg55VFKX6mbNrk3a3JmCiqqeioiXaW1MiSbM9vZyNyYwA== X-Received: by 2002:adf:ba47:: with SMTP id t7mr3391586wrg.285.1609926562136; Wed, 06 Jan 2021 01:49:22 -0800 (PST) Received: from localhost (host86-166-129-230.range86-166.btcentralplus.com. [86.166.129.230]) by smtp.gmail.com with ESMTPSA id u13sm2292866wrw.11.2021.01.06.01.49.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Jan 2021 01:49:21 -0800 (PST) Date: Wed, 6 Jan 2021 09:49:20 +0000 From: Andrew Burgess To: Mike Frysinger Subject: Re: [PATCH] gdb/sim: add support for exporting memory map Message-ID: <20210106094920.GF2945@embecosm.com> References: <20210106060433.12043-1-vapier@gentoo.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210106060433.12043-1-vapier@gentoo.org> X-Operating-System: Linux/5.8.13-100.fc31.x86_64 (x86_64) X-Uptime: 09:41:05 up 28 days, 14:25, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] 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: , Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" * Mike Frysinger via Gdb-patches [2021-01-06 01:04:33 -0500]: > This allows gdb to quickly dump & process the memory map that the sim > knows about. This isn't fully accurate, but is largely limited by the > gdb memory map format. While the sim supports RWX bits, gdb can only > handle RW or RO regions. > --- > gdb/remote-sim.c | 16 +++++++++++ > include/gdb/remote-sim.h | 9 +++++++ > sim/common/sim-core.c | 58 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 83 insertions(+) > > gdb/: > > * remote-sim.c: Include memory-map.h. > (gdbsim_target): Define memory_map override. > (gdbsim_target::memory_map): Define. > > include/gdb/: > > * remote-sim.h (sim_memory_map): Define. > > sim/common/: > > * sim-core.c (sim_memory_map): Define. > > diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c > index c4f3913edbad..e44add538afb 100644 > --- a/gdb/remote-sim.c > +++ b/gdb/remote-sim.c > @@ -42,6 +42,7 @@ > #include "readline/readline.h" > #include "gdbthread.h" > #include "gdbsupport/byte-vector.h" > +#include "memory-map.h" > > /* Prototypes */ > > @@ -164,6 +165,7 @@ struct gdbsim_target final > > bool has_all_memory () override; > bool has_memory () override; > + std::vector memory_map () override; > > private: > sim_inferior_data *get_inferior_data_by_ptid (ptid_t ptid, > @@ -1270,6 +1272,20 @@ gdbsim_target::has_memory () > return true; > } > > +std::vector > +gdbsim_target::memory_map () This should have a header comment, even if its just: /* Implement target_ops::memory_map. */ > +{ > + struct sim_inferior_data *sim_data > + = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NEEDED); > + std::vector result; > + gdb::unique_xmalloc_ptr text(sim_memory_map (sim_data->gdbsim_desc)); Whitespace missing after 'text'. > + > + if (text) GDB prefers 'if (text != nullptr)'. > + result = parse_memory_map (text.get ()); > + > + return result; > +} > + > void _initialize_remote_sim (); > void > _initialize_remote_sim () > diff --git a/include/gdb/remote-sim.h b/include/gdb/remote-sim.h > index 73fb670c17e5..a3ba3aa36cd2 100644 > --- a/include/gdb/remote-sim.h > +++ b/include/gdb/remote-sim.h > @@ -213,6 +213,15 @@ int sim_store_register (SIM_DESC sd, int regno, unsigned char *buf, int length); > void sim_info (SIM_DESC sd, int verbose); > > > +/* Return a memory map in XML format. > + > + The caller must free the returned string. > + > + For details on the format, see GDB's Memory Map Format documentation. */ > + > +char *sim_memory_map (SIM_DESC sd); > + > + > /* Run (or resume) the simulated program. > > STEP, when non-zero indicates that only a single simulator cycle > diff --git a/sim/common/sim-core.c b/sim/common/sim-core.c > index 74369aa99fe2..eb40d2588f58 100644 > --- a/sim/common/sim-core.c > +++ b/sim/common/sim-core.c > @@ -452,6 +452,64 @@ sim_core_translate (sim_core_mapping *mapping, > } > > > +#if EXTERN_SIM_CORE_P > +char * > +sim_memory_map (SIM_DESC sd) > +{ I would like to start following the GDB convention of requiring a header comment on all functions, so here we'd just say: /* See include/gdb/remote-sim.h. */ > + sim_core *core = STATE_CORE (sd); > + unsigned map; > + char *s1, *s2, *entry; > + > + s1 = xstrdup ( > + "\n" > + " + " 'http://sourceware.org/gdb/gdb-memory-map.dtd'>\n" > + "\n"); > + > + for (map = 0; > + map < nr_maps; > + map++) > + { > + sim_core_mapping *mapping = core->common.map[map].first; > + > + while (mapping != NULL) Instead of using while here and then 'goto next' inside, could we not say: for (mapping = core->common.map[map].first; mapping != NULL; mapping = mapping->next) and then replace the 'goto next' with 'continue' ? > + { > + if (mapping->level != 0) > + goto next; > + > + entry = xasprintf ("\n", > + mapping->base, mapping->nr_bytes); > + /* The sim memory map is organized by access, not by addresses. > + So a RWX memory map will have three independent mappings. > + GDB's format cannot support overlapping regions, so we have > + to filter those out. > + > + Further, GDB can only handle RX ("rom") or RWX ("ram") mappings. > + We just emit "ram" everywhere to keep it simple. If GDB ever > + gains support for more stuff, we can expand this. > + > + Using strstr is kind of hacky, but as long as the map is not huge > + (we're talking <10K), should be fine. */ > + if (strstr (s1, entry) == NULL) > + { > + s2 = concat (s1, entry, NULL); > + free (s1); > + s1 = s2; > + } > + free (entry); > + > + next: > + mapping = mapping->next; > + } > + } > + > + s2 = concat (s1, "", NULL); > + free (s1); > + return s2; > +} > +#endif > + > + > #if EXTERN_SIM_CORE_P > unsigned > sim_core_read_buffer (SIM_DESC sd, > -- > 2.28.0 >