Re: [PATCH 13/14] libdw: Apply DWARF package file section offsets where appropriate

[Omar Sandoval <osandov@osandov.com>]

On Thu, Nov 02, 2023 at 11:20:33PM +0100, Mark Wielaard wrote:
> Hi Omar,
> 
> On Wed, Sep 27, 2023 at 11:21:02AM -0700, Omar Sandoval wrote:
> > The final piece of DWARF package file support is that offsets have to be
> > interpreted relative to the section offset from the package index.
> > .debug_abbrev.dwo is already covered, so sprinkle around calls to
> > dwarf_cu_dwp_section_info for the remaining sections: .debug_line.dwo,
> > .debug_loclists.dwo/.debug_loc.dwo, .debug_str_offsets.dwo,
> > .debug_macro.dwo/.debug_macinfo.dwo, and .debug_rnglists.dwo.  With all
> > of that in place, we can finally test various libdw functions on dwp
> > files.
> 
> This looks good, but obviously needs some earlier patches.

[snip]

> > @@ -1222,18 +1223,22 @@ __libdw_cu_ranges_base (Dwarf_CU *cu)
> >  	}
> >        else
> >  	{
> > +	  Dwarf_Off dwp_offset = 0;
> > +	  dwarf_cu_dwp_section_info (cu, DW_SECT_RNGLISTS, &dwp_offset, NULL);
> 
> Shouldn't we check there wasn't an error?

Yeah, I was a little confused by the error handling in these foo_base
functions. It doesn't seem like most of the callers are checking for
errors. The foo_base functions also seem to be treating invalid
attributes as if they don't exist (e.g., when dwarf_formudata returns an
error):

  if (dwarf_attr (&cu_die, DW_AT_rnglists_base, &attr) != NULL)
    {
      Dwarf_Word off;
      if (dwarf_formudata (&attr, &off) == 0)
	offset += off;
    }

So I suppose the equivalent for this change would be

  Dwarf_Off dwp_offset = 0;
  if (dwarf_cu_dwp_section_info (cu, DW_SECT_RNGLISTS, &dwp_offset, NULL) == 0)
    offset = dwp_offset;

[snip]

Thanks!