From patchwork Thu Aug 28 21:58:24 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Evans X-Patchwork-Id: 2578 Received: (qmail 5787 invoked by alias); 28 Aug 2014 21:58:29 -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 5775 invoked by uid 89); 28 Aug 2014 21:58:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS, UNSUBSCRIBE_BODY autolearn=no version=3.3.2 X-HELO: mail-oi0-f74.google.com Received: from mail-oi0-f74.google.com (HELO mail-oi0-f74.google.com) (209.85.218.74) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 28 Aug 2014 21:58:27 +0000 Received: by mail-oi0-f74.google.com with SMTP id v63so333050oia.1 for ; Thu, 28 Aug 2014 14:58:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:mime-version:content-type :content-transfer-encoding:message-id:date:to:subject:in-reply-to :references; bh=DFBIludAMNdJcssoJeeM3Uf23PZV4wIsAd7Lsr84wbg=; b=f1pbx67Yo5ozQKvdIBVjI+HF0OTKbaR5luT5Cc/aDwp0yGZLL7/qZHxbPtLF+tf9yV g8LLhRnxudK4WIa/JbrFBPFok+kJayfAwzonh+zijP7KNCyLQ61i5sh1BnVobekJpU55 EZdcM+GjCGHIUIu+xhr5PLZAXdNvO/0fFrQmJ8cOCsG/4YOCN3lzYtSRAHNhg1Mvu1Lw gVF0Fw2Xbv80O4TVNdIz2AT8V64t5sXauTYUhZzyDodEBHjcRZPHKxuhuGXBneJ7gKxJ Slp6mclCptDCh4kmO2CMoeAPECbl8xsxvavTF9bkBh45w3JtVk3iC7Vn9HXCrgfe+j24 ocfg== X-Gm-Message-State: ALoCoQmaoFznHY++ldVIMqvZBHa2Kju0MTvzHRMNpR8GT4a1I6T9SfXmSZNQwOyKliB2QXzAAURRdMfxSyazoHQAodGwHCyeH39LaJwOnTmrPIgHZYz7v8ouOHYCIEwyQgVq/1CYOnYxv8M7JlGM+v0egQpE24b3lTPNGp2nZ/VBKLkNOp+7KlU= X-Received: by 10.42.16.69 with SMTP id o5mr4330527ica.17.1409263105592; Thu, 28 Aug 2014 14:58:25 -0700 (PDT) Received: from corp2gmr1-1.hot.corp.google.com (corp2gmr1-1.hot.corp.google.com [172.24.189.92]) by gmr-mx.google.com with ESMTPS id j25si298500yhb.0.2014.08.28.14.58.25 for (version=TLSv1.1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 28 Aug 2014 14:58:25 -0700 (PDT) Received: from ruffy2.mtv.corp.google.com (ruffy2.mtv.corp.google.com [172.17.128.107]) by corp2gmr1-1.hot.corp.google.com (Postfix) with ESMTP id 3946F31D7F1 for ; Thu, 28 Aug 2014 14:58:25 -0700 (PDT) From: Doug Evans MIME-Version: 1.0 Message-ID: <21503.42496.688813.787156@ruffy2.mtv.corp.google.com> Date: Thu, 28 Aug 2014 14:58:24 -0700 To: gdb-patches@sourceware.org Subject: Re: [PATCH] Show registers as live in better way in amd64-pseudo.c In-Reply-To: References: X-IsSubscribed: yes [apologies if this appears twice, my logs show it went out, but I haven't seen it show up on the list] Doug Evans writes: > Hi. > > This test is failing on clang because clang uses $eax as a temp register here > (grep for <<<<): > > (gdb) disas /m main > ... > 66 /* amd64-dword.exp writes eax-edi here. > 67 Tell gcc they're clobbered so it doesn't try to keep "data" in > 68 one of them. */ > 69 asm ("" > > 70 : /* no outputs */ > 71 : /* no inputs */ > 72 : "eax", "ebx", "ecx", "edx", "esi", "edi"); > 73 > 74 asm ("mov %%eax, 0(%0)\n\t" > 0x00000000004006c7 <+119>: mov $0x402020,%eax <<<< > 0x00000000004006cc <+124>: mov %eax,%r8d > 0x00000000004006cf <+127>: mov %eax,(%r8) > 0x00000000004006d2 <+130>: mov %ebx,0x4(%r8) > 0x00000000004006d6 <+134>: mov %ecx,0x8(%r8) > 0x00000000004006da <+138>: mov %edx,0xc(%r8) > 0x00000000004006de <+142>: mov %esi,0x10(%r8) > 0x00000000004006e2 <+146>: mov %edi,0x14(%r8) > > FAIL: gdb.arch/amd64-word.exp: check contents of %ax > FAIL: gdb.arch/amd64-word.exp: check contents of %r8w > FAIL: gdb.arch/i386-word.exp: check contents of %ax > [the {amd64,i386}-byte.exp and amd64-dword.exp tests fail similarly] > > This patch uses a different mechanism to indicate that eax,etc. > are live across the two asms. > > Regression tested with gcc 4.8.x-google and clang (trunk r215195). > > 2014-08-25 Doug Evans > > * gdb.arch/amd64-pseudo.c (main): Rewrite to better specify when > eax,etc. are live with values set by gdb and thus the compiler can't > use them. > * gdb.arch/i386-pseudo.c (main): Ditto. Hi. I checked in a slight variation of the patch. The original code had, effectively, asm ("nop"); /* set breakpoint here */ asm ("" /* inform compiler of register usage */); and I just went with that. It's slightly cleaner to indicate the register usage at the nop since that is where the .exp file will be setting the registers. commit fb0576e98388c6f4585b94684cea8d18c97a91aa Author: Doug Evans Date: Thu Aug 28 11:38:22 2014 -0700 Rewrite {amd64,i386}-pseudo.c to better specify register liveness. clang was using eax to construct %0 here: asm ("mov %%eax, 0(%0)\n\t" "mov %%ebx, 4(%0)\n\t" "mov %%ecx, 8(%0)\n\t" "mov %%edx, 12(%0)\n\t" "mov %%esi, 16(%0)\n\t" "mov %%edi, 20(%0)\n\t" : /* no output operands */ : "r" (data) : "eax", "ebx", "ecx", "edx", "esi", "edi"); which caused amd64-word.exp (and others similarly) to fail. It's a perfectly legit thing for clang to do given the available data. The patch fixes this by marking the registers as live from the time of the preceding breakpoint. gdb/testsuite/ChangeLog: * gdb.arch/amd64-pseudo.c (main): Rewrite to better specify when eax,etc. are live with values set by gdb and thus the compiler can't use them. * gdb.arch/i386-pseudo.c (main): Ditto. diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 4a65fef..d996b7c 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2014-08-28 Doug Evans + + * gdb.arch/amd64-pseudo.c (main): Rewrite to better specify when + eax,etc. are live with values set by gdb and thus the compiler can't + use them. + * gdb.arch/i386-pseudo.c (main): Ditto. + 2014-08-27 Doug Evans * lib/gdb.exp (gdb_compile_shlib): Add support for clang. diff --git a/gdb/testsuite/gdb.arch/amd64-pseudo.c b/gdb/testsuite/gdb.arch/amd64-pseudo.c index 4dac1f1..f6fd0e7 100644 --- a/gdb/testsuite/gdb.arch/amd64-pseudo.c +++ b/gdb/testsuite/gdb.arch/amd64-pseudo.c @@ -39,6 +39,21 @@ int data[] = { int main (int argc, char **argv) { + register int eax asm ("eax"); + register int ebx asm ("ebx"); + register int ecx asm ("ecx"); + register int edx asm ("edx"); + register int esi asm ("esi"); + register int edi asm ("edi"); + register long r8 asm ("r8"); + register long r9 asm ("r9"); + register long r10 asm ("r10"); + register long r11 asm ("r11"); + register long r12 asm ("r12"); + register long r13 asm ("r13"); + register long r14 asm ("r14"); + register long r15 asm ("r15"); + asm ("mov 0(%0), %%eax\n\t" "mov 4(%0), %%ebx\n\t" "mov 8(%0), %%ecx\n\t" @@ -61,15 +76,13 @@ main (int argc, char **argv) : /* no output operands */ : "r" (data) : "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15"); - asm ("nop"); /* second breakpoint here */ - /* amd64-dword.exp writes eax-edi here. - Tell gcc they're clobbered so it doesn't try to keep "data" in - one of them. */ - asm ("" - : /* no outputs */ - : /* no inputs */ - : "eax", "ebx", "ecx", "edx", "esi", "edi"); + asm ("nop" /* second breakpoint here */ + /* amd64-{byte,word,dword}.exp write eax-edi here. + Tell gcc/clang they're live. */ + : "=r" (eax), "=r" (ebx), "=r" (ecx), + "=r" (edx), "=r" (esi), "=r" (edi) + : /* no inputs */); asm ("mov %%eax, 0(%0)\n\t" "mov %%ebx, 4(%0)\n\t" @@ -78,17 +91,18 @@ main (int argc, char **argv) "mov %%esi, 16(%0)\n\t" "mov %%edi, 20(%0)\n\t" : /* no output operands */ - : "r" (data) - : "eax", "ebx", "ecx", "edx", "esi", "edi"); - asm ("nop"); /* third breakpoint here */ + : "r" (data), + /* Mark these as inputs so that gcc/clang won't try to use them as + a temp to build %0. */ + "r" (eax), "r" (ebx), "r" (ecx), + "r" (edx), "r" (esi), "r" (edi)); - /* amd64-dword.exp writes r8-r15 here. - Tell gcc they're clobbered so it doesn't try to keep "data" in - one of them. */ - asm ("" - : /* no outputs */ - : /* no inputs */ - : "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15"); + asm ("nop" /* third breakpoint here */ + /* amd64-{byte,word,dword}.exp write r8-r15 here. + Tell gcc/clang they're live. */ + : "=r" (r8), "=r" (r9), "=r" (r10), "=r" (r11), + "=r" (r12), "=r" (r13), "=r" (r14), "=r" (r15) + : /* no inputs */); asm ("mov %%r8d, 24(%0)\n\t" "mov %%r9d, 28(%0)\n\t" @@ -99,8 +113,11 @@ main (int argc, char **argv) "mov %%r14d, 48(%0)\n\t" "mov %%r15d, 52(%0)\n\t" : /* no output operands */ - : "r" (data) - : "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15"); + : "r" (data), + /* Mark these as inputs so that gcc/clang won't try to use them as + a temp to build %0. */ + "r" (r8), "r" (r9), "r" (r10), "r" (r11), + "r" (r12), "r" (r13), "r" (r14), "r" (r15)); puts ("Bye!"); /* forth breakpoint here */ return 0; diff --git a/gdb/testsuite/gdb.arch/i386-pseudo.c b/gdb/testsuite/gdb.arch/i386-pseudo.c index 11cbcd5..34f6b9b 100644 --- a/gdb/testsuite/gdb.arch/i386-pseudo.c +++ b/gdb/testsuite/gdb.arch/i386-pseudo.c @@ -29,6 +29,11 @@ int data[] = { int main (int argc, char **argv) { + register int eax asm ("eax"); + register int ebx asm ("ebx"); + register int ecx asm ("ecx"); + register int edx asm ("edx"); + asm ("mov 0(%0), %%eax\n\t" "mov 4(%0), %%ebx\n\t" "mov 8(%0), %%ecx\n\t" @@ -36,15 +41,22 @@ main (int argc, char **argv) : /* no output operands */ : "r" (data) : "eax", "ebx", "ecx", "edx"); - asm ("nop"); /* first breakpoint here */ + + asm ("nop" /* first breakpoint here */ + /* i386-{byte,word}.exp write eax-edx here. + Tell gcc/clang they're live. */ + : "=r" (eax), "=r" (ebx), "=r" (ecx), "=r" (edx) + : /* no inputs */); asm ("mov %%eax, 0(%0)\n\t" "mov %%ebx, 4(%0)\n\t" "mov %%ecx, 8(%0)\n\t" "mov %%edx, 12(%0)\n\t" : /* no output operands */ - : "r" (data) - : "eax", "ebx", "ecx", "edx"); + : "r" (data), + /* Mark these as inputs so that gcc/clang won't try to use them as + a temp to build %0. */ + "r" (eax), "r" (ebx), "r" (ecx), "r" (edx)); puts ("Bye!"); /* second breakpoint here */ return 0;