Re: [PATCH] read_file_scope - avoid eventual crash when the file or directory name is null.

[Andrew Burgess <aburgess@redhat.com>]

"Dominic Clifton" <me@dominicclifton.name> writes:

> * without this, launching the debugger results in the following error:
>
> ```
> terminate called after throwing an instance of 'std::logic_error' what():
> basic_string: construction from null is not valid
> ```

Thanks for looking into this failure.

>
> find_file_and_directory - return empty strings instead of null pointers.
>
> ---
>  gdb/dwarf2/read.c | 29 +++++++++++++++++++++--------
>  1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 955893c5f0c..287ce7b518c 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -5942,10 +5942,18 @@ find_file_and_directory (struct die_info *die,
> struct dwarf2_cu *cu)
>    if (cu->per_cu->fnd != nullptr)
>      return *cu->per_cu->fnd;
>  
> +  // Ensure that nullptrs become empty strings
> +  const char *name = dwarf2_string_attr(die, DW_AT_name, cu);
> +  const char *comp_dir = dwarf2_string_attr(die, DW_AT_comp_dir, cu);
> +
> +  if (!name)
> +    name = "";
> +  if (!comp_dir)
> +    comp_dir = "";
> +
>    /* Find the filename.  Do not use dwarf2_name here, since the filename
>       is not a source language identifier.  */
> -  file_and_directory res (dwarf2_string_attr (die, DW_AT_name, cu),
> -			  dwarf2_string_attr (die, DW_AT_comp_dir, cu));
> +  file_and_directory res(name, comp_dir);

I suspect this change is going to cause problems.  If you checkout
struct file_and_directory in file-and-dir.h you'll see there are a
couple of methods which check for nullptr as a special value, so masking
all of the nullptr inputs is going to change the behaviour.

>  
>    if (res.get_comp_dir () == nullptr

And just here we have an example of some code which appears to be
checking for nullptr, which is no longer going to work as expected after
this change.

Could you expand on what the DWARF looks like that is triggering this
failure?

We have a DWARF assembler in our testsuite, see gdb.dwarf2/ for some
examples (look for 'Dwarf::assemble'), so we should be able to reproduce
the problematic DWARF and trigger the failure.  I initially figured it
might be as simple as a missing DW_AT_name on a DW_TAG_compile_unit, but
that didn't seem to trigger the issue, so there must be more too it.

If you can show the DWARF in question then we can likely write a test
for this issue, or, if you feel adventurous, you could have a go
yourself.

Thanks,
Andrew




>        && cu->producer_is_gcc_lt_4_3 ()
> @@ -6083,28 +6091,33 @@ read_file_scope (struct die_info *die, struct
> dwarf2_cu *cu)
>  
>    file_and_directory &fnd = find_file_and_directory (die, cu);
>  
> +  // ensure fnd.get_name() and fnd.get_comp_dir() are never null, which
> they sometimes are
> +  const char *fname = fnd.get_name() ? fnd.get_name() : "";
> +  const char *compdir = fnd.get_comp_dir() ? fnd.get_comp_dir() : "";
> +
>    /* GAS supports generating dwarf-5 info starting version 2.35.  Versions
>       2.35-2.37 generate an incorrect CU name attribute: it's relative,
>       implicitly prefixing it with the compilation dir.  Work around this by
>       prefixing it with the source dir instead.  */
> -  if (cu->header.version == 5 && !IS_ABSOLUTE_PATH (fnd.get_name ())
> +  if (cu->header.version == 5 && !IS_ABSOLUTE_PATH (fname)
>        && cu->producer_is_gas_lt_2_38 ())
>      {
>        attr = dwarf2_attr (die, DW_AT_stmt_list, cu);
>        if (attr != nullptr && attr->form_is_unsigned ())
>  	{
>  	  sect_offset line_offset = (sect_offset) attr->as_unsigned ();
> -	  line_header_up lh = dwarf_decode_line_header (line_offset, cu,
> -							fnd.get_comp_dir
> ());
> +	  line_header_up lh = dwarf_decode_line_header (line_offset, cu,
> compdir);
>  	  if (lh->version == 5 && lh->include_dir_at (1) != nullptr)
>  	    {
> -	      std::string dir = lh->include_dir_at (1);
> -	      fnd.set_comp_dir (std::move (dir));
> +          const char* dir_ptr = lh->include_dir_at (1);
> +          std::string dir = dir_ptr ? dir_ptr : compdir;
> +
> +          fnd.set_comp_dir (std::move (dir));
>  	    }
>  	}
>      }
>  
> -  cu->start_compunit_symtab (fnd.get_name (), fnd.intern_comp_dir
> (objfile),
> +  cu->start_compunit_symtab (fname, fnd.intern_comp_dir (objfile),
>  			     lowpc);
>  
>    gdb_assert (per_objfile->sym_cu == nullptr);
> -- 
> 2.48.1