From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 62829 invoked by alias); 20 Nov 2017 03:16:33 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 62341 invoked by uid 89); 20 Nov 2017 03:16:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.7 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KB_WAM_FROM_NAME_SINGLEWORD,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=sensitivity, criterion X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 20 Nov 2017 03:16:31 +0000 Received: from [10.0.0.11] (192-222-251-162.qc.cable.ebox.net [192.222.251.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 8E4911E4C4; Sun, 19 Nov 2017 22:16:29 -0500 (EST) Subject: Re: [PATCH 2/3] Unit test name-component bounds searching directly To: Pedro Alves , gdb-patches@sourceware.org References: <5d721d13-d886-0400-db6b-76485c545142@redhat.com> <1511138515-25996-2-git-send-email-palves@redhat.com> From: Simon Marchi Message-ID: <856c728e-65b0-28da-5f8d-98a2d38d85de@simark.ca> Date: Mon, 20 Nov 2017 03:16:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <1511138515-25996-2-git-send-email-palves@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-11/txt/msg00390.txt.bz2 On 2017-11-19 07:41 PM, Pedro Alves wrote: > This commit factors out the name-components-vector building and bounds > searching out of dw2_expand_symtabs_matching_symbol into separate > functions, and adds unit tests that: > > - expose both the latent bug mentioned in the previous commit, and > also, > > - for completeness exercise the 0xff character handling fixed in the > previous commit more directly. > > The actual fix for the now-exposed bug is left for the following > patch. > > gdb/ChangeLog: > yyyy-mm-dd Pedro Alves > > * dwarf2read.c (mapped_index::name_components_casing): New field. > (mapped_index) find_name_components_bounds): Declare new methods. > (mapped_index::find_name_components_bounds) > (mapped_index::build_name_components): New methods, factored out > from dw2_expand_symtabs_matching_symbol. > (check_find_bounds_finds, test_find_bounds): New. > (run_test): Rename to ... > (test_dw2_expand_symtabs_matching_symbol): ... this. > (run_test): Reimplement. > --- > gdb/dwarf2read.c | 287 +++++++++++++++++++++++++++++++++++++++---------------- > 1 file changed, 204 insertions(+), 83 deletions(-) > > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c > index b08e81c..53548ca 100644 > --- a/gdb/dwarf2read.c > +++ b/gdb/dwarf2read.c > @@ -255,11 +255,24 @@ struct mapped_index > /* The name_component table (a sorted vector). See name_component's > description above. */ > std::vector name_components; > + /* How NAME_COMPONENTS is sorted. */ > + enum case_sensitivity name_components_casing; Missing newline above? Why is it important to store this in the structure now, rather than It's not really related to this patch, but it made me think about this edge case. Given that we build the vector only once, isn't it a problem if the user switches case sensitivity during a debug session? For example, the vector is built with case sensitivity one: - func_A - func_B - func_a - func_b Then we look to complete "func_a": we'll get func_a. The user then switches "set case-sensitive off". If we look to complete "func_a" in that vector, we will search (with lower_bound) with a different sorting criterion that the one that was used for sorting, which I guess will give funny results. I haven't actually tried, I'm just speculating. If that's indeed a problem, keeping the case_sensitivity that was used to sort the vector could be useful, since if we detect that it changed, we can re-sort it. Otherwise, the patch LGTM. Simon