Re: [PATCH] Dwarf 5: Handle debug_str_offsets and indexed attributes that have base offsets.

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2019-11-18 11:46 p.m., Ali Tamur via gdb-patches wrote:
> Reply:
> Sure, here's an example.
> $  cd /tmp/dw5 && echo "int calculate() { return 4; } int main(int argc, char** argv) { return calculate(); }" >>main.cc
> $  clang -gdwarf-5 -gsplit-dwarf main.cc
> $  ls
> a.out main.cc main.dwo 
> 
> $  llvm-dwarfdump --all a.out
> a.out:	file format ELF64-x86-64
> 
> .debug_abbrev contents:
> Abbrev table for offset: 0x00000000
> [1] DW_TAG_compile_unit	DW_CHILDREN_no
> 	DW_AT_stmt_list	DW_FORM_sec_offset
> 	DW_AT_str_offsets_base	DW_FORM_sec_offset
> 	DW_AT_comp_dir	DW_FORM_strx1
> 	DW_AT_GNU_pubnames	DW_FORM_flag_present
> 	DW_AT_GNU_dwo_name	DW_FORM_strx1
> 	DW_AT_low_pc	DW_FORM_addrx
> 	DW_AT_high_pc	DW_FORM_data4
> 	DW_AT_addr_base	DW_FORM_sec_offset
> 
> .debug_info contents:
> 0x00000000: Compile Unit: length = 0x00000024 version = 0x0005 unit_type = DW_UT_skeleton abbr_offset = 0x0000 addr_size = 0x08 DWO_id = 0xee1d4b42a2f0ca0b (next unit at 0x00000028)
> 
> 0x00000014: DW_TAG_compile_unit
>               DW_AT_stmt_list	(0x00000000)
>               DW_AT_str_offsets_base	(0x00000008)
>               DW_AT_comp_dir	("/tmp/dw5")
>               DW_AT_GNU_pubnames	(true)
>               DW_AT_GNU_dwo_name	("main.dwo")
>               DW_AT_low_pc	(0x0000000000401110)
>               DW_AT_high_pc	(0x0000000000401141)
>               DW_AT_addr_base	(0x00000008)
> ....
> .debug_str contents:
> 0x00000000: "/tmp/dw5"
> 0x00000009: "main.dwo"
> ....
> .debug_str_offsets contents:
> 0x00000000: Contribution size = 12, Format = DWARF32, Version = 5
> 0x00000008: 00000000 "/tmp/dw5"
> 0x0000000c: 00000009 "main.dwo"
> 
> Here is what happens. gdb starts to parse DW_TAG_compile_unit DIE. It comes
> to DW_AT_GNU_dwo_name. It is of form DW_FORM_strx1 and it has a value of 1.
> The actual value is somewhere in .debug_str section. To find it we need to
> process .debug_str_offsets (refer to 1st index somewhere within) and also know
> the value of DW_AT_str_offsets_base. However, we are in the middle of parsing
> the die and there is no guarantee that we have yet processed
> DW_AT_str_offsets_base, it may be parsed later. 
> 
> My solution is to temporarily write "1" as the value of the attribute
> DW_AT_GNU_dwo_name, and mark it as 'needs reprocessing'. After all the
> attributes of the die have been processed, DW_AT_str_offsets_base will hold the
> correct value if it exists. Then, we revisit the marked attributes. During
> reprocess, we don't need to read the binary file again, because we had already
> written the value we need (1) in the first pass. We calculate the correct
> address using that value, the contents of .debug_str* sections and
> DW_AT_str_offsets_base value.

Ok, so that reprocessing happens right after having read the attributes from the DIE.

Another approach that I would have expected would be to do it more lazily.  Only keep
the index in the attribute, and go read the actual string when we call dwarf2_string_attr
on it (and then maybe cache the value).  Did you consider doing something like that, and
perhaps it was a too involving change?

I see that this is kind of done in get_stub_string_attr, for attributes marked with
DW_STRING_IS_STR_INDEX, so I was wondering why not do it for all strx forms and avoid
the reprocessing.

> +/* Process the attributes that had to be skipped in the first round. These
> +   attributes are the ones that need str_offsets_base or addr_base attributes.
> +   They could not have been processed in the first round, because at the time
> +   the values of str_offsets_base or addr_base may not have been known.  */
> +void read_attribute_reprocess (const struct die_reader_specs *reader,
> +			       struct attribute *attr)
> +{
> +  struct dwarf2_cu *cu = reader->cu;
> +  switch (attr->form)
> +    {
> +      case DW_FORM_addrx:
> +      case DW_FORM_GNU_addr_index:
> +        DW_ADDR (attr) = read_addr_index (cu, DW_UNSND (attr));
> +        break;
> +      case DW_FORM_strx:
> +      case DW_FORM_strx1:
> +      case DW_FORM_strx2:
> +      case DW_FORM_strx3:
> +      case DW_FORM_strx4:
> +      case DW_FORM_GNU_str_index:
> +        unsigned int str_index = DW_UNSND (attr);
> +	if (reader->dwo_file != NULL)
> +	  {
> +	    DW_STRING (attr) = read_dwo_str_index (reader, str_index);
> +	    DW_STRING_IS_CANONICAL (attr) = 0;
> +	    DW_STRING_IS_STR_INDEX (attr) = false;
> +	  }
> +	else if (cu->str_offsets_base.has_value ())
> +	  {
> +	    DW_STRING (attr) = read_stub_str_index (cu, str_index);
> +	    DW_STRING_IS_CANONICAL (attr) = 0;
> +	    DW_STRING_IS_STR_INDEX (attr) = false;
> +	  }
> +	else
> +	  {
> +	    if (attr->name != DW_AT_comp_dir
> +		&& attr->name != DW_AT_GNU_dwo_name
> +		&& attr->name != DW_AT_dwo_name)
> +	      {
> +		error (_("Dwarf Error: %s/%s found in non-DWO CU"),
> +		       dwarf_form_name (attr->form),
> +		       dwarf_attr_name (attr->name));
> +	      }
> +	    DW_UNSND (attr) = str_index;
> +	    DW_STRING_IS_STR_INDEX (attr) = true;
> +	  }

I don't understand this part.  When read_attribute_reprocess is called,
if there's a DW_AT_str_offsets_base, we will have read it at this point.
So when is the else clause actually useful?  Can you give a practical
example of how to exercise this code?

Simon