public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gdb/testsuite: use proper int size for gdb.dwarf2/symbol_needs_eval*.exp
@ 2023-06-08 15:38 Lancelot SIX
  2023-06-08 21:13 ` Six, Lancelot
  0 siblings, 1 reply; 2+ messages in thread
From: Lancelot SIX @ 2023-06-08 15:38 UTC (permalink / raw)
  To: gdb-patches; +Cc: lsix, Lancelot SIX

We recently realized that symbol_needs_eval_fail.exp and
symbol_needs_eval_timeout.exp invalidly dereference an int (4 bytes on
x86_64) by reading 8 bytes (the size of a pointer).

Here how it goes:

In gdb/testsuite/gdb.dwarf2/symbol_needs_eval.c a global variable is
defined:

    int exec_mask = 1;

and later both tests build some DWARF using the assembler doing:

    set exec_mask_var [gdb_target_symbol exec_mask]
    ...
        DW_TAG_variable {
          {DW_AT_name a}
          {DW_AT_type :$int_type_label}
          {DW_AT_location {
            DW_OP_addr $exec_mask_var
            DW_OP_deref
            ...
          }
        }

The definition of the DW_OP_deref (from Dwarf5 2.5.1.3 Stack Operations)
says that "The size of the data retrieved from the dereferenced address
is the size of an address on the target machine."

On x86_64, the size of an int is 4 while the size of an address is 8.
The result is that when evaluating this expression, the debugger reads
outside of the `a` variable.

Fix this by using `DW_OP_deref_size $int_size` instead.  To achieve
this, this patch adds the necessary steps so we can figure out what
`sizeof(int)` evaluates to for the current target.

While at it, also change the definition of the int type in the assembled
DWARF information so we use the actual target's size for an int instead
of the literal 4.

Tested on x86_64 Linux.
---
 .../gdb.dwarf2/symbol_needs_eval_fail.exp          | 14 +++++++++++---
 .../gdb.dwarf2/symbol_needs_eval_timeout.exp       | 14 +++++++++++---
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_fail.exp b/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_fail.exp
index cb9826e90df..c3440d970dc 100644
--- a/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_fail.exp
+++ b/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_fail.exp
@@ -47,11 +47,19 @@ if { [is_aarch64_target] } {
 
 standard_testfile symbol_needs_eval.c ${gdb_test_file_name}-dw.S
 
+if [prepare_for_testing "failed to prepare" $testfile $srcfile {debug}] {
+    return
+}
+
+clean_restart ${testfile}
+
+set int_size [get_sizeof "int" -1]
+
 # Make some DWARF for the test.
 
 set asm_file [standard_output_file $srcfile2]
 Dwarf::assemble $asm_file {
-    global dwarf_regnum regname
+    global dwarf_regnum regname int_size
 
     set exec_mask_var [gdb_target_symbol exec_mask]
 
@@ -66,7 +74,7 @@ Dwarf::assemble $asm_file {
 	    int_type_label: DW_TAG_base_type {
 		{DW_AT_name "int"}
 		{DW_AT_encoding @DW_ATE_signed}
-		{DW_AT_byte_size 4 DW_FORM_sdata}
+		{DW_AT_byte_size $int_size DW_FORM_sdata}
 	    }
 
 	    # define artificial variable a
@@ -75,7 +83,7 @@ Dwarf::assemble $asm_file {
 		{DW_AT_type :$int_type_label}
 		{DW_AT_location {
 		    DW_OP_addr $exec_mask_var
-		    DW_OP_deref
+		    DW_OP_deref_size $int_size
 
 		    # conditional jump to DW_OP_bregx
 		    DW_OP_bra 4
diff --git a/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_timeout.exp b/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_timeout.exp
index 0e6ee6112ea..ecdeb62a1a8 100644
--- a/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_timeout.exp
+++ b/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_timeout.exp
@@ -47,11 +47,19 @@ if { [is_aarch64_target] } {
 
 standard_testfile symbol_needs_eval.c ${gdb_test_file_name}-dw.S
 
+if [prepare_for_testing "failed to prepare" $testfile $srcfile {debug}] {
+    return
+}
+
+clean_restart ${testfile}
+
+set int_size [get_sizeof "int" -1]
+
 # Make some DWARF for the test.
 
 set asm_file [standard_output_file $srcfile2]
 Dwarf::assemble $asm_file {
-    global dwarf_regnum regname
+    global dwarf_regnum regname int_size
 
     set exec_mask_var [gdb_target_symbol exec_mask]
 
@@ -66,7 +74,7 @@ Dwarf::assemble $asm_file {
 	    int_type_label: DW_TAG_base_type {
 		{DW_AT_name "int"}
 		{DW_AT_encoding @DW_ATE_signed}
-		{DW_AT_byte_size 4 DW_FORM_sdata}
+		{DW_AT_byte_size $int_size DW_FORM_sdata}
 	    }
 
 	    # add info for variable exec_mask
@@ -93,7 +101,7 @@ Dwarf::assemble $asm_file {
 		    {DW_AT_location {
 			DW_OP_lit1
 			DW_OP_addr $exec_mask_var
-			DW_OP_deref
+			DW_OP_deref_size $int_size
 
 			# jump to DW_OP_fbreg
 			DW_OP_skip 4

base-commit: 12f7174bf069f407d5b6f12e926ceabe45e450e1
-- 
2.34.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

* RE: [PATCH] gdb/testsuite: use proper int size for gdb.dwarf2/symbol_needs_eval*.exp
  2023-06-08 15:38 [PATCH] gdb/testsuite: use proper int size for gdb.dwarf2/symbol_needs_eval*.exp Lancelot SIX
@ 2023-06-08 21:13 ` Six, Lancelot
  0 siblings, 0 replies; 2+ messages in thread
From: Six, Lancelot @ 2023-06-08 21:13 UTC (permalink / raw)
  To: gdb-patches, Six, Lancelot; +Cc: lsix

[AMD Official Use Only - General]

Replying to myself.

> diff --git a/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_fail.exp
> b/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_fail.exp
> index cb9826e90df..c3440d970dc 100644
> --- a/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_fail.exp
> +++ b/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_fail.exp
> @@ -47,11 +47,19 @@ if { [is_aarch64_target] } {
>
>  standard_testfile symbol_needs_eval.c ${gdb_test_file_name}-dw.S
>
> +if [prepare_for_testing "failed to prepare" $testfile $srcfile {debug}] {
> +    return
> +}
> +
> +clean_restart ${testfile}

The clean_restart is not needed here as this is the last thing prepare_for_testing does.

> diff --git a/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_timeout.exp
> b/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_timeout.exp
> index 0e6ee6112ea..ecdeb62a1a8 100644
> --- a/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_timeout.exp
> +++ b/gdb/testsuite/gdb.dwarf2/symbol_needs_eval_timeout.exp
> @@ -47,11 +47,19 @@ if { [is_aarch64_target] } {
>
>  standard_testfile symbol_needs_eval.c ${gdb_test_file_name}-dw.S
>
> +if [prepare_for_testing "failed to prepare" $testfile $srcfile {debug}] {
> +    return
> +}
> +
> +clean_restart ${testfile}

Same here.

I'll prepare a V2 removing those 2 lines.

Best,
Lancelot.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-06-08 21:13 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-08 15:38 [PATCH] gdb/testsuite: use proper int size for gdb.dwarf2/symbol_needs_eval*.exp Lancelot SIX
2023-06-08 21:13 ` Six, Lancelot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).