Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Tom de Vries <tdevries@suse.de>
To: Tom Tromey <tom@tromey.com>,
	Gary Benson via Gdb-patches <gdb-patches@sourceware.org>
Subject: [PATCH][gdb/symtab] Fix gdb.base/vla-optimized-out.exp with clang
Date: Fri, 20 Nov 2020 16:15:12 +0100	[thread overview]
Message-ID: <3fe3ebd1-835d-b603-5cb1-254364731ec2@suse.de> (raw)
In-Reply-To: <0a3f590c-2a76-e470-136f-e8f88aef1d17@suse.de>

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

  reply	other threads:[~2020-11-20 15:15 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-16 17:17 [PATCH] KFAIL variable-length array tests which fail with Clang Gary Benson via Gdb-patches
2020-11-18 16:14 ` Tom Tromey
2020-11-19 22:53   ` Tom de Vries
2020-11-20 15:15     ` Tom de Vries [this message]
2020-11-21 17:16       ` [gdb/testsuite] Add clang xfail in gdb.base/vla-ptr.exp Tom de Vries
2020-11-20 15:50     ` [PATCH] KFAIL variable-length array tests which fail with Clang Tom Tromey
2020-11-20 16:30       ` Tom de Vries
2020-11-20 16:51         ` Tom Tromey
2020-11-23 10:34           ` [PATCH][gdb/symtab] Fix gdb.base/vla-optimized-out.exp with clang Tom de Vries
2020-11-25 14:50             ` Gary Benson via Gdb-patches
2020-11-25 15:25               ` Gary Benson via Gdb-patches
2020-11-25 20:05                 ` Tom de Vries
2020-11-26 10:10                   ` Gary Benson via Gdb-patches
2020-11-30 12:51             ` [committed][PATCH][gdb/symtab] " Tom de Vries

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3fe3ebd1-835d-b603-5cb1-254364731ec2@suse.de \
    --to=tdevries@suse.de \
    --cc=gdb-patches@sourceware.org \
    --cc=tom@tromey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox