diff mbox

Show registers as live in better way in amd64-pseudo.c

Message ID 21503.42496.688813.787156@ruffy2.mtv.corp.google.com
State New
Headers show

Commit Message

Doug Evans Aug. 28, 2014, 9:58 p.m. UTC
[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  <dje@google.com>
 > 
 > 	* 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 <dje@google.com>
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 mbox

Patch

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  <dje@google.com>
+
+	* 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  <dje@google.com>
 
 	* 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;