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

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

[Gary Benson via Gdb-patches]

Hi all,

Clang describes the upper bounds of variable length arrays using
a DW_AT_count attribute which references the DIE of a synthetic
variable whose value is specified using a DW_AT_location.  In
some cases GDB correctly handles these, but in other cases GDB
adds an extra dereference causing the test to fail.  This commit
marks these failing tests as caused by PR gdb/26905 when the
test executable was built using Clang.

Checked on Fedora 32 x86_64, with GCC and Clang.  Ok to commit?

Thanks,
Gary

---
gdb/testsuite/ChangeLog:

	PR gdb/26905
	* gdb.base/vla-optimized-out.exp: Add KFAILs for PR gdb/26905.
	* gdb.base/vla-ptr.exp: Likewise.
---
 gdb/testsuite/ChangeLog                      | 6 ++++++
 gdb/testsuite/gdb.base/vla-optimized-out.exp | 8 +++++++-
 gdb/testsuite/gdb.base/vla-ptr.exp           | 2 ++
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/vla-optimized-out.exp b/gdb/testsuite/gdb.base/vla-optimized-out.exp
index 203a82d..aaa6cd5 100644
--- a/gdb/testsuite/gdb.base/vla-optimized-out.exp
+++ b/gdb/testsuite/gdb.base/vla-optimized-out.exp
@@ -16,6 +16,7 @@
 # Check whether we can determine the size of an optimized-out vla.
 
 standard_testfile
+set using_clang [test_compiler_info clang*]
 
 # The EXE_SUFFIX is a string appended to the name of the test binary
 # to make it unique per variation.
@@ -23,7 +24,7 @@ standard_testfile
 # flags used for building the test binary, and the second item is a
 # pattern which matches some expected output within this proc.
 proc vla_optimized_out {exe_suffix options} {
-    global testfile srcfile
+    global testfile srcfile using_clang
 
     lassign $options compile_flags sizeof_result
 
@@ -37,10 +38,12 @@ proc vla_optimized_out {exe_suffix options} {
 	return
     }
 
+    if {$using_clang} { setup_kfail "gdb/26905" *-*-* }
     gdb_test "p a" \
 	" = <optimized out>" \
 	"printed optimized out vla"
 
+    if {$using_clang} { setup_kfail "gdb/26905" *-*-* }
     gdb_test "p sizeof (a)" \
 	" = ($sizeof_result)" \
 	"printed size of optimized out vla"
@@ -55,14 +58,17 @@ proc vla_optimized_out {exe_suffix options} {
     # been removed.  As such GDB can't be expected to know if the
     # array contains _any_ elements at all.  It seems reasonable in
     # that case to reply with 'no such vector element'.
+    if {$using_clang} { setup_kfail "gdb/26905" *-*-* }
     gdb_test "p a\[0\]" \
 	"(= <optimized out>|no such vector element)" \
 	"print out of range element of vla (0)"
 
+    if {$using_clang} { setup_kfail "gdb/26905" *-*-* }
     gdb_test "p a\[6\]" \
 	"no such vector element" \
 	"print out of range element of vla (6)"
 
+    if {$using_clang} { setup_kfail "gdb/26905" *-*-* }
     gdb_test "p a\[0xffffffff\]" \
 	"no such vector element" \
 	"print out of range element of vla (0xffffffff)"
diff --git a/gdb/testsuite/gdb.base/vla-ptr.exp b/gdb/testsuite/gdb.base/vla-ptr.exp
index 346ff00..adf1779 100644
--- a/gdb/testsuite/gdb.base/vla-ptr.exp
+++ b/gdb/testsuite/gdb.base/vla-ptr.exp
@@ -14,6 +14,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 standard_testfile
+set using_clang [test_compiler_info clang*]
 
 if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
     return -1
@@ -38,4 +39,5 @@ gdb_test "print *vla_ptr" " = 2" "print *vla_ptr (bar)"
 
 gdb_breakpoint [gdb_get_line_number "vla_func_bp"]
 gdb_continue_to_breakpoint "vla_func_bp"
+if {$using_clang} { setup_kfail "gdb/26905" *-*-* }
 gdb_test "print td_vla" " = \\\{4, 5, 6, 7, 8\\\}"
-- 
1.8.3.1

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

[Tom Tromey]

>>>>> "Gary" == Gary Benson via Gdb-patches <gdb-patches@sourceware.org> writes:

Gary> Clang describes the upper bounds of variable length arrays using
Gary> a DW_AT_count attribute which references the DIE of a synthetic
Gary> variable whose value is specified using a DW_AT_location.  In
Gary> some cases GDB correctly handles these, but in other cases GDB
Gary> adds an extra dereference causing the test to fail.  This commit
Gary> marks these failing tests as caused by PR gdb/26905 when the
Gary> test executable was built using Clang.

Shouldn't we just fix the gdb bug instead?
Or is there something preventing that?  I feel like I may not fully
appreciate the problem.

Tom

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

[Tom de Vries]

On 11/18/20 5:14 PM, Tom Tromey wrote:
>>>>>> "Gary" == Gary Benson via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Gary> Clang describes the upper bounds of variable length arrays using
> Gary> a DW_AT_count attribute which references the DIE of a synthetic
> Gary> variable whose value is specified using a DW_AT_location.  In
> Gary> some cases GDB correctly handles these, but in other cases GDB
> Gary> adds an extra dereference causing the test to fail.  This commit
> Gary> marks these failing tests as caused by PR gdb/26905 when the
> Gary> test executable was built using Clang.
> 
> Shouldn't we just fix the gdb bug instead?

This adhoc patch:
...
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 3c598262913..d27e8613d1c 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -18644,7 +18644,9 @@ attr_to_dynamic_prop (const struct attribute
*attr, struct die
_info *die,
                struct dwarf_block *block = target_attr->as_block ();
                baton->locexpr.size = block->size;
                baton->locexpr.data = block->data;
-               baton->locexpr.is_reference = true;
+               gdb_byte last = block->data[block->size - 1];
+               baton->locexpr.is_reference
+                 = !(last == DW_OP_stack_value || last ==
DW_OP_implicit_value);
                prop->set_locexpr (baton);
                gdb_assert (prop->baton () != NULL);
              }
...
fixes gdb.base/vla-optimized-out.exp with clang-10 for me.

Thanks,
- Tom

> Or is there something preventing that?  I feel like I may not fully
> appreciate the problem.
> 
> Tom
> 

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

[Tom de Vries]

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

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

On 11/19/20 11:53 PM, Tom de Vries wrote:
> On 11/18/20 5:14 PM, Tom Tromey wrote:
>>>>>>> "Gary" == Gary Benson via Gdb-patches <gdb-patches@sourceware.org> writes:
>>
>> Gary> Clang describes the upper bounds of variable length arrays using
>> Gary> a DW_AT_count attribute which references the DIE of a synthetic
>> Gary> variable whose value is specified using a DW_AT_location.  In
>> Gary> some cases GDB correctly handles these, but in other cases GDB
>> Gary> adds an extra dereference causing the test to fail.  This commit
>> Gary> marks these failing tests as caused by PR gdb/26905 when the
>> Gary> test executable was built using Clang.
>>
>> Shouldn't we just fix the gdb bug instead?
> 
> This adhoc patch:

Here's the complete and tested patch.

Any comments?

Thanks,
- Tom

[-- Attachment #2: 0004-gdb-symtab-Fix-gdb.base-vla-optimized-out.exp-with-clang.patch --]
[-- Type: text/x-patch, Size: 7415 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 regardless:
...
               baton->locexpr.is_reference = true;
...
which causes the access to memory at address 0x6.

Fix this by detecting that the dwarf expresssion is terminated with
DW_OP_stack_value, and if so setting baton->locexpr.is_reference to false,
such that we have instead:
...
(gdb) p sizeof (a)^M
$2 = 6^M
(gdb) PASS: gdb.base/vla-optimized-out.exp: o1: printed size of \
  optimized out vla
...

Add dwarf assembly test-case in gdb.dwarf2/count.exp.

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

This FAIL (with clang) is due to missing debuginfo:
...
FAIL: gdb.base/vla-ptr.exp: print td_vla
...
so mark it as XFAIL.

gdb/ChangeLog:

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

	PR symtab/26905
	* dwarf2/read.c (attr_to_dynamic_prop): Handle DW_OP_stack_value as
	end of DW_AT_location expression.

gdb/testsuite/ChangeLog:

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

	PR symtab/26905
	* gdb.base/vla-ptr.exp: Add XFAIL.
	* gdb.dwarf2/count.exp: Add test-case.

---
 gdb/dwarf2/read.c                  | 12 +++++++++++-
 gdb/testsuite/gdb.base/vla-ptr.exp | 39 +++++++++++++++++++++++++++++++++++++-
 gdb/testsuite/gdb.dwarf2/count.exp | 31 +++++++++++++++++++++++++++++-
 3 files changed, 79 insertions(+), 3 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 3c598262913..3782a516a3b 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -18644,7 +18644,17 @@ attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
 		struct dwarf_block *block = target_attr->as_block ();
 		baton->locexpr.size = block->size;
 		baton->locexpr.data = block->data;
-		baton->locexpr.is_reference = true;
+		if (block->size > 0
+		    && block->data[block->size - 1] == DW_OP_stack_value)
+		  {
+		    /* A DW_OP_stack_value at the end of a location
+		       description means that the expression represents the
+		       actual value of the object, rather than its
+		       location.  */
+		    baton->locexpr.is_reference = false;
+		  }
+		else
+		  baton->locexpr.is_reference = true;
 		prop->set_locexpr (baton);
 		gdb_assert (prop->baton () != NULL);
 	      }
diff --git a/gdb/testsuite/gdb.base/vla-ptr.exp b/gdb/testsuite/gdb.base/vla-ptr.exp
index 346ff00c524..72d7d36489e 100644
--- a/gdb/testsuite/gdb.base/vla-ptr.exp
+++ b/gdb/testsuite/gdb.base/vla-ptr.exp
@@ -14,6 +14,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 standard_testfile
+set using_clang [test_compiler_info clang*]
 
 if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
     return -1
@@ -38,4 +39,40 @@ gdb_test "print *vla_ptr" " = 2" "print *vla_ptr (bar)"
 
 gdb_breakpoint [gdb_get_line_number "vla_func_bp"]
 gdb_continue_to_breakpoint "vla_func_bp"
-gdb_test "print td_vla" " = \\\{4, 5, 6, 7, 8\\\}"
+
+gdb_test_multiple "print td_vla" "" {
+    -re -wrap " = \\\{4, 5, 6, 7, 8\\\}" {
+	pass $gdb_test_name
+    }
+    -re -wrap " = $hex" {
+	if { $using_clang } {
+	    # Clang 10.0.1 fails to generate complete type info, see
+	    # note below.
+	    xfail $gdb_test_name
+	    # Verify that despite the incomplete type info, the variable is
+	    # there and has the right value.
+	    gdb_test "p *td_vla@5" " = \\\{4, 5, 6, 7, 8\\\}"
+	} else {
+	    fail $gdb_test_name
+	}
+    }
+}
+
+# Clang 10.0.1 generates this DWARF for td_vla:
+#
+# A variable DIE:
+# <2><19f>: Abbrev Number: 6 (DW_TAG_variable)
+#    <1a0>   DW_AT_location    : 0x39 (location list)
+#    <1a4>   DW_AT_name        : td_vla
+#    <1aa>   DW_AT_type        : <0x1ae>
+# with type:
+# <2><1ae>: Abbrev Number: 7 (DW_TAG_typedef)
+#    <1af>   DW_AT_type        : <0x1fc>
+#    <1b3>   DW_AT_name        : typedef_vla
+# pointing to:
+# <1><1fc>: Abbrev Number: 11 (DW_TAG_array_type)
+#    <1fd>   DW_AT_type        : <0x1d3>
+# <2><201>: Abbrev Number: 14 (DW_TAG_subrange_type)
+#    <202>   DW_AT_type        : <0x1f5>
+#
+# The subrange type is missing the count attribute.
diff --git a/gdb/testsuite/gdb.dwarf2/count.exp b/gdb/testsuite/gdb.dwarf2/count.exp
index 5cefb15da4a..4d44a29331e 100644
--- a/gdb/testsuite/gdb.dwarf2/count.exp
+++ b/gdb/testsuite/gdb.dwarf2/count.exp
@@ -28,7 +28,10 @@ set asm_file [standard_output_file $srcfile2]
 Dwarf::assemble $asm_file {
     cu {} {
 	compile_unit {{language @DW_LANG_C99}} {
-	    declare_labels char_label array_label array_label2 static_array_label
+	    declare_labels char_label \
+		array_label array_label2 array_label3 \
+		static_array_label \
+		len_label
 
 	    char_label: base_type {
 		{name char}
@@ -54,6 +57,24 @@ Dwarf::assemble $asm_file {
 		}
 	    }
 
+	    len_label: DW_TAG_variable {
+		{type :$char_label}
+		{location {
+		    const1u 6
+		    stack_value
+		} SPECIAL_expr}
+	    }
+
+	    array_label3: array_type {
+		{name arraytype3}
+		{type :$char_label}
+	    } {
+		subrange_type {
+		    {count :$len_label}
+		    {type :$char_label}
+		}
+	    }
+
 	    static_array_label: array_type {
 		{type :$char_label}
 	    } {
@@ -69,6 +90,11 @@ Dwarf::assemble $asm_file {
 		{const_value 65 DW_FORM_udata}
 	    }
 
+	    DW_TAG_variable {
+		{name array3}
+		{type :$array_label3}
+	    }
+
 	    DW_TAG_variable {
 		{name array}
 		{type :$array_label}
@@ -123,3 +149,6 @@ gdb_test "ptype static_array" "type = char \\\[5\\\]"
 gdb_test "whatis static_array" "type = char \\\[5\\\]"
 gdb_test "print static_array" " = \"world\""
 gdb_test "print sizeof static_array" " = 5"
+
+gdb_test "ptype array3" "type = char \\\[6\\\]"
+gdb_test "p array3" " = <optimized out>"

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

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> +               gdb_byte last = block->data[block->size - 1];
Tom> +               baton->locexpr.is_reference
Tom> +                 = !(last == DW_OP_stack_value || last == DW_OP_implicit_value);

I don't really understand the is_reference stuff, but my first reaction
is that it must be incorrect somehow.

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

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

[Tom de Vries]

On 11/20/20 4:50 PM, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> +               gdb_byte last = block->data[block->size - 1];
> Tom> +               baton->locexpr.is_reference
> Tom> +                 = !(last == DW_OP_stack_value || last == DW_OP_implicit_value);
> 
> I don't really understand the is_reference stuff

In case a dwarf expression is used for an DW_AT_location attribute, by
default it represents an address, and needs to be dereferenced to get
the value.

> but my first reaction
> is that it must be incorrect somehow.
> 
> 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.

So, I'm referring to this bit in the dwarf standard (v4):
...
2.6.1.1.3 Implicit Location Descriptions

An implicit location description represents a piece or all of an object
which has no actual location but whose contents are nonetheless either
known or known to be undefined.

The following DWARF operations may be used to specify a value that has
no location in the program but is a known constant or is computed from
other locations and values in the program.

   ...

2.DW_OP_stack_value

The DW_OP_stack_value operation specifies that the object does not exist
in memory but its value is nonetheless known and is at the top of the
DWARF expression stack. In this form of location description, the DWARF
expression represents the actual value of the object, rather than its
location. The DW_OP_stack_value operation terminates the expression.
...

AFAIU, the spec specifically says how to interpret a DW_OP_stack_value
at the end of the dwarf expression which is used a location description,
and the code in the patch follows that reasoning.

So, for my understanding, can you give an example of the problem you're
envisioning?

Thanks,
- Tom

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

[Tom Tromey]

>> 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.

Tom

[gdb/testsuite] Add clang xfail in gdb.base/vla-ptr.exp

[Tom de Vries]

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

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

On 11/20/20 4:15 PM, Tom de Vries wrote:
> [ was: Re: [PATCH] KFAIL variable-length array tests which fail with Clang ]
> 
> On 11/19/20 11:53 PM, Tom de Vries wrote:
>> On 11/18/20 5:14 PM, Tom Tromey wrote:
>>>>>>>> "Gary" == Gary Benson via Gdb-patches <gdb-patches@sourceware.org> writes:
>>>
>>> Gary> Clang describes the upper bounds of variable length arrays using
>>> Gary> a DW_AT_count attribute which references the DIE of a synthetic
>>> Gary> variable whose value is specified using a DW_AT_location.  In
>>> Gary> some cases GDB correctly handles these, but in other cases GDB
>>> Gary> adds an extra dereference causing the test to fail.  This commit
>>> Gary> marks these failing tests as caused by PR gdb/26905 when the
>>> Gary> test executable was built using Clang.
>>>
>>> Shouldn't we just fix the gdb bug instead?
>>
>> This adhoc patch:
> 
> Here's the complete and tested patch.
> 
> Any comments?
> 

Given the comments elsewhere in the thread, for now committing the
trivial testsuite part of the patch.

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-testsuite-Add-clang-xfail-in-gdb.base-vla-ptr.exp.patch --]
[-- Type: text/x-patch, Size: 3179 bytes --]

[gdb/testsuite] Add clang xfail in gdb.base/vla-ptr.exp

When running gdb.base/vla-ptr.exp with clang-10, we run into this FAIL:
...
(gdb) print td_vla^M
$6 = 0x7fffffffd2b0^M
(gdb) FAIL: gdb.base/vla-ptr.exp: print td_vla
...

Clang 10.0.1 generates the following DWARF for td_vla.  A variable DIE:
...
 <2><19f>: Abbrev Number: 6 (DW_TAG_variable)
    <1a0>   DW_AT_location    : 0x39 (location list)
    <1a4>   DW_AT_name        : td_vla
    <1aa>   DW_AT_type        : <0x1ae>
....
with typedef type:
...
 <2><1ae>: Abbrev Number: 7 (DW_TAG_typedef)
    <1af>   DW_AT_type        : <0x1fc>
    <1b3>   DW_AT_name        : typedef_vla
...
pointing to:
...
 <1><1fc>: Abbrev Number: 11 (DW_TAG_array_type)
    <1fd>   DW_AT_type        : <0x1d3>
 <2><201>: Abbrev Number: 14 (DW_TAG_subrange_type)
    <202>   DW_AT_type        : <0x1f5>
...

The subrange type is missing the count attribute.  This was filed as
llvm PR48247 - "vla var with typedef'd type has incomplete debug info".

Mark this as xfail.

gdb/testsuite/ChangeLog:

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

	* gdb.base/vla-ptr.exp: Add XFAIL.

---
 gdb/testsuite/gdb.base/vla-ptr.exp | 40 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/vla-ptr.exp b/gdb/testsuite/gdb.base/vla-ptr.exp
index 346ff00c524..23b3383903e 100644
--- a/gdb/testsuite/gdb.base/vla-ptr.exp
+++ b/gdb/testsuite/gdb.base/vla-ptr.exp
@@ -14,6 +14,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 standard_testfile
+set using_clang [test_compiler_info clang*]
 
 if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
     return -1
@@ -38,4 +39,41 @@ gdb_test "print *vla_ptr" " = 2" "print *vla_ptr (bar)"
 
 gdb_breakpoint [gdb_get_line_number "vla_func_bp"]
 gdb_continue_to_breakpoint "vla_func_bp"
-gdb_test "print td_vla" " = \\\{4, 5, 6, 7, 8\\\}"
+
+gdb_test_multiple "print td_vla" "" {
+    -re -wrap " = \\\{4, 5, 6, 7, 8\\\}" {
+	pass $gdb_test_name
+    }
+    -re -wrap " = $hex" {
+	if { $using_clang } {
+	    # Clang 10.0.1 fails to generate complete type info, filed as
+	    # llvm PR48247 - "vla var with typedef'd type has incomplete
+	    # debug info".  See note below.
+	    xfail $gdb_test_name
+	    # Verify that despite the incomplete type info, the variable is
+	    # there and has the right value.
+	    gdb_test "p *td_vla@5" " = \\\{4, 5, 6, 7, 8\\\}"
+	} else {
+	    fail $gdb_test_name
+	}
+    }
+}
+
+# Clang 10.0.1 generates this DWARF for td_vla:
+#
+# A variable DIE:
+# <2><19f>: Abbrev Number: 6 (DW_TAG_variable)
+#    <1a0>   DW_AT_location    : 0x39 (location list)
+#    <1a4>   DW_AT_name        : td_vla
+#    <1aa>   DW_AT_type        : <0x1ae>
+# with type:
+# <2><1ae>: Abbrev Number: 7 (DW_TAG_typedef)
+#    <1af>   DW_AT_type        : <0x1fc>
+#    <1b3>   DW_AT_name        : typedef_vla
+# pointing to:
+# <1><1fc>: Abbrev Number: 11 (DW_TAG_array_type)
+#    <1fd>   DW_AT_type        : <0x1d3>
+# <2><201>: Abbrev Number: 14 (DW_TAG_subrange_type)
+#    <202>   DW_AT_type        : <0x1f5>
+#
+# The subrange type is missing the count attribute.

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

[Tom de Vries]

[-- 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"

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

[Gary Benson via Gdb-patches]

Tom de Vries wrote:
> [ 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?

Thank you for picking this up Tom (de Vries).  Your patch looks good,
however I wanted to point out that the location expressions Clang
generates for gdb.base/vla-ptr.exp don't end with DW_OP_stack_value:

  < 2><0x000000a3>  DW_TAG_variable
                      DW_AT_location    len 0x0002: 9168: DW_OP_fbreg -24
		      DW_AT_name	__vla_expr0
		      DW_AT_type	<0x00000118>
		      DW_AT_artificial	yes(1)
  < 2><0x000000af>  DW_TAG_variable
		      DW_AT_location	len 0x0002: 9160: DW_OP_fbreg -32
		      DW_AT_name	__vla_expr1
		      DW_AT_type	<0x00000118>
		      DW_AT_artificial	yes(1)

It wasn't obvious to me how GDB with your patch would handle these.
Did you check your patch using that test?

Thanks,
Gary

-- 
Gary Benson - he / him / his
Principal Software Engineer, Red Hat

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

[Gary Benson via Gdb-patches]

Gary Benson wrote:
> Thank you for picking this up Tom (de Vries).  Your patch looks good,
> however I wanted to point out that the location expressions Clang
> generates for gdb.base/vla-ptr.exp don't end with DW_OP_stack_value:
> 
>   < 2><0x000000a3>  DW_TAG_variable
>                       DW_AT_location    len 0x0002: 9168: DW_OP_fbreg -24
>                       DW_AT_name      __vla_expr0
>                       DW_AT_type      <0x00000118>
>                       DW_AT_artificial        yes(1)
>   < 2><0x000000af>  DW_TAG_variable
>                       DW_AT_location  len 0x0002: 9160: DW_OP_fbreg -32
>                       DW_AT_name      __vla_expr1
>                       DW_AT_type      <0x00000118>
>                       DW_AT_artificial        yes(1)
> 
> It wasn't obvious to me how GDB with your patch would handle these.
> Did you check your patch using that test?

I forgot to add, that I did, and the final test failed with my setup:

  gdb.base/vla-ptr.exp
  ...
  print td_vla
  GCC compiled:       $6 = {4, 5, 6, 7, 8}
  Clang-12 compiled:  $6 = 0x7fffffffc0d0

But!  This is potentially a different issue.  I don't think that test
accesses the array's size, and I've definitely seen typedef-related
failures with Clang compared with GCC.

(I don't need you to debug this for me, I'm just making sure you're
aware!)

Thanks,
Gary

-- 
Gary Benson - he / him / his
Principal Software Engineer, Red Hat

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

[Tom de Vries]

On 11/25/20 4:25 PM, Gary Benson wrote:
> Gary Benson wrote:
>> Thank you for picking this up Tom (de Vries).  Your patch looks good,
>> however I wanted to point out that the location expressions Clang
>> generates for gdb.base/vla-ptr.exp don't end with DW_OP_stack_value:
>>
>>   < 2><0x000000a3>  DW_TAG_variable
>>                       DW_AT_location    len 0x0002: 9168: DW_OP_fbreg -24
>>                       DW_AT_name      __vla_expr0
>>                       DW_AT_type      <0x00000118>
>>                       DW_AT_artificial        yes(1)
>>   < 2><0x000000af>  DW_TAG_variable
>>                       DW_AT_location  len 0x0002: 9160: DW_OP_fbreg -32
>>                       DW_AT_name      __vla_expr1
>>                       DW_AT_type      <0x00000118>
>>                       DW_AT_artificial        yes(1)
>>
>> It wasn't obvious to me how GDB with your patch would handle these.
>> Did you check your patch using that test?
> 

Yes.

> I forgot to add, that I did, and the final test failed with my setup:
> 
>   gdb.base/vla-ptr.exp
>   ...
>   print td_vla
>   GCC compiled:       $6 = {4, 5, 6, 7, 8}
>   Clang-12 compiled:  $6 = 0x7fffffffc0d0
> 
> But!  This is potentially a different issue.  I don't think that test
> accesses the array's size, and I've definitely seen typedef-related
> failures with Clang compared with GCC.
> 
> (I don't need you to debug this for me, I'm just making sure you're
> aware!)

Yes, I've seen this, I've filed a clang PR (
https://bugs.llvm.org/show_bug.cgi?id=48247 ), then committed an xfail
in gdb (
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=da39d3ba57639b715dea7ce68b6a4a3d70dfcb03
).

Thanks,
- Tom

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

[Gary Benson via Gdb-patches]

Tom de Vries wrote:
> On 11/25/20 4:25 PM, Gary Benson wrote:
> > Gary Benson wrote:
> > > Thank you for picking this up Tom (de Vries).  Your patch looks good,
> > > however I wanted to point out that the location expressions Clang
> > > generates for gdb.base/vla-ptr.exp don't end with DW_OP_stack_value:
> > >
> > >   < 2><0x000000a3>  DW_TAG_variable
> > >                       DW_AT_location    len 0x0002: 9168: DW_OP_fbreg -24
> > >                       DW_AT_name      __vla_expr0
> > >                       DW_AT_type      <0x00000118>
> > >                       DW_AT_artificial        yes(1)
> > >   < 2><0x000000af>  DW_TAG_variable
> > >                       DW_AT_location  len 0x0002: 9160: DW_OP_fbreg -32
> > >                       DW_AT_name      __vla_expr1
> > >                       DW_AT_type      <0x00000118>
> > >                       DW_AT_artificial        yes(1)
> > >
> > > It wasn't obvious to me how GDB with your patch would handle these.
> > > Did you check your patch using that test?
> 
> Yes.
> 
> > I forgot to add, that I did, and the final test failed with my setup:
> > 
> >   gdb.base/vla-ptr.exp
> >   ...
> >   print td_vla
> >   GCC compiled:       $6 = {4, 5, 6, 7, 8}
> >   Clang-12 compiled:  $6 = 0x7fffffffc0d0
> > 
> > But!  This is potentially a different issue.  I don't think that test
> > accesses the array's size, and I've definitely seen typedef-related
> > failures with Clang compared with GCC.
> > 
> > (I don't need you to debug this for me, I'm just making sure you're
> > aware!)
> 
> Yes, I've seen this, I've filed a clang PR (
> https://bugs.llvm.org/show_bug.cgi?id=48247 ), then committed an xfail
> in gdb (
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=da39d3ba57639b715dea7ce68b6a4a3d70dfcb03
> ).

Amazing, thank you!

Cheers,
Gary

-- 
Gary Benson - he / him / his
Principal Software Engineer, Red Hat

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

[Tom de Vries]

On 11/23/20 11:34 AM, Tom de Vries wrote:
> [ 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?
> 

Committed.

Thanks,
- Tom