[PATCH][gdb/symtab] Fix gdb.base/vla-optimized-out.exp with clang

[Tom de Vries <tdevries@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 1671 bytes --]

[ was: Re: [PATCH] KFAIL variable-length array tests which fail with Clang ]

On 11/20/20 5:51 PM, Tom Tromey wrote:
>>> I don't really understand the is_reference stuff
> 
> Tom> In case a dwarf expression is used for an DW_AT_location attribute, by
> Tom> default it represents an address, and needs to be dereferenced to get
> Tom> the value.
> 
> Yeah, I guess I'd need to see some examples to understand why this
> decision is made here and not at the point of use.
> 
>>> Anyway, gdb can't do this sort of check.  It will fail if the expression
>>> has a different shape, which is completely allowed by the spec.
> 
> Tom> AFAIU, the spec specifically says how to interpret a DW_OP_stack_value
> Tom> at the end of the dwarf expression which is used a location description,
> Tom> and the code in the patch follows that reasoning.
> ...
> Tom> So, for my understanding, can you give an example of the problem you're
> Tom> envisioning?
> 
> Nothing prevents an expression from ending with some other DW_OP_* with
> 0x9f as an operand to the opcode.  This would confuse this simple
> checker.  Or to put it another way, nothing guarantees that the last
> byte of an expression is an opcode.  I think it could even be both,
> depending on a runtime condition, because AFAIK nothing prevents a DWARF
> expression from branching to the middle of some other operation.

Hmm, indeed, thanks for pointing this out.  That means that this needs
to be dealt with in the evaluator.  AFAICT, DWARF_VALUE_STACK is used
already to represent the DW_OP_stack_value op in the evaluator, it's
just not used for this scenario.

Another try below.  Any more comments?

Thanks,
- Tom



[-- Attachment #2: 0004-gdb-symtab-Fix-gdb.base-vla-optimized-out.exp-with-clang.patch --]
[-- Type: text/x-patch, Size: 5022 bytes --]

[gdb/symtab] Fix gdb.base/vla-optimized-out.exp with clang

Consider test-case gdb.base/vla-optimized-out.exp, compiled using clang-10.

GDB fails to get the size of the vla a:
...
(gdb) p sizeof (a)^M
Cannot access memory at address 0x6^M
(gdb) FAIL: gdb.base/vla-optimized-out.exp: o1: printed size of \
  optimized out vla
...

The relevant DWARF looks like this: the variable a:
...
 <2><12b>: Abbrev Number: 5 (DW_TAG_variable)
    <12c>   DW_AT_name        : a
    <132>   DW_AT_type        : <0x189>
...
has type:
...
 <1><189>: Abbrev Number: 10 (DW_TAG_array_type)
    <18a>   DW_AT_type        : <0x198>
 <2><18e>: Abbrev Number: 11 (DW_TAG_subrange_type)
    <18f>   DW_AT_type        : <0x19f>
    <193>   DW_AT_count       : <0x117>
...
with the count attribute equated to the value of this artificial variable:
...
 <2><117>: Abbrev Number: 4 (DW_TAG_variable)
    <118>   DW_AT_location    : 10 byte block: 75 1 10 ff ff ff ff f 1a 9f \
              (DW_OP_breg5 (rdi): 1;
	       DW_OP_constu: 4294967295;
	       DW_OP_and;
	       DW_OP_stack_value)
    <123>   DW_AT_name        : __vla_expr0
    <127>   DW_AT_type        : <0x182>
    <12b>   DW_AT_artificial  : 1
...

The location description of the variable is terminated with DW_OP_stack_value,
which according to the DWARF spec means that "the DWARF expression represents
the actual value of the object, rather than its location".

However, in attr_to_dynamic_prop, we set is_reference to true:
...
               baton->locexpr.is_reference = true;
...
and use it in dwarf2_evaluate_property to dereference the value of the DWARF
expression, which causes the access to memory at address 0x6.

Fix this by ignoring the baton->locexpr.is_reference == true setting if
the expression evaluation has ctx.location == DWARF_VALUE_STACK, such that we
get:
...
(gdb) p sizeof (a)^M
$2 = 6^M
(gdb) PASS: gdb.base/vla-optimized-out.exp: o1: printed size of \
  optimized out vla
...

Tested on x86_64-linux, with gcc.

Tested the following test-cases (the ones mentioned in PR26905) on
x86_64-linux with clang-10:
- gdb.base/vla-optimized-out.exp
- gdb.base/vla-ptr.exp
- gdb.mi/mi-vla-c99

gdb/ChangeLog:

2020-11-20  Tom de Vries  <tdevries@suse.de>

	PR symtab/26905
	* dwarf2/loc.c (dwarf2_locexpr_baton_eval): Add and handle
	is_reference parameter.
	(dwarf2_evaluate_property): Update dwarf2_locexpr_baton_eval call.

gdb/testsuite/ChangeLog:

2020-11-22  Tom de Vries  <tdevries@suse.de>

	PR symtab/26905
	* gdb.dwarf2/count.exp: Remove kfails.

---
 gdb/dwarf2/loc.c                   | 13 +++++++++----
 gdb/testsuite/gdb.dwarf2/count.exp |  4 ----
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 86d0ca496a2..fea49c340ff 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -2499,7 +2499,8 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
 			   struct frame_info *frame,
 			   const struct property_addr_info *addr_stack,
 			   CORE_ADDR *valp,
-			   bool push_initial_value)
+			   bool push_initial_value,
+			   bool *is_reference)
 {
   if (dlbaton == NULL || dlbaton->size == 0)
     return 0;
@@ -2546,9 +2547,12 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
 
   switch (ctx.location)
     {
+    case DWARF_VALUE_STACK:
+      *is_reference = false;
+      /* FALLTHROUGH */
+
     case DWARF_VALUE_REGISTER:
     case DWARF_VALUE_MEMORY:
-    case DWARF_VALUE_STACK:
       *valp = ctx.fetch_address (0);
       if (ctx.location == DWARF_VALUE_REGISTER)
 	*valp = ctx.read_addr_from_reg (*valp);
@@ -2589,10 +2593,11 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,
 	  = (const struct dwarf2_property_baton *) prop->baton ();
 	gdb_assert (baton->property_type != NULL);
 
+	bool is_reference = baton->locexpr.is_reference;
 	if (dwarf2_locexpr_baton_eval (&baton->locexpr, frame, addr_stack,
-				       value, push_initial_value))
+				       value, push_initial_value, &is_reference))
 	  {
-	    if (baton->locexpr.is_reference)
+	    if (is_reference)
 	      {
 		struct value *val = value_at (baton->property_type, *value);
 		*value = value_as_address (val);
diff --git a/gdb/testsuite/gdb.dwarf2/count.exp b/gdb/testsuite/gdb.dwarf2/count.exp
index 6dcccb57409..ff11958827b 100644
--- a/gdb/testsuite/gdb.dwarf2/count.exp
+++ b/gdb/testsuite/gdb.dwarf2/count.exp
@@ -166,11 +166,7 @@ gdb_test "whatis static_array" "type = char \\\[5\\\]"
 gdb_test "print static_array" " = \"world\""
 gdb_test "print sizeof static_array" " = 5"
 
-setup_kfail "gdb/26905" *-*-*
 gdb_test "ptype vla_array" "type = char \\\[6\\\]"
-setup_kfail "gdb/26905" *-*-*
 gdb_test "whatis vla_array" "type = char \\\[6\\\]"
-setup_kfail "gdb/26905" *-*-*
 gdb_test "print vla_array" " = \"saluto\""
-setup_kfail "gdb/26905" *-*-*
 gdb_test "print sizeof vla_array" " = 6"