From patchwork Mon Oct 9 17:09:16 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 23416 Received: (qmail 4724 invoked by alias); 9 Oct 2017 17:09:21 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 4710 invoked by uid 89); 9 Oct 2017 17:09:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=locals, TCL, tcl 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; Mon, 09 Oct 2017 17:09:19 +0000 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 10640C04AC67; Mon, 9 Oct 2017 17:09:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 10640C04AC67 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves@redhat.com 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 3DF6E675D0; Mon, 9 Oct 2017 17:09:17 +0000 (UTC) Subject: Re: [PATCH 2/2] gdb.multi/multi-arch-exec.exp: Also test -m32 => -m64 To: Simon Marchi , gdb-patches@sourceware.org References: <1507544177-24880-1-git-send-email-palves@redhat.com> <1507544177-24880-3-git-send-email-palves@redhat.com> <3bed052d-aa37-eaca-99af-c05fdd4128bd@simark.ca> From: Pedro Alves Message-ID: <8835f711-8618-239b-ed2a-9eed9592ab2a@redhat.com> Date: Mon, 9 Oct 2017 18:09:16 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <3bed052d-aa37-eaca-99af-c05fdd4128bd@simark.ca> On 10/09/2017 03:02 PM, Simon Marchi wrote: >> +proc build_executables { first_arch } { >> + >> + # Can't use standard_testfile, we want executables with specialized >> + # names. >> + set exec1 "$first_arch-multi-arch-exec" >> + set srcfile1 multi-arch-exec.c >> + set binfile1 [standard_output_file ${exec1}] >> + >> + set exec2 "$first_arch-multi-arch-exec-hello" >> + set srcfile2 hello.c >> + set binfile2 [standard_output_file ${exec2}] >> + >> + # Build two executables, one for each arch. >> + >> + if {$first_arch == 1} { >> + set arch1 1 >> + set arch2 2 >> + } elseif {$first_arch == 2} { >> + set arch1 2 >> + set arch2 1 >> + } else { >> + error "unhandled first architecture: $first_arch" >> + } > > I would rename arch1/arch2 to arch_from/arch_to. 1/2 is already used > to name the two architectures, so it's confusing if it's also used to > mean from/to. Good idea, I like that. In that case, I'm renaming the other locals to match as well (exec1 -> from_exec, etc.). >> + >> + set options [list debug] >> + if {![append_arch_options $arch2 options]} { > > append_arch_options doesn't explicitly return something. Is the result of the > last command executed in the procedure returned implicitly? Exactly, in TCL, a procedure implicitly returns the result of the last command executed. Since that can create confusion, I've added the explicit returns now. > Otherwise, LGTM. Great, here's what I'm pushing in in a bit. ----------------- gdb.multi/multi-arch-exec.exp: Also test -m32 => -m64 The gdb.multi/multi-arch-exec.exp testcase currently tests execing from -m64 to -m32, but does not test the other direction. For thoroughness, this commit fixes that. Without the fix in the previous commit for example ("Multi-arch exec, more register reading avoidance"), on x86_64 we would get different symptoms depending on "execing direction". Vis: Continuing. Truncated register 50 in remote 'g' packet Truncated register 50 in remote 'g' packet (gdb) FAIL: gdb.multi/multi-arch-exec.exp: first_arch=1: selected_thread=2: follow_exec_mode=same: continue across exec that changes architecture Vs: Continuing. Remote 'g' packet reply is too long (expected 440 bytes, got 816 bytes): daffffffffffffff0000[snip] Remote 'g' packet reply is too long (expected 440 bytes, got 816 bytes): daffffffffffffff0000[snip] (gdb) FAIL: gdb.multi/multi-arch-exec.exp: first_arch=2: selected_thread=2: follow_exec_mode=same: continue across exec that changes architecture gdb/testsuite/ChangeLog: yyyy-mm-dd Pedro Alves Test both arch1=>arch2 and arch2=>arch1. * gdb.multi/multi-arch-exec.exp (exec1, srcfile1, binfile1, exec2) (srcfile2, binfile2, march1, march2): Remove globals. Largely factored out to... (append_arch1_options, append_arch2_options, append_arch_options) (build_executables): New procedures. (do_test): New 'first_arch' parameter. Use it to define 'from_exec' local. (top level): Add new 'first_arch' testing axis. --- gdb/testsuite/gdb.multi/multi-arch-exec.exp | 168 ++++++++++++++++++++------- 1 file changed, 122 insertions(+), 46 deletions(-) diff --git a/gdb/testsuite/gdb.multi/multi-arch-exec.exp b/gdb/testsuite/gdb.multi/multi-arch-exec.exp index 3196cf3..53e1280 100644 --- a/gdb/testsuite/gdb.multi/multi-arch-exec.exp +++ b/gdb/testsuite/gdb.multi/multi-arch-exec.exp @@ -29,57 +29,126 @@ if [istarget "i?86-*linux*"] { return } -# Can't use standard_testfile, we want executables with specialized -# names. -set exec1 "multi-arch-exec" -set srcfile1 multi-arch-exec.c -set binfile1 [standard_output_file ${exec1}] - -set exec2 "multi-arch-exec-hello" -set srcfile2 hello.c -set binfile2 [standard_output_file ${exec2}] - -# Build two executables, one for each arch. - -if [istarget "s390*-*-*"] { - set march1 "-m64" - set march2 "-m31" -} elseif { [istarget "aarch64*-*-*"] } { - set march1 "" - set march2 "" -} else { - set march1 "-m64" - set march2 "-m32" +# The testcase builds two programs, each of its own architecture. For +# example, one built with -m64, another with -m32. The exact compiler +# options depends on target triplet. We generically refer to the +# architectures simply as 'architecture 1' and 'architecture 2'. Each +# program is actually built twice, once for each architecture, because +# we test both execing from arch1 to arch2 and from arch2 to arch1. +# The architecture of the executable that execs is encoded in the +# binaries' names, like so: +# +# $first_arch-multi-arch-exec # execing program +# $first_arch-multi-arch-exec-hello # execed program + +# Append the options necessary to build a program for architecture 1 +# to the OPTIONS_VAR list. + +proc append_arch1_options {options_var} { + upvar 1 $options_var options + + if { [istarget "aarch64*-*-*"] } { + return 1 + } + + lappend options "additional_flags=-m64" + return 1 } -if { [prepare_for_testing "failed to prepare" ${exec1} "${srcfile1}" \ - [list debug pthreads \ - additional_flags=${march1}]] } { - return -1 +# Append the options necessary to build a program for architecture 2 +# to the OPTIONS_VAR list. + +proc append_arch2_options {options_var} { + upvar 1 $options_var options + + if { [istarget "aarch64*-*-*"] } { + if {[info exists ARM_CC_FOR_TARGET]} { + lappend options "compiler=${ARM_CC_FOR_TARGET}" + return 1 + } else { + unsupported "ARM compiler is not known" + return 0 + } + } + + if [istarget "s390*-*-*"] { + set march "-m31" + } else { + set march "-m32" + } + lappend options "additional_flags=${march}" + return 1 } -set options [list debug] +# Append the options necessary to build a program for architecture +# ARCH to the OPTIONS_VAR list. Returns true on success. -if { [istarget "aarch64*-*-*"] } { - if {[info exists ARM_CC_FOR_TARGET]} { - lappend options "compiler=${ARM_CC_FOR_TARGET}" +proc append_arch_options {arch options_var} { + upvar 1 $options_var options + + if {$arch == 1} { + return [append_arch1_options options] + } elseif {$arch == 2} { + return [append_arch2_options options] } else { - unsupported "ARM compiler is not known" - return -1 + error "unhandled architecture: $arch" } -} else { - lappend options "additional_flags=${march2}" } -if { [prepare_for_testing "failed to prepare" ${exec2} "${srcfile2}" \ - $options] } { - return -1 +# Build the executables for testing with FIRST_ARCH (either 1 or 2) as +# the architecture before the exec. Returns true on success. + +proc build_executables { first_arch } { + + # Can't use standard_testfile, we want executables with specialized + # names. + set from_exec "$first_arch-multi-arch-exec" + set from_srcfile multi-arch-exec.c + set from_binfile [standard_output_file ${from_exec}] + + set to_exec "$first_arch-multi-arch-exec-hello" + set to_srcfile hello.c + set to_binfile [standard_output_file ${to_exec}] + + # Build two executables, one for each arch. + + if {$first_arch == 1} { + set from_arch 1 + set to_arch 2 + } elseif {$first_arch == 2} { + set from_arch 2 + set to_arch 1 + } else { + error "unhandled first architecture: $first_arch" + } + + set from_options [list debug pthreads] + if {![append_arch_options $from_arch from_options]} { + return 0 + } + + if { [build_executable "failed to prepare" ${from_exec} "${from_srcfile}" \ + $from_options] } { + return 0 + } + + set to_options [list debug] + if {![append_arch_options $to_arch to_options]} { + return 0 + } + + if { [build_executable "failed to prepare" ${to_exec} "${to_srcfile}" \ + $to_options] } { + return 0 + } + + return 1 } -proc do_test { mode selected_thread } { - global exec1 +proc do_test { first_arch mode selected_thread } { + set from_exec "$first_arch-multi-arch-exec" - clean_restart ${exec1} + clean_restart ${from_exec} if ![runto all_started] then { fail "couldn't run to all_started" return -1 @@ -102,12 +171,19 @@ proc do_test { mode selected_thread } { gdb_test "continue" "Breakpoint 2, main.*" "continue across exec that changes architecture" } -# Test handling the exec event with either the main thread or the -# second thread selected. This tries to ensure that GDB doesn't read -# registers off of the execing thread before figuring out its -# architecture. -foreach_with_prefix selected_thread {1 2} { - foreach_with_prefix follow_exec_mode {"same" "new"} { - do_test $follow_exec_mode $selected_thread +# Test both arch1=>arch2 and arch2=>arch1. +foreach_with_prefix first_arch {1 2} { + if {![build_executables $first_arch]} { + continue + } + + # Test handling the exec event with either the main thread or the + # second thread selected. This tries to ensure that GDB doesn't read + # registers off of the execing thread before figuring out its + # architecture. + foreach_with_prefix selected_thread {1 2} { + foreach_with_prefix follow_exec_mode {"same" "new"} { + do_test $first_arch $follow_exec_mode $selected_thread + } } }