Re: FYI: fix latent bug in dw2_find_symbol_file

[Doug Evans <dje at google dot com>]

On Mon, Jun 11, 2012 at 11:42 AM, Tom Tromey <tromey@redhat.com> wrote:
> I'm checking this in.
>
> dw2_find_symbol_file has a latent bug. ?It assumes that the last file in
> file name table is the primary file name for the CU.
>
> However, there's no reason this must be true. ?The primary file name
> could appear anywhere.
>
> And, since this method returns just the file name, and not the directory
> name, there is no reason to even bother reading the line number program
> -- we can just use the CU DIE's DW_AT_name.
>
> I don't have a test case for this; but the situation came up with the
> dwz multifile code.
>
> Built and regtested on x86-64 Fedora 16.
>
> Tom
>
> 2012-06-11 ?Tom Tromey ?<tromey@redhat.com>
>
> ? ? ? ?* dwarf2read.c (dw2_get_primary_filename_reader): New function.
> ? ? ? ?(dw2_find_symbol_file): Use it.
>
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 589361e..7e25d08 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -2836,12 +2836,34 @@ dw2_expand_symtabs_with_filename (struct objfile *objfile,
> ? ? }
> ?}
>
> +/* A helper function for dw2_find_symbol_file that finds the primary
> + ? file name for a given CU. ?This is a die_reader_func. ?*/
> +
> +static void
> +dw2_get_primary_filename_reader (const struct die_reader_specs *reader,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?gdb_byte *info_ptr,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct die_info *comp_unit_die,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int has_children,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?void *data)
> +{
> + ?const char **result_ptr = data;
> + ?struct dwarf2_cu *cu = reader->cu;
> + ?struct attribute *attr;
> +
> + ?attr = dwarf2_attr (comp_unit_die, DW_AT_name, cu);
> + ?if (attr == NULL)
> + ? ?*result_ptr = NULL;
> + ?else
> + ? ?*result_ptr = DW_STRING (attr);
> +}
> +
> ?static const char *
> ?dw2_find_symbol_file (struct objfile *objfile, const char *name)
> ?{
> ? struct dwarf2_per_cu_data *per_cu;
> ? offset_type *vec;
> ? struct quick_file_names *file_data;
> + ?const char *filename;
>
> ? dw2_setup (objfile);
>
> @@ -2873,12 +2895,17 @@ dw2_find_symbol_file (struct objfile *objfile, const char *name)
> ? /* vec[0] is the length, which must always be >0. ?*/
> ? per_cu = dw2_get_cu (MAYBE_SWAP (vec[1]));
>
> - ?file_data = dw2_get_file_names (objfile, per_cu);
> - ?if (file_data == NULL
> - ? ? ?|| file_data->num_file_names == 0)
> - ? ?return NULL;
> + ?if (per_cu->v.quick->symtab != NULL)
> + ? ?return per_cu->v.quick->symtab->filename;
> +
> + ?if (per_cu->is_debug_types)
> + ? ?init_cutu_and_read_dies (per_cu, 0, 0, dw2_get_primary_filename_reader,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?&filename);
> + ?else
> + ? ?init_cutu_and_read_dies_simple (per_cu, dw2_get_primary_filename_reader,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &filename);
>
> - ?return file_data->file_names[file_data->num_file_names - 1];
> + ?return filename;
> ?}
>
> ?static void

You need to call init_cutu_and_read_dies in both debug-types and
non-debug-types cases.
When DWO files are in use DW_AT_name lives in the DWO file.
OTOH TUs typically don't have DW_AT_name (you need to look at DW_AT_decl_file).
OTOOH this function is only called by find_main_filename.

So what to do?

I'm tempted to rename the routine {,*_}find_function_file (or some
such) and have dw2_find_symbol_file ignore TUs.