[11/11] gdb/testsuite: disable gdb.cp/call-method-register.exp with clang

  The test gdb.cp/call-method-register.exp assumes that the class will be
placed on a register. However, this keyword has been deprecated since
C++11, and clang does not feel the need to follow it. Since this test is
not usable without this working, this commit marks this test as
untested.
---
 gdb/testsuite/gdb.cp/call-method-register.exp | 5 +++++
 1 file changed, 5 insertions(+)
  

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

> The test gdb.cp/call-method-register.exp assumes that the class will be
> placed on a register. However, this keyword has been deprecated since
> C++11, and clang does not feel the need to follow it. Since this test is
> not usable without this working, this commit marks this test as
> untested.

As I understand it, the combination of register and asm, as used in the
test source is a GCC extension, and so...

> ---
>  gdb/testsuite/gdb.cp/call-method-register.exp | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/gdb/testsuite/gdb.cp/call-method-register.exp b/gdb/testsuite/gdb.cp/call-method-register.exp
> index a1e6498d66c..71d1f61f59f 100644
> --- a/gdb/testsuite/gdb.cp/call-method-register.exp
> +++ b/gdb/testsuite/gdb.cp/call-method-register.exp
> @@ -26,6 +26,11 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
>      return -1
>  }
>  
> +if {[test_compiler_info clang-*-*]} {
> +    untested "clang does not place the class in the register"
> +    return
> +}

... I think this would be better written as:

  if {![test_compiler_info gcc-*-* c++]} {
    untested "test relies on a gcc extension"
    return
  }

However, I also noticed that this test is only currently supported for
x86-64, i386, and ppc, as the test source itself needs a particular
register to be selected for each architecture.

I wondered if we could do better using the DWARF assembler.  Below is my
proposal that would replace this patch.  What do you think?

Thanks,
Andrew

---

commit 613e0a042c3220f183e02c9c3cf76c68b4d7e453
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Oct 27 10:15:09 2022 +0100

    gdb/testsuite: port gdb.cp/call-method-register.exp to DWARF assembler
    
    The test gdb.cp/call-method-register.exp relies on a GCC extension,
    that is combining the register keyword with the asm keyword to place a
    variable into a known register.
    
    This test already suffers from requiring each architecture to pick a
    register to use, currently the test only supports x86-64, i386, and
    ppc64.  Plus we know that the test will only ever work with GCC.
    
    In this commit I add a guard to the call-method-register.exp script so
    if the C++ compiler is not g++ then the test will be skipped.
    
    However, the main change in this commit is that I have added a
    complete new test gdb.dwarf2/dw2-call-method-register.exp, this is a
    copy of the original test but rewritten to use the DWARF assembler.
    I've tested the new test on x86-64, aarch64, and ppc64le.
    
    I did consider removing the original test, however, I thought there
    might be some value in retaining it, just so we can have some
    validation that GDB can correctly handle GCC's register extension, the
    test is pretty short, so doesn't take much time to run.

diff --git a/gdb/testsuite/gdb.cp/call-method-register.exp b/gdb/testsuite/gdb.cp/call-method-register.exp
index a1e6498d66c..2662d6b0891 100644
--- a/gdb/testsuite/gdb.cp/call-method-register.exp
+++ b/gdb/testsuite/gdb.cp/call-method-register.exp
@@ -18,6 +18,19 @@
 
 if { [skip_cplus_tests] } { continue }
 
+# This test relies on a GCC extension to place a structure into a
+# named register.  As a result this test will only work with GCC.  But
+# also, only a few architectures have a register named.  Any
+# architecture not explicitly supported in the source file will fail
+# to compile.
+#
+# However, the test gdb.dwarf2/dw2-call-method-register.exp is a
+# reimplementation of this test using the DWARF assembler, and should
+# work for any architecture, with any compiler (that supports the
+# DWARF assembler).  This test is retained mostly so we can track that
+# nothing weird happens w.r.t. how GDB handles GCC's register extension.
+if { ![test_compiler_info {gcc-*-*} c++] } { continue }
+
 load_lib "cp-support.exp"
 
 standard_testfile .cc
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.c b/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.c
new file mode 100644
index 00000000000..d5328156c55
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.c
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2022 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main ()
+{
+  asm ("main_label: .globl main_label");
+  return 0;
+}
+
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.exp b/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.exp
new file mode 100644
index 00000000000..3b761698076
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.exp
@@ -0,0 +1,127 @@
+# Copyright 2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test callling a method on a variable that has been put in a register.
+#
+# We use the DWARF assembler to generate DWARF that says the global variable
+# is located in a register.  We don't care which register we use as the
+# value in the register is not important, we only care that the DWARF says
+# the variable is in a register.  In fact, there is no variable in the
+# source program at all!
+
+load_lib dwarf.exp
+
+# Check the DWARF assembler is supported.
+if {![dwarf2_support]} { return }
+
+standard_testfile .c -dw.S
+
+# Compile and start the .c file so we can figure out pointer sizes.
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
+    return -1
+}
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+
+    # Get information about function main.
+    set main_result \
+	[function_range main ${::srcdir}/${::subdir}/${::srcfile}]
+    set main_start [lindex $main_result 0]
+    set main_length [lindex $main_result 1]
+
+    cu {} {
+	compile_unit {
+	    {DW_AT_language @DW_LANG_C_plus_plus}
+	    {DW_AT_name $::srcfile}
+	    {DW_AT_comp_dir /tmp}
+	} {
+	    declare_labels int_type_label struct_type_label \
+		struct_ptr_type_label
+            set ptr_size [get_sizeof "void *" 96]
+
+	    DW_TAG_subprogram {
+		{name main}
+		{low_pc $main_start addr}
+		{high_pc $main_length data8}
+		{DW_AT_type :$int_type_label}
+	    }
+
+	    int_type_label: DW_TAG_base_type {
+		{DW_AT_byte_size 4 DW_FORM_sdata}
+		{DW_AT_encoding @DW_ATE_signed}
+		{DW_AT_name int}
+	    }
+
+	    struct_type_label: DW_TAG_structure_type {
+		{DW_AT_byte_size 4 DW_FORM_sdata}
+		{DW_AT_name small}
+	    } {
+		member {
+		    {name xxx}
+		    {type :$int_type_label}
+		    {data_member_location 0 data1}
+		}
+		subprogram {
+		    {name yyy}
+		    {linkage_name _ZN5small3yyyEv}
+		    {external 1 flag_present}
+		    {type :$int_type_label}
+		    {data_member_location 0 data1}
+		} {
+		    formal_parameter {
+			{type :$struct_ptr_type_label}
+			{artificial 1 flag_present}
+		    }
+		}
+	    }
+
+	    struct_ptr_type_label: DW_TAG_pointer_type {
+		{DW_AT_byte_size $ptr_size DW_FORM_data1}
+		{type :$struct_type_label}
+	    }
+
+	    # This is where we place the variable into a register.  Just
+	    # use DWARF register 0, whatever that might be.  See the
+	    # comments at the start of the file for why we don't care
+	    # about the choice of register.
+            DW_TAG_variable {
+                {DW_AT_name global_var}
+                {DW_AT_type :$struct_type_label}
+                {DW_AT_location {
+                    DW_OP_reg0
+                } SPECIAL_expr}
+                {external 1 flag}
+            }
+	}
+    }
+}
+
+# Rebuild the test program, this time include the generated debug
+# information.
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+# Try to call a method on a variable of structure type, however, the
+# variable is located in a register.
+gdb_test "print global_var.yyy ()" \
+    "Address requested for identifier \"global_var\" which is in register .*" \
+    "call method on register local"
  

On 27/10/2022 11:49, Andrew Burgess wrote:
> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
>
>> The test gdb.cp/call-method-register.exp assumes that the class will be
>> placed on a register. However, this keyword has been deprecated since
>> C++11, and clang does not feel the need to follow it. Since this test is
>> not usable without this working, this commit marks this test as
>> untested.
> As I understand it, the combination of register and asm, as used in the
> test source is a GCC extension, and so...
>
>> ---
>>   gdb/testsuite/gdb.cp/call-method-register.exp | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/gdb/testsuite/gdb.cp/call-method-register.exp b/gdb/testsuite/gdb.cp/call-method-register.exp
>> index a1e6498d66c..71d1f61f59f 100644
>> --- a/gdb/testsuite/gdb.cp/call-method-register.exp
>> +++ b/gdb/testsuite/gdb.cp/call-method-register.exp
>> @@ -26,6 +26,11 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
>>       return -1
>>   }
>>   
>> +if {[test_compiler_info clang-*-*]} {
>> +    untested "clang does not place the class in the register"
>> +    return
>> +}
> ... I think this would be better written as:
>
>    if {![test_compiler_info gcc-*-* c++]} {
>      untested "test relies on a gcc extension"
>      return
>    }
>
> However, I also noticed that this test is only currently supported for
> x86-64, i386, and ppc, as the test source itself needs a particular
> register to be selected for each architecture.
>
> I wondered if we could do better using the DWARF assembler.  Below is my
> proposal that would replace this patch.  What do you think?
I did think about this, but it felt like it wasn't worth it, especially 
given that we can't use the dwarf assembler with clang. That said, I 
just tried your patch and it works just fine, so it is a pretty good 
improvement. I only have a very minor inlined.
>
> Thanks,
> Andrew
>
> ---
>
> commit 613e0a042c3220f183e02c9c3cf76c68b4d7e453
> Author: Andrew Burgess <aburgess@redhat.com>
> Date:   Thu Oct 27 10:15:09 2022 +0100
>
>      gdb/testsuite: port gdb.cp/call-method-register.exp to DWARF assembler
>      
>      The test gdb.cp/call-method-register.exp relies on a GCC extension,
>      that is combining the register keyword with the asm keyword to place a
>      variable into a known register.
>      
>      This test already suffers from requiring each architecture to pick a
>      register to use, currently the test only supports x86-64, i386, and
>      ppc64.  Plus we know that the test will only ever work with GCC.
>      
>      In this commit I add a guard to the call-method-register.exp script so
>      if the C++ compiler is not g++ then the test will be skipped.
>      
>      However, the main change in this commit is that I have added a
>      complete new test gdb.dwarf2/dw2-call-method-register.exp, this is a
>      copy of the original test but rewritten to use the DWARF assembler.
>      I've tested the new test on x86-64, aarch64, and ppc64le.
>      
>      I did consider removing the original test, however, I thought there
>      might be some value in retaining it, just so we can have some
>      validation that GDB can correctly handle GCC's register extension, the
>      test is pretty short, so doesn't take much time to run.
>
> diff --git a/gdb/testsuite/gdb.cp/call-method-register.exp b/gdb/testsuite/gdb.cp/call-method-register.exp
> index a1e6498d66c..2662d6b0891 100644
> --- a/gdb/testsuite/gdb.cp/call-method-register.exp
> +++ b/gdb/testsuite/gdb.cp/call-method-register.exp
> @@ -18,6 +18,19 @@
>   
>   if { [skip_cplus_tests] } { continue }
>   
> +# This test relies on a GCC extension to place a structure into a
> +# named register.  As a result this test will only work with GCC.  But
> +# also, only a few architectures have a register named.  Any
> +# architecture not explicitly supported in the source file will fail
> +# to compile.
> +#
> +# However, the test gdb.dwarf2/dw2-call-method-register.exp is a
> +# reimplementation of this test using the DWARF assembler, and should
> +# work for any architecture, with any compiler (that supports the
> +# DWARF assembler).  This test is retained mostly so we can track that
> +# nothing weird happens w.r.t. how GDB handles GCC's register extension.
> +if { ![test_compiler_info {gcc-*-*} c++] } { continue }
> +
>   load_lib "cp-support.exp"
>   
>   standard_testfile .cc
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.c b/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.c
> new file mode 100644
> index 00000000000..d5328156c55
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.c
> @@ -0,0 +1,24 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2022 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +int
> +main ()
> +{
> +  asm ("main_label: .globl main_label");
> +  return 0;
> +}
> +
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.exp b/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.exp
> new file mode 100644
> index 00000000000..3b761698076
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.exp
> @@ -0,0 +1,127 @@
> +# Copyright 2022 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Test callling a method on a variable that has been put in a register.
> +#
> +# We use the DWARF assembler to generate DWARF that says the global variable
> +# is located in a register.  We don't care which register we use as the
> +# value in the register is not important, we only care that the DWARF says
> +# the variable is in a register.  In fact, there is no variable in the
> +# source program at all!
> +
> +load_lib dwarf.exp
> +
> +# Check the DWARF assembler is supported.
> +if {![dwarf2_support]} { return }
> +
> +standard_testfile .c -dw.S
> +
> +# Compile and start the .c file so we can figure out pointer sizes.
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
> +    return -1
> +}
> +
> +set asm_file [standard_output_file $srcfile2]
> +Dwarf::assemble $asm_file {
> +
> +    # Get information about function main.
> +    set main_result \
> +	[function_range main ${::srcdir}/${::subdir}/${::srcfile}]
> +    set main_start [lindex $main_result 0]
> +    set main_length [lindex $main_result 1]
> +
> +    cu {} {
> +	compile_unit {
> +	    {DW_AT_language @DW_LANG_C_plus_plus}
> +	    {DW_AT_name $::srcfile}
> +	    {DW_AT_comp_dir /tmp}
> +	} {
> +	    declare_labels int_type_label struct_type_label \
> +		struct_ptr_type_label
> +            set ptr_size [get_sizeof "void *" 96]
> +
> +	    DW_TAG_subprogram {
> +		{name main}
> +		{low_pc $main_start addr}
> +		{high_pc $main_length data8}
> +		{DW_AT_type :$int_type_label}
> +	    }
> +
> +	    int_type_label: DW_TAG_base_type {
> +		{DW_AT_byte_size 4 DW_FORM_sdata}
> +		{DW_AT_encoding @DW_ATE_signed}
> +		{DW_AT_name int}
> +	    }
> +
> +	    struct_type_label: DW_TAG_structure_type {
> +		{DW_AT_byte_size 4 DW_FORM_sdata}
> +		{DW_AT_name small}
> +	    } {
> +		member {
> +		    {name xxx}
> +		    {type :$int_type_label}
> +		    {data_member_location 0 data1}
> +		}
> +		subprogram {
> +		    {name yyy}
> +		    {linkage_name _ZN5small3yyyEv}
Why did you decide to give the method a linkage name? It doesn't seem 
related to the error message and I think we should only leave important 
things in the tests.

Cheers,
Bruno

> +		    {external 1 flag_present}
> +		    {type :$int_type_label}
> +		    {data_member_location 0 data1}
> +		} {
> +		    formal_parameter {
> +			{type :$struct_ptr_type_label}
> +			{artificial 1 flag_present}
> +		    }
> +		}
> +	    }
> +
> +	    struct_ptr_type_label: DW_TAG_pointer_type {
> +		{DW_AT_byte_size $ptr_size DW_FORM_data1}
> +		{type :$struct_type_label}
> +	    }
> +
> +	    # This is where we place the variable into a register.  Just
> +	    # use DWARF register 0, whatever that might be.  See the
> +	    # comments at the start of the file for why we don't care
> +	    # about the choice of register.
> +            DW_TAG_variable {
> +                {DW_AT_name global_var}
> +                {DW_AT_type :$struct_type_label}
> +                {DW_AT_location {
> +                    DW_OP_reg0
> +                } SPECIAL_expr}
> +                {external 1 flag}
> +            }
> +	}
> +    }
> +}
> +
> +# Rebuild the test program, this time include the generated debug
> +# information.
> +if { [prepare_for_testing "failed to prepare" ${testfile} \
> +	  [list $srcfile $asm_file] {nodebug}] } {
> +    return -1
> +}
> +
> +if ![runto_main] {
> +    return -1
> +}
> +
> +# Try to call a method on a variable of structure type, however, the
> +# variable is located in a register.
> +gdb_test "print global_var.yyy ()" \
> +    "Address requested for identifier \"global_var\" which is in register .*" \
> +    "call method on register local"
>