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.
> diff --git a/libdw/ChangeLog b/libdw/ChangeLog
> index f37d9411..08b2c3e6 100644
> --- a/libdw/ChangeLog
> +++ b/libdw/ChangeLog
> @@ -12,10 +12,13 @@
> * dwarf_getmacros.c (get_macinfo_table): Replace assignment of
> table->is_64bit with assignments of table->address_size and
> table->offset_size. Assume default DW_AT_stmt_list of 0 for split
> - DWARF. Set table->dbg.
> + DWARF. Set table->dbg. Call dwarf_cu_dwp_section_info and add offset
> + to line_offset.
> (get_table_for_offset): Ditto.
> (read_macros): Get fake CU offset_size from table->offset_size instead
> of table->is_64bit.
> + (get_offset_from): Call dwarf_cu_dwp_section_info and add offset to
> + *retp.
> * dwarf_macro_getsrcfiles.c (dwarf_macro_getsrcfiles): Get
> address_size for __libdw_getsrclines from table->address_size instead
> of table->is_64bit. Get dbg for __libdw_getsrclines from table->dbg
> @@ -36,6 +39,9 @@
> (__libdw_set_debugdir): New declaration.
> (__libdw_dwp_findcu_id): New declaration.
> (__libdw_link_skel_split): Handle .debug_addr for dwp.
> + (str_offsets_base_off): Call dwarf_cu_dwp_section_info and add offset.
> + (__libdw_cu_ranges_base): Ditto.
> + (__libdw_cu_locs_base): Ditto.
> * dwarf_begin_elf.c (dwarf_scnnames): Add IDX_debug_cu_index and
> IDX_debug_tu_index.
> (scn_to_string_section_idx): Ditto.
> @@ -60,6 +66,8 @@
> __libdw_dwp_find_unit, and use it to adjust abbrev_offset and assign
> newp->dwp_row.
> * dwarf_cu_dwp_section_info.c: New file.
> + * dwarf_getlocation.c (initial_offset): Call dwarf_cu_dwp_section_info
> + and add offset to start_offset.
>
> 2023-02-22 Mark Wielaard <m...@klomp.org>
>
> diff --git a/libdw/dwarf_getlocation.c b/libdw/dwarf_getlocation.c
> index 553fdc98..37b32fc1 100644
> --- a/libdw/dwarf_getlocation.c
> +++ b/libdw/dwarf_getlocation.c
> @@ -812,6 +812,12 @@ initial_offset (Dwarf_Attribute *attr, ptrdiff_t *offset)
> : DWARF_E_NO_DEBUG_LOCLISTS),
> NULL, &start_offset) == NULL)
> return -1;
> +
> + Dwarf_Off loc_off;
> + if (INTUSE(dwarf_cu_dwp_section_info) (attr->cu, DW_SECT_LOCLISTS,
> + &loc_off, NULL) != 0)
> + return -1;
> + start_offset += loc_off;
> }
>
> *offset = start_offset;
OK. This already handled DW_FORM_loclistx, now also other loclist
refs.
> diff --git a/libdw/dwarf_getmacros.c b/libdw/dwarf_getmacros.c
> index a3a78884..2667eb45 100644
> --- a/libdw/dwarf_getmacros.c
> +++ b/libdw/dwarf_getmacros.c
> @@ -47,7 +47,15 @@ get_offset_from (Dwarf_Die *die, int name, Dwarf_Word
> *retp)
> return -1;
>
> /* Offset into the corresponding section. */
> - return INTUSE(dwarf_formudata) (&attr, retp);
> + if (INTUSE(dwarf_formudata) (&attr, retp) != 0)
> + return -1;
> +
> + Dwarf_Off offset;
> + if (INTUSE(dwarf_cu_dwp_section_info) (die->cu, DW_SECT_MACRO, &offset,
> NULL)
> + != 0)
> + return -1;
> + *retp += offset;
> + return 0;
> }
OK.
> static int
> @@ -131,6 +139,14 @@ get_macinfo_table (Dwarf *dbg, Dwarf_Word macoff,
> Dwarf_Die *cudie)
> else if (cudie->cu->unit_type == DW_UT_split_compile
> && dbg->sectiondata[IDX_debug_line] != NULL)
> line_offset = 0;
> + if (line_offset != (Dwarf_Off) -1)
> + {
> + Dwarf_Off dwp_offset;
> + if (INTUSE(dwarf_cu_dwp_section_info) (cudie->cu, DW_SECT_LINE,
> + &dwp_offset, NULL) != 0)
> + return NULL;
> + line_offset += dwp_offset;
> + }
OK.
> Dwarf_Macro_Op_Table *table = libdw_alloc (dbg, Dwarf_Macro_Op_Table,
> macinfo_data_size, 1);
> @@ -188,6 +204,14 @@ get_table_for_offset (Dwarf *dbg, Dwarf_Word macoff,
> if (unlikely (INTUSE(dwarf_formudata) (attr, &line_offset) != 0))
> return NULL;
> }
> + if (line_offset != (Dwarf_Off) -1 && cudie != NULL)
> + {
> + Dwarf_Off dwp_offset;
> + if (INTUSE(dwarf_cu_dwp_section_info) (cudie->cu, DW_SECT_LINE,
> + &dwp_offset, NULL) != 0)
> + return NULL;
> + line_offset += dwp_offset;
> + }
>
> uint8_t address_size;
> if (cudie != NULL)
OK.
> diff --git a/libdw/libdwP.h b/libdw/libdwP.h
> index 16355274..bdbf5e5a 100644
> --- a/libdw/libdwP.h
> +++ b/libdw/libdwP.h
> @@ -1111,25 +1111,27 @@ str_offsets_base_off (Dwarf *dbg, Dwarf_CU *cu)
> cu = first_cu;
> }
>
> + Dwarf_Off off = 0;
> if (cu != NULL)
> {
> if (cu->str_off_base == (Dwarf_Off) -1)
> {
> + dwarf_cu_dwp_section_info (cu, DW_SECT_STR_OFFSETS, &off, NULL);
> Dwarf_Die cu_die = CUDIE(cu);
> Dwarf_Attribute attr;
> if (dwarf_attr (&cu_die, DW_AT_str_offsets_base, &attr) != NULL)
> {
> - Dwarf_Word off;
> - if (dwarf_formudata (&attr, &off) == 0)
> + Dwarf_Word base;
> + if (dwarf_formudata (&attr, &base) == 0)
> {
> - cu->str_off_base = off;
> + cu->str_off_base = off + base;
> return cu->str_off_base;
> }
> }
> /* For older DWARF simply assume zero (no header). */
> if (cu->version < 5)
> {
> - cu->str_off_base = 0;
> + cu->str_off_base = off;
> return cu->str_off_base;
> }
>
> @@ -1142,7 +1144,6 @@ str_offsets_base_off (Dwarf *dbg, Dwarf_CU *cu)
>
> /* No str_offsets_base attribute, we have to assume "zero".
> But there could be a header first. */
> - Dwarf_Off off = 0;
> if (dbg == NULL)
> goto no_header;
>
> @@ -1184,7 +1185,7 @@ str_offsets_base_off (Dwarf *dbg, Dwarf_CU *cu)
> /* padding */
> read_2ubyte_unaligned_inc (dbg, readp);
>
> - off = (Dwarf_Off) (readp - start);
> + off += (Dwarf_Off) (readp - start);
>
> no_header:
> if (cu != NULL)
OK, so the dwp section offset gets added to the base attribute offset.
> @@ -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?
> + offset = dwp_offset;
> +
> if (dwarf_attr (&cu_die, DW_AT_rnglists_base, &attr) != NULL)
> {
> Dwarf_Word off;
> if (dwarf_formudata (&attr, &off) == 0)
> - offset = off;
> + offset += off;
> }
>
> /* There wasn't an rnglists_base, if the Dwarf does have a
> .debug_rnglists section, then it might be we need the
> base after the first header. */
> Elf_Data *data = cu->dbg->sectiondata[IDX_debug_rnglists];
> - if (offset == 0 && data != NULL)
> + if (offset == dwp_offset && data != NULL)
> {
> Dwarf *dbg = cu->dbg;
> const unsigned char *readp = data->d_buf;
> @@ -1279,8 +1284,8 @@ __libdw_cu_ranges_base (Dwarf_CU *cu)
> if (unit_length - 8 < needed)
> goto no_header;
>
> - offset = (Dwarf_Off) (offset_array_start
> - - (unsigned char *) data->d_buf);
> + offset += (Dwarf_Off) (offset_array_start
> + - (unsigned char *) data->d_buf);
> }
> }
> no_header:
OK, same for ranges.
> @@ -1297,21 +1302,24 @@ __libdw_cu_locs_base (Dwarf_CU *cu)
> {
> if (cu->locs_base == (Dwarf_Off) -1)
> {
> - Dwarf_Off offset = 0;
> + Dwarf_Off dwp_offset = 0;
> + dwarf_cu_dwp_section_info (cu, DW_SECT_LOCLISTS, &dwp_offset, NULL);
No error check?
> + Dwarf_Off offset = dwp_offset;
> +
> Dwarf_Die cu_die = CUDIE(cu);
> Dwarf_Attribute attr;
> if (dwarf_attr (&cu_die, DW_AT_loclists_base, &attr) != NULL)
> {
> Dwarf_Word off;
> if (dwarf_formudata (&attr, &off) == 0)
> - offset = off;
> + offset += off;
> }
>
> /* There wasn't an loclists_base, if the Dwarf does have a
> .debug_loclists section, then it might be we need the
> base after the first header. */
> Elf_Data *data = cu->dbg->sectiondata[IDX_debug_loclists];
> - if (offset == 0 && data != NULL)
> + if (offset == dwp_offset && data != NULL)
> {
> Dwarf *dbg = cu->dbg;
> const unsigned char *readp = data->d_buf;
> @@ -1357,8 +1365,8 @@ __libdw_cu_locs_base (Dwarf_CU *cu)
> if (unit_length - 8 < needed)
> goto no_header;
>
> - offset = (Dwarf_Off) (offset_array_start
> - - (unsigned char *) data->d_buf);
> + offset += (Dwarf_Off) (offset_array_start
> + - (unsigned char *) data->d_buf);
> }
>
> no_header:
OK. Same for loclists.
> diff --git a/tests/ChangeLog b/tests/ChangeLog
> index 97261444..93503f57 100644
> --- a/tests/ChangeLog
> +++ b/tests/ChangeLog
> @@ -1,6 +1,7 @@
> 2023-09-27 Omar Sandoval <osan...@fb.com>
>
> - * run-varlocs.sh: Add entry PC to split units.
> + * run-varlocs.sh: Add entry PC to split units. Check testfile-dwp-5
> + and testfile-dwp-4.
> * Makefile.am (check_PROGRAMS): Add cu-dwp-section-info.
> (TESTS): Add run-cu-dwp-section-info.sh
> (EXTRA_DIST): Add run-cu-dwp-section-info.sh and new test files.
> @@ -20,6 +21,11 @@
> (getmacros): New function.
> (main): If second argument is empty, call getmacros on every unit.
> Otherwise, call getmacros on given unit.
> + * run-all-dwarf-ranges.sh: Check testfile-dwp-5 and testfile-dwp-4.
> + * run-dwarf-getmacros.sh: Check testfile-dwp-5 and
> + testfile-dwp-4-strict.
> + * run-get-units-split.sh: Check testfile-dwp-5, testfile-dwp-4, and
> + testfile-dwp-4-strict.
Nice collection of tests.
Thanks,
Mark
