From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.120]) by sourceware.org (Postfix) with ESMTP id 885CF3851C25 for ; Wed, 20 May 2020 16:33:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 885CF3851C25 Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-193-u016mxebOwK9NVpEaMqbJQ-1; Wed, 20 May 2020 12:33:15 -0400 X-MC-Unique: u016mxebOwK9NVpEaMqbJQ-1 Received: by mail-wr1-f71.google.com with SMTP id 90so1608263wrg.23 for ; Wed, 20 May 2020 09:33:15 -0700 (PDT) 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=W+Biqi0BNdVBktTZ3QA4mepn+JCwiBzuqiBTrjaM9ds=; b=WCI145iZDI4c0DLegrkN0SQXMoJ2t/M5xLXoHtXAnMPBwjg9bCMPraLR76j6/n5cAt PctlLR4beV1quxaN1ZA3eN8vg8zxuNV5nw94iSDSHraZRquYix5kqxkZU5PF66YBpABc 5+k7R+aoVfK4abqxmWzrEngjw6xnXTtBhMQrO/OydQc+T/kL6zk2hOkdOCf6jxcb9eEh pQoYhvd8PilitA6AxHS4njHP/wdNFz3I2l/6SDfdRSsb/85uc+booIFYGs1yvJ7aLVIV 1SIZgSy2VTopzTHWrXI2pl52kNOp/tjfe5fg1nCIcxnCEwp1u4ls0or3VrIF1uM1MzDj xanQ== X-Gm-Message-State: AOAM533Xb3tGOFCHJrWk/wa+saKgv1KVHABGtTHkaOfunObIpDnCNSfN t18vRH0LupALzL7ZDza2UfCktJF4vs7U8PBZXUDynOWw2FeDFnTFZmVuNGQoPxUHMdTl0WqWTcy UBIManJN86wvZgi+RbqB3hg== X-Received: by 2002:a1c:790b:: with SMTP id l11mr5105212wme.2.1589992392150; Wed, 20 May 2020 09:33:12 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwWknoDvYLIZkza4n+Ha3QYNb/JvsWpGCm0G+7RF4d671AY+QpPeGllFDo5TzbWAYTou+Snag== X-Received: by 2002:a1c:790b:: with SMTP id l11mr5105197wme.2.1589992391819; Wed, 20 May 2020 09:33:11 -0700 (PDT) Received: from ?IPv6:2001:8a0:f909:7b00:56ee:75ff:fe8d:232b? ([2001:8a0:f909:7b00:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id n65sm2742132wmb.48.2020.05.20.09.33.10 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 20 May 2020 09:33:11 -0700 (PDT) Subject: Re: [PATCH v2 3/5] section_table_xfer_memory: Replace section name with callback predicate To: Kevin Buettner , gdb-patches@sourceware.org References: <20200513171155.645761-1-kevinb@redhat.com> <20200513171155.645761-4-kevinb@redhat.com> From: Pedro Alves Message-ID: <550ef44f-7b41-d999-fe58-b1fbc73f4403@redhat.com> Date: Wed, 20 May 2020 17:33:10 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20200513171155.645761-4-kevinb@redhat.com> Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, 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: Wed, 20 May 2020 16:33:19 -0000 On 5/13/20 6:11 PM, Kevin Buettner via Gdb-patches wrote: > This patch is motivated by the need to be able to select sections > that section_table_xfer_memory_partial should consider for memory > transfers. I'll use this facility in the next patch in this series. > > section_table_xfer_memory_partial() can currently be passed a section > name which may be used to make name-based selections. This is similar > to what I want to do, except that I want to be able to consider > section flags instead of the name. > > I'm replacing the section name parameter with a predicate that, > when passed a pointer to a target_section struct, will return > true if that section should be further considered, or false which > indicates that it shouldn't. > > I've converted the one existing use where a non-NULL section > name is passed to section_table_xfer_memory_partial(). Instead > of passing the section name, it now looks like this: > > auto match_cb = [&] (const struct target_section *s) > { > return (strcmp (section_name, s->the_bfd_section->name) == 0); > }; > > return section_table_xfer_memory_partial (readbuf, writebuf, > memaddr, len, xfered_len, > table->sections, > table->sections_end, > match_cb); > > The other callers all passed NULL; they've been simplified somewhat > in that they no longer need to pass NULL. > > gdb/ChangeLog: > > * exec.h (section_table_xfer_memory): Revise declaration, > replacing section name parameter with an optional callback > predicate. > * exec.c (section_table_xfer_memory): Likewise. > * bfd-target.c, exec.c, target.c, corelow.c: Adjust all callers > of section_table_xfer_memory. > --- > gdb/bfd-target.c | 3 +-- > gdb/corelow.c | 3 +-- > gdb/exec.c | 8 ++++---- > gdb/exec.h | 13 ++++++++++--- > gdb/target.c | 11 ++++++++--- > 5 files changed, 24 insertions(+), 14 deletions(-) > > diff --git a/gdb/bfd-target.c b/gdb/bfd-target.c > index b75abd7fb0..3d266951c5 100644 > --- a/gdb/bfd-target.c > +++ b/gdb/bfd-target.c > @@ -77,8 +77,7 @@ target_bfd::xfer_partial (target_object object, > return section_table_xfer_memory_partial (readbuf, writebuf, > offset, len, xfered_len, > m_table.sections, > - m_table.sections_end, > - NULL); > + m_table.sections_end); > } > default: > return TARGET_XFER_E_IO; > diff --git a/gdb/corelow.c b/gdb/corelow.c > index b60010453d..e3bd0bc452 100644 > --- a/gdb/corelow.c > +++ b/gdb/corelow.c > @@ -617,8 +617,7 @@ core_target::xfer_partial (enum target_object object, const char *annex, > (readbuf, writebuf, > offset, len, xfered_len, > m_core_section_table.sections, > - m_core_section_table.sections_end, > - NULL)); > + m_core_section_table.sections_end)); > > case TARGET_OBJECT_AUXV: > if (readbuf) > diff --git a/gdb/exec.c b/gdb/exec.c > index a2added5e2..02dd37ed10 100644 > --- a/gdb/exec.c > +++ b/gdb/exec.c > @@ -908,7 +908,8 @@ section_table_xfer_memory_partial (gdb_byte *readbuf, const gdb_byte *writebuf, > ULONGEST *xfered_len, > struct target_section *sections, > struct target_section *sections_end, > - const char *section_name) > + gdb::function_view + (const struct target_section *)> match_cb) > { > int res; > struct target_section *p; > @@ -922,7 +923,7 @@ section_table_xfer_memory_partial (gdb_byte *readbuf, const gdb_byte *writebuf, > struct bfd_section *asect = p->the_bfd_section; > bfd *abfd = asect->owner; > > - if (section_name && strcmp (section_name, asect->name) != 0) > + if (!match_cb (p)) > continue; /* not the section we need. */ > if (memaddr >= p->addr) > { > @@ -995,8 +996,7 @@ exec_target::xfer_partial (enum target_object object, > return section_table_xfer_memory_partial (readbuf, writebuf, > offset, len, xfered_len, > table->sections, > - table->sections_end, > - NULL); > + table->sections_end); > else > return TARGET_XFER_E_IO; > } > diff --git a/gdb/exec.h b/gdb/exec.h > index 54e6ff4d9b..7057b1be5a 100644 > --- a/gdb/exec.h > +++ b/gdb/exec.h > @@ -65,8 +65,13 @@ extern enum target_xfer_status > Request to transfer up to LEN 8-bit bytes of the target sections > defined by SECTIONS and SECTIONS_END. The OFFSET specifies the > starting address. > - If SECTION_NAME is not NULL, only access sections with that same > - name. > + > + The MATCH_CB predicate is optional; when provided it will be called > + for each section under consideration. When MATCH_CB evaluates as > + true, the section remains under consideration; a false result > + removes it from consideration for performing the memory transfers > + noted above. See memory_xfer_partial_1() in target.c for an > + example. > > Return the number of bytes actually transfered, or zero when no > data is available for the requested range. > @@ -83,7 +88,9 @@ extern enum target_xfer_status > ULONGEST, ULONGEST, ULONGEST *, > struct target_section *, > struct target_section *, > - const char *); > + gdb::function_view + const struct target_section *)> match_cb > + = [] (auto *) { return true; } ); The '(' after bool should go on the next line. I think a default of nullptr, along with the obvious change in section_table_xfer_memory_partial is better than this default lambda. Like: gdb::function_view match_cb = nullptr); This allows writing code where you can explicitly write "no filter" just using the convenient nullptr, like: gdb::function_view match_cb = nullptr; if (whatever) match_cb = [] (....) { .......; }; section_table_xfer_memory_partial (...., match_cb); Plus, I suspect is generates less code, not sure the linker merges all the instances of the lambda generated at each call site that uses the default. > > /* Read from mappable read-only sections of BFD executable files. > Similar to exec_read_partial_read_only, but return > diff --git a/gdb/target.c b/gdb/target.c > index 6982a806e3..b83da21710 100644 > --- a/gdb/target.c > +++ b/gdb/target.c > @@ -1036,11 +1036,17 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object, > const char *section_name = section->the_bfd_section->name; > > memaddr = overlay_mapped_address (memaddr, section); > + > + auto match_cb = [&] (const struct target_section *s) [=] is pedantically cheaper, since you're just copying a pointer, compared to [&] resulting in a pointer (reference) to pointer. > + { > + return (strcmp (section_name, s->the_bfd_section->name) == 0); > + }; > + > return section_table_xfer_memory_partial (readbuf, writebuf, > memaddr, len, xfered_len, > table->sections, > table->sections_end, > - section_name); > + match_cb); > } > } > OK otherwise. Thanks, Pedro Alves