Re: [PATCH v5] gdb: Support embedded source in DWARF

[Tom Tromey <tom@tromey.com>]

>>>>> "Will" == Will Hawkins <hawkinsw@obs.cr> writes:

Will> While DW_LNCT_source is not yet finalized in the DWARF standard
Will> (https://dwarfstd.org/issues/180201.1.html), LLVM does emit it.

Will> This patch adds support for it in gdb.

Thanks.

Will> -      m_fullname = find_source_or_rewrite (get_name (), get_comp_dir ());
Will> +      {
Will> +	if (m_is_embedded)
Will> +	  m_fullname = make_unique_xstrdup (embedded_fullname (
Will> +							       get_name (),
Will> +							       get_comp_dir ()));

No line break before the get_name call.

Will> +	  else if (entry.source != nullptr)
Will> +	    {
Will> +	      /* We have an embedded source for the CU.  */
Will> +	      gdb_assert (offset == 1);
Will> +	      cu_file_embedded = true;

This assert makes me a bit nervous.  Can this be fooled by bad input
data?  Or is it really testing some internal invariant?  I couldn't
convince myself it was the latter.

Will> +	fullname.reset (embedded_fullname (dirname, qfn->file_names[index]));

fullname = ...

Will> +  /* Because the line header may tell us information about the CU
Will> +     filename (e.g., whether it is embedded) which will affect other
Will> +     calculations, we have to read that information here.  */
Will> +  line_header *lh = cu->line_header;
Will> +  struct attribute *attr = dwarf2_attr (die, DW_AT_stmt_list, cu);
Will> +  if (lh == nullptr && attr != nullptr && attr->form_is_unsigned ())
Will> +    {
Will> +      sect_offset line_offset = (sect_offset) attr->as_unsigned ();
Will> +      line_header_up lhu = dwarf_decode_line_header (line_offset, cu,
Will> +				     res.get_comp_dir ());
Will> +      if (lhu != nullptr)
Will> +	  lh = lhu.release();

This will leak memory.

However if this is the first time reading the header, should it be
stashed somewhere?  It seems to me it shouldn't have to be read more
than once.

IOW, how does this case get reached?

Will> +/* See source.h.  */
Will> +
Will> +char *
Will> +embedded_fullname (const char *dirname, const char *filename)

Should return a unique_xmalloc_ptr<char>

Will> +{
Will> +  if (dirname != nullptr)
Will> +    {
Will> +      return concat (dirname, SLASH_STRING, filename, (char *) nullptr);
Will> +    }

No braces needed.

Will> +
Will> +  return xstrdup (filename);

make_unique_xstrdup

thanks,
Tom