public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: Bruno Larsen <blarsen@redhat.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH 11/11] gdb/testsuite: disable gdb.cp/call-method-register.exp with clang
Date: Thu, 27 Oct 2022 10:49:10 +0100	[thread overview]
Message-ID: <87h6zp61gp.fsf@redhat.com> (raw)
In-Reply-To: <20221004170747.154307-13-blarsen@redhat.com>

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"


  reply	other threads:[~2022-10-27  9:49 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-04 17:07 [PATCH 00/11] Cleanup gdb.cp tests when running " Bruno Larsen
2022-10-04 17:07 ` [PATCH 01/11] gdb/testsuite: ignore Non-C-typedefs for gdb.cp/class2.exp Bruno Larsen
2022-10-26 12:02   ` Andrew Burgess
2022-10-27  8:37     ` Bruno Larsen
2022-10-28 11:37       ` Andrew Burgess
2022-10-04 17:07 ` [PATCH 02/11] gdb/testsuite: enable running gdb.cp/classes.exp with clang Bruno Larsen
2022-10-26 12:19   ` Andrew Burgess
2022-10-04 17:07 ` [PATCH 03/11] gdb/testsuite: account for clang's recursive destructor calls on gdb.cp/mb-ctor.exp Bruno Larsen
2022-10-26 12:35   ` Andrew Burgess
2022-10-04 17:07 ` [PATCH 4/9] gdb/testsuite: add XFAIL to gdb.cp/derivation.exp when using clang Bruno Larsen
2022-10-04 17:09   ` Bruno Larsen
2022-10-04 17:07 ` [PATCH 04/11] " Bruno Larsen
2022-10-26 12:37   ` Andrew Burgess
2022-10-26 14:50   ` Andrew Burgess
2022-10-04 17:07 ` [PATCH 05/11] gdb/testsuite: allow for clang style destructors on gdb.cp/m-static.exp Bruno Larsen
2022-10-26 13:04   ` Andrew Burgess
2022-10-26 14:51   ` Andrew Burgess
2022-10-04 17:07 ` [PATCH 06/11] gdb/testsuite: add XFAIL to gdb.cp/ptype-flags.exp when using clang Bruno Larsen
2022-10-26 14:08   ` Andrew Burgess
2022-10-27 13:17     ` Bruno Larsen
2022-10-28 11:38       ` Andrew Burgess
2022-10-31 12:45         ` Bruno Larsen
2022-10-04 17:07 ` [PATCH 07/11] gdb/testsuite: skip gdb.cp/anon-struct.exp " Bruno Larsen
2022-10-26 14:49   ` Andrew Burgess
2022-10-27 13:46     ` Bruno Larsen
2022-10-04 17:07 ` [PATCH 08/11] gdb/testsuite: disable gdb.cp/typeid.exp " Bruno Larsen
2022-10-26 15:37   ` Andrew Burgess
2022-10-04 17:07 ` [PATCH 09/11] gdb/testsuite: fix gdb.cp/converts.exp to run with clang Bruno Larsen
2022-10-26 15:54   ` Andrew Burgess
2022-10-31 12:46     ` Bruno Larsen
2022-10-04 17:07 ` [PATCH 10/11] gdb/testsuite: remove XFAIL on gdb.cp/temargs.exp Bruno Larsen
2022-10-26 15:57   ` Andrew Burgess
2022-10-04 17:07 ` [PATCH 11/11] gdb/testsuite: disable gdb.cp/call-method-register.exp with clang Bruno Larsen
2022-10-27  9:49   ` Andrew Burgess [this message]
2022-10-31 10:51     ` Bruno Larsen
2022-10-18  8:10 ` [PING][PATCH 00/11] Cleanup gdb.cp tests when running " Bruno Larsen

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=87h6zp61gp.fsf@redhat.com \
    --to=aburgess@redhat.com \
    --cc=blarsen@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).