Re: [PATCH v3 04/14] change gdb.base/nodebug.c to not fail with clang

[Andrew Burgess <aburgess@redhat.com>]

Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:

> Clang organizes the variables differently to gcc in the original version
> of this code, leading to the following differences when testing
> p (int*) &dataglobal + 1
>
> gcc:
> $16 = (int *) 0x404034 <datalocal>
>
> clang:
> $16 = (int *) 0x404034 <dataglobal8>
>
> The code change to nodebug.c makes it so gcc and clang will place the
> same variable under dataglobal8, generating the same output.
> ---
>
> No change in v3.
>
> No change in v2.
>
> ---
>  gdb/testsuite/gdb.base/nodebug.c   | 2 +-
>  gdb/testsuite/gdb.base/nodebug.exp | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/nodebug.c b/gdb/testsuite/gdb.base/nodebug.c
> index c7bc93991b8..572255697bb 100644
> --- a/gdb/testsuite/gdb.base/nodebug.c
> +++ b/gdb/testsuite/gdb.base/nodebug.c
> @@ -3,8 +3,8 @@
>  
>  /* Test that things still (sort of) work when compiled without -g.  */
>  
> -int dataglobal = 3;			/* Should go in global data */
>  static int datalocal = 4;		/* Should go in local data */
> +int dataglobal = 3;			/* Should go in global data */
>  int bssglobal;				/* Should go in global bss */
>  static int bsslocal;			/* Should go in local bss */

Something about just reordering the symbols like this makes me a little
uncomfortable.

I spent some time thinking about this test, wondering what the point of
the check was.  The test was added in commit:

  commit 46a4882b3c7d9ec981568b8b13a3c9c39c8f8e61
  Date:   Mon Sep 4 20:21:15 2017 +0100
  
      Stop assuming no-debug-info variables have type int

But there's no hint what this specific test was for.

My best guess is that we're checking that the '+ 1' correctly increases
the address by the size of an int.  With that in mind, I wonder if the
label after the address is really that important.  I mean, right not it
kind-of acts to confirm that we increased the address by the correct
amount, but, we could achieve that same goal by just comparing
addresses.

And so, I wonder about the patch below instead?  Instead of looking for
a specific label I now accept any label.  I then add an extra test
afterwards that checks that '+ 1' increases the address by the size of
an int.

What do you think of this?

Thanks,
Andrew

---

diff --git a/gdb/testsuite/gdb.base/nodebug.exp b/gdb/testsuite/gdb.base/nodebug.exp
index 0dbd0ad153e..a4db483d6af 100644
--- a/gdb/testsuite/gdb.base/nodebug.exp
+++ b/gdb/testsuite/gdb.base/nodebug.exp
@@ -174,6 +174,7 @@ if [nodebug_runto inner] then {
     set unk_type_re "has unknown type.*to its declared type"
     set ptr_math_re "Cannot perform pointer math on incomplete type \"$data_var_type\", try casting to a known type, or void \\*\\."
     set not_mem_re "Attempt to take address of value not located in memory\\."
+    set any_label_regexp "<\[^>\]+>"
 
     set dataglobal_unk_re "dataglobal.*$unk_type_re"
 
@@ -187,7 +188,7 @@ if [nodebug_runto inner] then {
 	{"dataglobal + 1"		""   $dataglobal_unk_re					$dataglobal_unk_re}
 	{"&dataglobal"			""   "\\($data_var_type \\*\\) $hex <dataglobal>"	" = $data_var_type \\*"}
 	{"&dataglobal + 1"		""   $ptr_math_re					$ptr_math_re}
-	{"(int *) &dataglobal + 1"	""   " = \\(int \\*\\) $hex <datalocal>"		"int \\*"}
+	{"(int *) &dataglobal + 1"	""   " = \\(int \\*\\) $hex $any_label_regexp"		"int \\*"}
 	{"&(int) dataglobal + 1"	""   $not_mem_re					$not_mem_re}
 	{"&dataglobal, &dataglobal"	""   "\\($data_var_type \\*\\) $hex <dataglobal>"	" = $data_var_type \\*"}
 	{"*dataglobal"			""   $dataglobal_unk_re					$dataglobal_unk_re}
@@ -218,7 +219,14 @@ if [nodebug_runto inner] then {
 	gdb_test "whatis $exp" $whatis
 	gdb_test "ptype $exp" $whatis
     }
-    
+
+    # Check that pointer arithmetic works as expected.
+    set addr1 [get_hexadecimal_valueof "&dataglobal" "*UNKNOWN*"]
+    set addr2 [get_hexadecimal_valueof "(int *) &dataglobal + 1" "*UNKNOWN*"]
+    set offset [expr $addr2 - $addr1]
+    set int_size [get_integer_valueof "sizeof (int)" "*UNKNOWN*"]
+    gdb_assert { $offset == $int_size }
+
     # The only symbol xcoff puts out for statics is for the TOC entry.
     # Possible, but hairy, for gdb to deal.  Right now it doesn't, it
     # doesn't know the variables exist at all.