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!
