From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29959 invoked by alias); 14 Nov 2018 16:58:45 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 29947 invoked by uid 89); 14 Nov 2018 16:58:44 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-6.9 required=5.0 tests=BAYES_00,GIT_PATCH_3,KAM_SHORT,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=stress, Pedro, 1134, H*M:518e X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 14 Nov 2018 16:58:42 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0CCE531524EF; Wed, 14 Nov 2018 16:58:41 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 03C376090D; Wed, 14 Nov 2018 16:58:39 +0000 (UTC) Subject: Re: [PATCH v4 3/3] Aarch64: Fix segfault when casting dummy calls To: Alan Hayward , "gdb-patches@sourceware.org" References: <20181031111754.64707-1-alan.hayward@arm.com> <20181031111754.64707-4-alan.hayward@arm.com> Cc: nd From: Pedro Alves Message-ID: <769330eb-0a85-518e-5677-4e665b04cad1@redhat.com> Date: Wed, 14 Nov 2018 16:58:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20181031111754.64707-4-alan.hayward@arm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-11/txt/msg00226.txt.bz2 On 10/31/2018 11:18 AM, Alan Hayward wrote: > +++ b/gdb/testsuite/gdb.base/infcall-across-obj.exp > @@ -0,0 +1,134 @@ > +# This testcase is part of GDB, the GNU debugger. > +# Copyright 2018 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 function calls on functions in another object. > +# See gdb/22736 Add missing period. This comment misses the most important aspect of the testcase/bugfix, which is to call a C++ function with no debug info. Please extend the comment to stress that. The "in another object" thing is really a red herring; there's nothing special about it, and there are other tests that cover that already. Putting the called-function in a separate object is just so that the main objfile is always compiled with debug info, so that GDB knows the program is a C++ program. (The only reason the infcall tests in gdb.base/nodebug.exp don't exercise this is that that testcase is C. That's why I had suggested gdb.cp/nodebug-infcall.exp previously. Anyway, naming not that important, I guess.) > + > +if [target_info exists gdb,cannot_call_functions] { > + unsupported "this target can not call functions" > + continue > +} > + > +# Only test c++ if we are able. Always use c Double space after period. Add missing period in second sentence. > +if { [skip_cplus_tests] || [get_compiler_info "c++"] } { > + set lang {c} > +} else { > + set lang {c c++} > +} > + > +set main_basename infcall-across-obj-main > +set lib_basename infcall-across-obj-lib > +standard_testfile ${main_basename}.c ${lib_basename}.c > + > +set mainsrc "${srcdir}/${subdir}/${srcfile}" > +set libsrc "${srcdir}/${subdir}/${srcfile2}" > + > +# Build both source files to objects using language LANG. Use SYMBOLS to build > +# the with either debug symbols or without - but always build the main file "build the with" is missing a word? > +# with debug. Then make function calls across the files. Double space after periods please, twice. > + > +proc build_and_run_test { lang symbols } { > + > + global main_basename lib_basename mainsrc libsrc binfile testfile > + global gdb_prompt > + > + if { $symbols == "debug" } { > + set debug_flags "debug" > + } else { > + set debug_flags "" > + } Mind space vs tabs above. Double check the rest of the patch too, please. > + > + # Setup directory. > + > + set dir "$lang-$symbols" > + remote_exec build "rm -rf [standard_output_file ${dir}]" > + remote_exec build "mkdir -p [standard_output_file ${dir}]" > + > + # Compile both files to objects, then link together. > + > + set main_flags "$lang debug" > + set lib_flags "$lang $debug_flags" > + set main_o [standard_output_file ${dir}/${main_basename}.o] > + set lib_o [standard_output_file ${dir}/${lib_basename}.o] > + set binfile [standard_output_file ${dir}/${testfile}] > + > + if { [gdb_compile $mainsrc $main_o object ${main_flags}] != "" } { > + untested "failed to compile main file to object" > + return -1 > + } > + > + if { [gdb_compile $libsrc $lib_o object ${lib_flags}] != "" } { > + untested "failed to compile secondary file to object" > + return -1 > + } > + > + if { [gdb_compile "$main_o $lib_o" ${binfile} executable ""] != "" } { > + untested "failed to compile" > + return -1 > + } > + > + # Startup and run to main. > + > + clean_restart $binfile > + > + if ![runto_main] then { > + fail "can't run to main" > + return > + } > + > + # Function call with cast. > + > + set test "p (int)foo()" > + gdb_test_multiple $test $test { > + -re " = 1\r\n$gdb_prompt " { > + pass $test > + } > + default { > + fail $test > + } > + } No need for the "default" case. gdb_test_multiple already issues a FAIL in that case. And if you remove that, a plain gdb_test instead should work: gdb_test "p (int)foo()" " = 1" Unless the missing $ anchor was on purpose? > + > + # Function call without cast. Will error if there are no debug symbols. Double-space after periods. > + > + set test "p foo()" > + gdb_test_multiple $test $test { > + -re " = 1\r\n$gdb_prompt " { > + if { $symbols == "debug" } { > + pass $test > + } else { > + fail $test > + } You can write gdb_assert { $symbols == "debug" } instead of the if/else. > + } > + -re "has unknown return type; cast the call to its declared return type\r\n$gdb_prompt " { > + if { $symbols != "debug" } { > + pass $test > + } else { > + fail $test > + } Ditto. > + } > + default { > + fail $test > + } Drop the default case. > + } > + > +} > + > +foreach_with_prefix l $lang { > + foreach_with_prefix s {debug nodebug} { > + build_and_run_test $l $s > + } > +} > + > OK with those fixed. Please post what you end up pushing. Thanks, Pedro Alves