From patchwork Thu Mar 22 23:01:36 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 26436 Received: (qmail 71714 invoked by alias); 22 Mar 2018 23:01:42 -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 71063 invoked by uid 89); 22 Mar 2018 23:01:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-wr0-f173.google.com Received: from mail-wr0-f173.google.com (HELO mail-wr0-f173.google.com) (209.85.128.173) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 22 Mar 2018 23:01:40 +0000 Received: by mail-wr0-f173.google.com with SMTP id 80so9238985wrb.2 for ; Thu, 22 Mar 2018 16:01:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=pOed3Aa7/Mi3Y4Q6vU0cbqJJ/Ey/awKMMM/lLvY6Kww=; b=l6GbSgfljl8cckmCs/6diGogC0x9uU2WZSVYNOT4Sj1hBIyanIcxIrOtgUUZyq8tD8 lXvd6mTinsmGmIoPEhMOtX9TA4pihRqfyM5a2IgS25AmTJwWoGzUfXgwJ/s3X763PCai i7ca7bzVrTKcmPmRkKBeIz6mvgoPAj5Rzy1cEKqZjLqNxXLZ4nUstoIytujQLvvLN6zB HlYH11xYNlVvsBr9keEyfQc6n+HIEpQRpxBkbWF5A4izu8TuEi/gQ/u5QaqlmpQ1RGJC DXJr/DjOGAxq9ZugqOfSvfOoIdZVDPlGyQYt3eA6rNSgbOh7fvwZiVwsmwlUvG7mZNqe 9wfQ== X-Gm-Message-State: AElRT7GeCRlApocrtVL4G02zrGC6rcuLS6ki9d029IVyf82EwhySl9K/ GbAxBVu2ue4sy9JqM09mCnvDNzJS X-Google-Smtp-Source: AG47ELv7qvkIUWPJUJv6u5wZGBUCDlo0NuqIZbpixdv9qAedi9vfRurvADl+1AbAAqEzHzdLTKwGZg== X-Received: by 10.223.225.2 with SMTP id d2mr12823694wri.56.1521759697904; Thu, 22 Mar 2018 16:01:37 -0700 (PDT) Received: from localhost (host86-177-103-167.range86-177.btcentralplus.com. [86.177.103.167]) by smtp.gmail.com with ESMTPSA id n29sm11333214wmi.32.2018.03.22.16.01.36 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 22 Mar 2018 16:01:37 -0700 (PDT) Date: Thu, 22 Mar 2018 23:01:36 +0000 From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Pedro Alves Subject: Re: [PATCH 2/2] gdb: Fix testsuite issue in gdb.arch/amd64-disp-step-avx.exp Message-ID: <20180322230136.GB13407@embecosm.com> References: <963bfa7c7483c4c907f4a63e9588b243845c8acd.1521722330.git.andrew.burgess@embecosm.com> <0a12132f-5423-4319-02da-4c61d072f237@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <0a12132f-5423-4319-02da-4c61d072f237@redhat.com> X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes * Pedro Alves [2018-03-22 13:44:51 +0000]: > Hi Andrew, > > Sounds fine to me in principle. A couple comments below. > > On 03/22/2018 12:57 PM, Andrew Burgess wrote: > > > @@ -103,8 +105,10 @@ proc disp_step_func { func } { > > with_test_prefix "vex2" { > > # This case writes to the 'xmm0' register. Confirm the register's > > # value is what we believe it is before the AVX instruction runs. > > - gdb_test "p /x \$xmm0.uint128" " = 0x0" \ > > - "xmm0 has expected value before" > > + for {set i 0 } { $i < 16 } { incr i } { > > + gdb_test "p /x \$xmm${i}.uint128" " = 0x0" \ > > + "xmm${i} has expected value before" > > + } > > This leaves a slight disconnect between the comment and the > code. I.e., someone reading the comment may wonder why > we check more than xmm0? > > Also, should we test xmm1-15 are still 0 after, too, for > completeness? > > > > > disp_step_func "test_rip_vex2" > > Thanks for the review. I've updated the patch inline with your feedback, the comment has been expanded to better match what we actually do, and we now confirm that xmm1 -> xmm15 are still 0 after the test instruction. Checked on x86-64 GNU/Linux and buildbot. Thanks, Andrew --- gdb: Fix testsuite issue in gdb.arch/amd64-disp-step-avx.exp This test starts up and confirms that $xmm0 has the value 0, it then modifies $xmm0 (in the inferior) and confirms that the new value can be read (in GDB). On some machines I was noticing that this test would occasionally fail, and on investigation I believe that the reason for this is that the test is linked as a dynamically linked executable and makes use of the system libraries during startup. The reason that this causes problems is that both the runtime linker and the startup code run before main can, and do (on at least some platforms) make use of the XMM registers. In this commit I modify the test program slightly to allow it to be linked statically, without using the startup libraries. Now by the time GDB reaches the symbol main we have only executed one 'nop' instruction, and the XMM registers should all have the value 0. I've extended the test script to confirm that $xmm0 to $xmm15 are all initially 0, and I also check that at the point after $xmm0 has been modified, all the other XMM registers ($xmm1 to $xmm15) are still 0. The test program is still linked against libc in order that we can call the exit function, however, we now call _exit rather than exit in order to avoid all of the usual cleanup that exit does. This clean up tries to tear down things that are usually setup during the startup code, but now this isn't called calling exit will just result in a crash. gdb/testsuite/ChangeLog: * gdb.arch/amd64-disp-step-avx.S: Add '_start' label. (done): Call '_exit' not 'exit' to avoid atexit handlers. * gdb.arch/amd64-disp-step-avx.exp: Pass -static, and -nostartfiles when compiling the test. Confirm that all registers xmm0 to xmm15 are initially 0, and that xmm1 to xmm15 are 0 after. --- gdb/testsuite/ChangeLog | 8 ++++++++ gdb/testsuite/gdb.arch/amd64-disp-step-avx.S | 8 +++++--- gdb/testsuite/gdb.arch/amd64-disp-step-avx.exp | 24 +++++++++++++++++++----- 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step-avx.S b/gdb/testsuite/gdb.arch/amd64-disp-step-avx.S index b56a552f6e6..1c06ceebab3 100644 --- a/gdb/testsuite/gdb.arch/amd64-disp-step-avx.S +++ b/gdb/testsuite/gdb.arch/amd64-disp-step-avx.S @@ -20,9 +20,11 @@ .text - .global main -main: + .global _start,main +_start: nop +main: + nop /***********************************************/ @@ -59,7 +61,7 @@ ro_var: done: mov $0,%rdi - call exit + call _exit hlt /* RIP-relative data for VEX3 test above. */ diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step-avx.exp b/gdb/testsuite/gdb.arch/amd64-disp-step-avx.exp index 1f85fa77c1a..fe6eb6729be 100644 --- a/gdb/testsuite/gdb.arch/amd64-disp-step-avx.exp +++ b/gdb/testsuite/gdb.arch/amd64-disp-step-avx.exp @@ -25,7 +25,10 @@ if { ![istarget x86_64-*-* ] || ![is_lp64_target] } { standard_testfile .S -if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } { +set options [list debug \ + additional_flags=-static \ + additional_flags=-nostartfiles] +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} $options] } { return -1 } @@ -100,10 +103,15 @@ proc disp_step_func { func } { # Test a VEX2-encoded RIP-relative instruction. with_test_prefix "vex2" { - # This case writes to the 'xmm0' register. Confirm the register's - # value is what we believe it is before the AVX instruction runs. - gdb_test "p /x \$xmm0.uint128" " = 0x0" \ - "xmm0 has expected value before" + # This test writes to the 'xmm0' register. As the test is + # statically linked, we know that the XMM registers should all + # have the default value of 0 at this point in time. We're about + # to run an AVX instruction that will modify $xmm0, but lets first + # confirm that all XMM registers are 0. + for {set i 0 } { $i < 16 } { incr i } { + gdb_test "p /x \$xmm${i}.uint128" " = 0x0" \ + "xmm${i} has expected value before" + } disp_step_func "test_rip_vex2" @@ -111,6 +119,12 @@ with_test_prefix "vex2" { # modified xmm0. gdb_test "p /x \$xmm0.uint128" " = 0x1122334455667788" \ "xmm0 has expected value after" + + # And all of the other XMM register should still be 0. + for {set i 1 } { $i < 16 } { incr i } { + gdb_test "p /x \$xmm${i}.uint128" " = 0x0" \ + "xmm${i} has expected value after" + } } # Test a VEX3-encoded RIP-relative instruction.