[PATCH] Rewrite the codes for opcode 0x0f01 and add more instructions support

Michael Snyder msnyder@vmware.com
Mon Oct 12 02:08:00 GMT 2009


Jiang Jilin wrote:
> Hi, guys
> 
> I've rewrite the codes for opcode 0x0f01 with more readable, add
> xgetbv/xsetbv/rdtscp/vmcall/vmlaunch/vmresume/vmxoff instructions
> support as well.
> 
> However, I'm *not* sure it's whether right or not, especially with
> the new supported instructions beginning with "vm". And I remove all
> codes to save EFLAGS register which is not specified to be saved by
> Intel's manual, so please help me review them.
> 
> Luckily, there is no regression when using precord.exp board file to test.
> 
> At last but not least, there is some differences in gdb.sum when
> 'make check' before and after applying this patch. I cannot make
> a decision whether it's correct, so please help me. The diff are
> as follows:

Ah well, but you see, now the change is too big to be accepted
without a copyright assignment.  Do you want to start the process
of filing one?

[Cc: Tom Tromey]

> 
> --- testsuite/gdb.sum.before	2009-10-10 20:02:35.000000000 +0800
> +++ testsuite/gdb.sum.after	2009-10-10 20:21:35.000000000 +0800
> @@ -1,4 +1,4 @@
> -Test Run By jiang on Sat Oct 10 19:44:30 2009
> +Test Run By jiang on Sat Oct 10 20:05:20 2009
>  Native configuration is i686-pc-linux-gnu
> 
>  		=== gdb tests ===
> @@ -14113,18 +14113,18 @@
>  PASS: gdb.threads/watchthreads2.exp: all threads started
>  PASS: gdb.threads/watchthreads2.exp: watch x
>  PASS: gdb.threads/watchthreads2.exp: set var test_ready = 1
> -KFAIL: gdb.threads/watchthreads2.exp: gdb can drop watchpoints in multithreaded app (PRMS: gdb/10116)
> +PASS: gdb.threads/watchthreads2.exp: all threads incremented x
>  Running ./gdb.threads/watchthreads.exp ...
>  PASS: gdb.threads/watchthreads.exp: successfully compiled posix threads test case
>  PASS: gdb.threads/watchthreads.exp: watch args[0]
>  PASS: gdb.threads/watchthreads.exp: watch args[1]
>  PASS: gdb.threads/watchthreads.exp: disable 2
> -FAIL: gdb.threads/watchthreads.exp: threaded watch loop
> +PASS: gdb.threads/watchthreads.exp: threaded watch loop
>  PASS: gdb.threads/watchthreads.exp: first watchpoint on args[0] hit
>  PASS: gdb.threads/watchthreads.exp: first watchpoint on args[1] hit
>  PASS: gdb.threads/watchthreads.exp: watchpoint on args[0] hit in thread
>  PASS: gdb.threads/watchthreads.exp: watchpoint on args[1] hit in thread
> -FAIL: gdb.threads/watchthreads.exp: combination of threaded watchpoints = 30
> +PASS: gdb.threads/watchthreads.exp: combination of threaded watchpoints = 30
>  Running ./gdb.trace/actions.exp ...
>  PASS: gdb.trace/actions.exp: 5.1a: set three tracepoints, no actions
>  PASS: gdb.trace/actions.exp: 5.1b: set actions for first tracepoint
> @@ -14288,8 +14288,8 @@
> 
>  		=== gdb Summary ===
> 
> -# of expected passes		13512
> -# of unexpected failures	76
> +# of expected passes		13515
> +# of unexpected failures	74
> 
> 2009-10-10  Jiang Jilin  <freephp@gmail.com>
> 	* i386-tdep.c (i386_process_record): Rewrite the codes for
> 	opcode 0x0f01 and add more instructions support
> ---
>  gdb/i386-tdep.c |  234 ++++++++++++++++++++++++------------------------------
>  1 files changed, 104 insertions(+), 130 deletions(-)
> 
> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
> index b79bcd2..2300a91 100644
> --- a/gdb/i386-tdep.c
> +++ b/gdb/i386-tdep.c
> @@ -5075,152 +5075,126 @@ reswitch:
>      case 0x0f01:
>        if (i386_record_modrm (&ir))
>  	return -1;
> -      switch (ir.reg)
> +      if (ir.mod == 3)
>  	{
> -	  /* sgdt */
> -	case 0:
> -	  {
> -	    uint64_t tmpu64;
> -
> -	    if (ir.mod == 3)
> -	      {
> -		ir.addr -= 3;
> -		opcode = opcode << 8 | ir.modrm;
> -		goto no_support;
> -	      }
> -	    if (ir.override >= 0)
> -	      {
> -		warning (_("Process record ignores the memory "
> -                           "change of instruction at "
> -                           "address %s because it can't get "
> -                           "the value of the segment "
> -                           "register."),
> -                         paddress (gdbarch, ir.orig_addr));
> -	      }
> -	    else
> -	      {
> -		if (i386_record_lea_modrm_addr (&ir, &tmpu64))
> -		  return -1;
> -		if (record_arch_list_add_mem (tmpu64, 2))
> -		  return -1;
> -		tmpu64 += 2;
> -                if (ir.regmap[X86_RECORD_R8_REGNUM])
> -                  {
> -                    if (record_arch_list_add_mem (tmpu64, 8))
> -		      return -1;
> -                  }
> -                else
> -                  {
> -                    if (record_arch_list_add_mem (tmpu64, 4))
> -		      return -1;
> -                  }
> -	      }
> -	  }
> -	  break;
> -	case 1:
> -	  if (ir.mod == 3)
> +	  uint8_t reg_rm = (ir.reg << 4) | ir.rm;
> +
> +	  switch (reg_rm)
>  	    {
> -	      switch (ir.rm)
> -		{
> -		  /* monitor */
> -		case 0:
> -		  break;
> -		  /* mwait */
> -		case 1:
> -		  I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
> -		  break;
> -		default:
> -		  ir.addr -= 3;
> -		  opcode = opcode << 8 | ir.modrm;
> -		  goto no_support;
> -		  break;
> +	    /* vmcall */
> +	    case 0x01:
> +	    /* vmlaunch */
> +	    case 0x02:
> +	    /* vmresume */
> +	    case 0x03:
> +	    /* vmxoff */
> +	    case 0x04:
> +	      I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
> +	      break;
> +	    /* monitor */
> +	    case 0x10:
> +	      break;
> +	    /* mwait */
> +	    case 0x11:
> +	      I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
> +	      break;
> +	    /* xgetbv */
> +	    case 0x20:
> +	      I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
> +	      I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
> +	      break;
> +	    /* xsetbv */
> +	    case 0x21:
> +	      break;
> +	    /* swapgs */
> +	    case 0x70:
> +	      if (ir.regmap[X86_RECORD_R8_REGNUM])
> +	        I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_GS_REGNUM);
> +	      else
> +	        {
> +	          ir.addr -= 3;
> +	          opcode = opcode << 8 | ir.modrm;
> +	          goto no_support;
> +	        }
> +	      break;
> +	    /* rdtscp */
> +	    case 0x71:
> +	      I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
> +	      I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RECX_REGNUM);
> +	      I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
> +	      break;
> +	    default: 
> +	      /* smsw */
> +	      if (ir.reg == 4)
> +	        {
> +		  I386_RECORD_ARCH_LIST_ADD_REG (ir.rm | ir.rex_b); 
> +		  break; 
>  		}
> +	      /* lmsw */
> +	      else if (ir.reg == 6)
> +		  break; 
> +	      ir.addr -= 3;
> +	      opcode = opcode << 8 | ir.modrm;
> +	      goto no_support;
>  	    }
> -	  else
> +	}
> +      else
> +	{
> +	  switch (ir.reg)
>  	    {
> -	      /* sidt */
> +	    /* sgdt */
> +	    case 0:
> +	    /* sidt */
> +	    case 1: 
>  	      if (ir.override >= 0)
> -		{
> +	        {
>  		  warning (_("Process record ignores the memory "
> -                             "change of instruction at "
> -                             "address %s because it can't get "
> -                             "the value of the segment "
> -                             "register."),
> -                           paddress (gdbarch, ir.orig_addr));
> +			     "change of instruction at "
> +			     "address %s because it can't get "
> +			     "the value of the segment "
> +			     "register."),
> +			   paddress (gdbarch, ir.orig_addr));
>  		}
>  	      else
> -		{
> -		  uint64_t tmpu64;
> +	        {
> +	          uint64_t tmpu64;
>  
> -		  if (i386_record_lea_modrm_addr (&ir, &tmpu64))
> +		  /* We have to store at least (4 + 2 = 6) bytes, 
> +		     or (8 + 2 = 10) bytes at most.  */
> +	  	  if (i386_record_lea_modrm_addr (&ir, &tmpu64))
> +	  	    return -1;
> +		  if (record_arch_list_add_mem (tmpu64, 6))
>  		    return -1;
> -		  if (record_arch_list_add_mem (tmpu64, 2))
> -		    return -1;
> -		  addr += 2;
> -                  if (ir.regmap[X86_RECORD_R8_REGNUM])
> -                    {
> -                      if (record_arch_list_add_mem (tmpu64, 8))
> -		        return -1;
> -                    }
> -                  else
> -                    {
> -                      if (record_arch_list_add_mem (tmpu64, 4))
> -		        return -1;
> -                    }
> +		  tmpu64 += 6;
> +		  if (ir.regmap[X86_RECORD_R8_REGNUM])
> +		    {
> +		      if (record_arch_list_add_mem (tmpu64, 4))
> +		 	return -1;
> +		    }
>  		}
> -	    }
> -	  break;
> -	  /* lgdt */
> -	case 2:
> -	  /* lidt */
> -	case 3:
> -	  if (ir.mod == 3)
> -	    {
> -	      ir.addr -= 3;
> -	      opcode = opcode << 8 | ir.modrm;
> -	      goto no_support;
> -	    }
> -	  break;
> -	  /* smsw */
> -	case 4:
> -	  if (ir.mod == 3)
> -	    {
> -	      if (record_arch_list_add_reg (ir.regcache, ir.rm | ir.rex_b))
> -		return -1;
> -	    }
> -	  else
> -	    {
> +	      break;
> +	    /* lgdt */
> +	    case 2:
> +	    /* lidt */
> +	    case 3:
> +	      break;
> +	    /* smsw */
> +	    case 4:
>  	      ir.ot = OT_WORD;
>  	      if (i386_record_lea_modrm (&ir))
>  		return -1;
> +	      break;
> +	    /* lmsw */
> +	    case 6:
> +	      break;
> +	    /* invlpg */
> +	    case 7:
> +	      break;
> +	    default: 
> +	      ir.addr -= 3;
> +	      opcode = opcode << 8 | ir.modrm;
> +	      goto no_support;
>  	    }
> -	  I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
> -	  break;
> -	  /* lmsw */
> -	case 6:
> -	  I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
> -	  break;
> -	  /* invlpg */
> -	case 7:
> -	  if (ir.mod == 3)
> -	    {
> -	      if (ir.rm == 0 && ir.regmap[X86_RECORD_R8_REGNUM])
> -	        I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_GS_REGNUM);
> -	      else
> -	        {
> -	          ir.addr -= 3;
> -	          opcode = opcode << 8 | ir.modrm;
> -	          goto no_support;
> -	        }
> -	    }
> -	  else
> -	    I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
> -	  break;
> -	default:
> -	  ir.addr -= 3;
> -	  opcode = opcode << 8 | ir.modrm;
> -	  goto no_support;
> -	  break;
>  	}
>        break;
>  



More information about the Gdb-patches mailing list