From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 1F133385C322 for ; Mon, 31 Oct 2022 10:51:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1F133385C322 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1667213517; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=4dRdxGrC73sktotJfZwkREuOMGpZMOpBs15KS4y7uA8=; b=DzC+msUz1bVdh6UDajMLKyrPRlR/3F3aXLjycg9Axw5kDyLlANp4fnTebGCTc/5cukRaXZ Mw1wUmVrYg2qSqQlIP8z23J1IXjJ6U2/IBs52OzfNIFDaI/gWnzO0k/xzF5qGDxMr0VobW +jNe9KSWUCpml1hOrMsF1hHq3a/dq7M= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-553-Fn3jmNW7PLiSw2QrY1LXsw-1; Mon, 31 Oct 2022 06:51:56 -0400 X-MC-Unique: Fn3jmNW7PLiSw2QrY1LXsw-1 Received: by mail-qk1-f198.google.com with SMTP id ay43-20020a05620a17ab00b006fa30ed61fdso1337742qkb.5 for ; Mon, 31 Oct 2022 03:51:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=4dRdxGrC73sktotJfZwkREuOMGpZMOpBs15KS4y7uA8=; b=vo1i+x07rnn+X9K9eslP5mDMQrjGEAkKgfcBXyHJqTB1ZxPZBBfLr+8Pr/Dsgfxtb2 JWnQdFAc5GtjQm/OMgGdSMQZIJmKimHq2S06kI8RyYaGVk0DiFmbMSGx4jyv9mYYEQFm /0uqyz0e7Wr5te7IiZpkAcyBc717y9AKZHFyLMq8hQjzX9RPEbUcXaME0DB4veNqn+MB zEYEdQn7DXVWir/+Do4RbTDwHvZ1N6fTn4xHNfpKtAD/ALX0YspmM7X3zzeCaQ83Lmre PmldvLqsGvrJWVOKDG5Z1lweVgAq0RrpAiXK5PKH2zODxrHBzR7lU+W8mpIgC8kRkCK1 eQhg== X-Gm-Message-State: ACrzQf0lTS+kdSuqdHwAJxN0uBDov2qBIhcYmLWPvzCIfIC4eiQUiZxn yVhGNBPM7Kq9ZNzCDEaP5FsoDwvLk1i55vmMEYaRUw0c1cs+5tjSfx40L4tXBiahzP3p9SodIUN 6BJSjhf5JUfhFeLrvWdypDg== X-Received: by 2002:a37:688:0:b0:6f8:77da:87e7 with SMTP id 130-20020a370688000000b006f877da87e7mr8459541qkg.105.1667213514959; Mon, 31 Oct 2022 03:51:54 -0700 (PDT) X-Google-Smtp-Source: AMsMyM6QjKbWYwPlVMnbDrSwue1RZrQbxxrVxM4u4CDm/ZRumekDGFSK0H6O71jC7Osp5c90yLqkPA== X-Received: by 2002:a37:688:0:b0:6f8:77da:87e7 with SMTP id 130-20020a370688000000b006f877da87e7mr8459529qkg.105.1667213514637; Mon, 31 Oct 2022 03:51:54 -0700 (PDT) Received: from [10.43.2.105] (nat-pool-brq-t.redhat.com. [213.175.37.10]) by smtp.gmail.com with ESMTPSA id gd26-20020a05622a5c1a00b0039764587192sm3445600qtb.57.2022.10.31.03.51.53 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 31 Oct 2022 03:51:53 -0700 (PDT) Message-ID: <7f1128e0-bf74-40b7-11f6-7f1a20ffbbdc@redhat.com> Date: Mon, 31 Oct 2022 11:51:52 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.0 Subject: Re: [PATCH 11/11] gdb/testsuite: disable gdb.cp/call-method-register.exp with clang To: Andrew Burgess , gdb-patches@sourceware.org References: <20221004170747.154307-1-blarsen@redhat.com> <20221004170747.154307-13-blarsen@redhat.com> <87h6zp61gp.fsf@redhat.com> From: Bruno Larsen In-Reply-To: <87h6zp61gp.fsf@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 27/10/2022 11:49, Andrew Burgess wrote: > Bruno Larsen via Gdb-patches 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 > 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 . */ > + > +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 . > + > +# 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" >