Re: [PATCH 2/3] Unit test name-component bounds searching directly

Simon Marchi
2017-11-20T03:16:33Z

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 <palves@redhat.com>
>
> * dwarf2read.c (mapped_index::name_components_casing): New field.
> (mapped_index) <build_name_components,
> 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_component> 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