[PATCH][gdb/testsuite] Don't use default form in Dwarf::_guess_form

[PATCH][gdb/testsuite] Don't use default form in Dwarf::_guess_form

[Tom de Vries]

Hi,

The only possible form for a DW_AT_low_pc attribute is DW_FORM_addr.

When specifying in dwarf assembly a low_pc attribute without explicit form:
...
  {low_pc {main_label - 4}}
...
the resulting attribute uses DW_FORM_string, which is misinterpreted by gdb
when reading it as:
...
        cu->base_address = attr->as_address ();
...

Stop using DW_FORM_string as default form.  Instead, use a default form based
on the attribute name, if possible and unambiguous.  Otherwise, error out.

F.i.:
- for DW_AT_low_pc we use DW_FORM_addr.
- For DW_AT_high_pc, we don't specify a default form because it could be
  either address or constant class.
- For DW_AT_name, we use DW_FORM_string.  While we could encode with
  DW_FORM_strp instead, DW_FORM_string is always ok.

Tested on x86_64-linux.

Any comments?

Thanks,
- Tom

[gdb/testsuite] Don't use default form in Dwarf::_guess_form

gdb/testsuite/ChangeLog:

2020-10-23  Tom de Vries  <tdevries@suse.de>

	* lib/dwarf.exp (Dwarf::_guess_form): Return "" by default instead of
	DW_FORM_string.
	(Dwarf::_default_form): New proc.
	(Dwarf::_handle_DW_TAG): Use _default_form.  Error out if no form was
	guessed.

---
 gdb/testsuite/lib/dwarf.exp | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index 5c84063105..b6b7844726 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -247,11 +247,13 @@ proc get_func_info { name {options {debug}} } {
 #   and DW_FORM_ref4 is used.  See 'new_label' and 'define_label'.
 # * If VALUE starts with the "%" character, then it is a label
 #   reference too, but DW_FORM_ref_addr is used.
-# * Otherwise, VALUE is taken to be a string and DW_FORM_string is
-#   used.  In order to prevent bugs where a numeric value is given but
+# * In order to prevent bugs where a numeric value is given but
 #   no form is specified, it is an error if the value looks like a number
 #   (using Tcl's "string is integer") and no form is provided.
-# More form-guessing functionality may be added.
+# * Otherwise, if the attribute name has a default form (f.i. DW_FORM_addr for
+#   DW_AT_low_pc), then that one is used.
+# * Otherwise, an error is reported.  Either specify a form explicitly, or
+#   add a default for the the attribute name in _default_form.
 #
 # CHILDREN is just Tcl code that can be used to define child DIEs.  It
 # is evaluated in the caller's context.
@@ -590,9 +592,25 @@ namespace eval Dwarf {
 	    }
 
 	    default {
+		return ""
+	    }
+	}
+    }
+
+    proc _default_form { attr } {
+	switch -exact -- $attr {
+	    DW_AT_low_pc  {
+		return DW_FORM_addr
+	    }
+	    DW_AT_producer -
+	    DW_AT_comp_dir -
+	    DW_AT_linkage_name -
+	    DW_AT_MIPS_linkage_name -
+	    DW_AT_name {
 		return DW_FORM_string
 	    }
 	}
+	return ""
     }
 
     # Map NAME to its canonical form.
@@ -697,6 +715,12 @@ namespace eval Dwarf {
 			error "Integer value requires a form"
 		    }
 		    set attr_form [_guess_form $attr_value attr_value]
+		    if { $attr_form eq "" } {
+			set attr_form [_default_form $attr_name]
+		    }
+		    if { $attr_form eq "" } {
+			error "No form for $attr_name $attr_value"
+		    }
 		}
 		set attr_form [_map_name $attr_form _FORM]
 

Re: [PATCH][gdb/testsuite] Don't use default form in Dwarf::_guess_form

[Simon Marchi]

On 2020-10-23 7:46 a.m., Tom de Vries wrote:
> Hi,
>
> The only possible form for a DW_AT_low_pc attribute is DW_FORM_addr.
>
> When specifying in dwarf assembly a low_pc attribute without explicit form:
> ...
>   {low_pc {main_label - 4}}
> ...
> the resulting attribute uses DW_FORM_string, which is misinterpreted by gdb
> when reading it as:
> ...
>         cu->base_address = attr->as_address ();
> ...
>
> Stop using DW_FORM_string as default form.  Instead, use a default form based
> on the attribute name, if possible and unambiguous.  Otherwise, error out.
>
> F.i.:
> - for DW_AT_low_pc we use DW_FORM_addr.
> - For DW_AT_high_pc, we don't specify a default form because it could be
>   either address or constant class.
> - For DW_AT_name, we use DW_FORM_string.  While we could encode with
>   DW_FORM_strp instead, DW_FORM_string is always ok.
>
> Tested on x86_64-linux.
>
> Any comments?
>
> Thanks,
> - Tom

I like it, I like specifying explicit forms in any case.

I think it makes the "If the value looks like an integer, a form is
required." redundant though.  You could remove it, in which case
specifying an integer attribute without a form would fall in the "No
form for..." error branch.

Simon

Re: [PATCH][gdb/testsuite] Don't use default form in Dwarf::_guess_form

[Tom de Vries]

On 10/23/20 5:12 PM, Simon Marchi wrote:
> On 2020-10-23 7:46 a.m., Tom de Vries wrote:
>> Hi,
>>
>> The only possible form for a DW_AT_low_pc attribute is DW_FORM_addr.
>>
>> When specifying in dwarf assembly a low_pc attribute without explicit form:
>> ...
>>   {low_pc {main_label - 4}}
>> ...
>> the resulting attribute uses DW_FORM_string, which is misinterpreted by gdb
>> when reading it as:
>> ...
>>         cu->base_address = attr->as_address ();
>> ...
>>
>> Stop using DW_FORM_string as default form.  Instead, use a default form based
>> on the attribute name, if possible and unambiguous.  Otherwise, error out.
>>
>> F.i.:
>> - for DW_AT_low_pc we use DW_FORM_addr.
>> - For DW_AT_high_pc, we don't specify a default form because it could be
>>   either address or constant class.
>> - For DW_AT_name, we use DW_FORM_string.  While we could encode with
>>   DW_FORM_strp instead, DW_FORM_string is always ok.
>>
>> Tested on x86_64-linux.
>>
>> Any comments?
>>
>> Thanks,
>> - Tom
> 
> I like it, I like specifying explicit forms in any case.
> 
> I think it makes the "If the value looks like an integer, a form is
> required." redundant though.  You could remove it, in which case
> specifying an integer attribute without a form would fall in the "No
> form for..." error branch.

Done, and committed.

Thanks,
- Tom