This is the mail archive of the binutils-cvs@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[binutils-gdb] x86: correct abort check


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=c5d0745b0d3edfea9e82a4b304ea1847790d72e4

commit c5d0745b0d3edfea9e82a4b304ea1847790d72e4
Author: Jan Beulich <jbeulich@novell.com>
Date:   Fri Dec 15 09:12:37 2017 +0100

    x86: correct abort check
    
    I'm rather certain the missing ! was just a typo, the more with the
    similar check in mind that's in the same function a few hundred lines
    down (in the body of "if (vex_reg != (unsigned int) ~0)"). Of course
    this can't be demonstrated by a test case - internal data structure
    consistency is being checked here, and neither form of the check
    triggers with any current template.
    
    It is also not really clear to me why operand_type_equal() is being used
    in the {X,Y,Z}MM register check here, rather than just testing the
    respective bits: Just like Reg32|Reg64 is legal in an operand template,
    I don't see why e.g. RegXMM|RegYMM wouldn't be. For example it ought to
    be possible to combine
    
    vaddpd, 3, 0x6658, None, 1, CpuAVX, Modrm|Vex|VexOpcode=0|VexVVVV=1|VexW=1|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Xmmword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S|RegXMM, RegXMM, RegXMM }
    vaddpd, 3, 0x6658, None, 1, CpuAVX, Modrm|Vex=2|VexOpcode=0|VexVVVV=1|VexW=1|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Ymmword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S|RegYMM, RegYMM, RegYMM }
    
    into a single template (with setting of VEX.L suitably handled elsewhere
    if that's not already happening anyway).
    
    Additionally I don't understand why this uses abort() instead of
    gas_assert().
    
    Both of these latter considerations then also apply to the
    aforementioned other check in the same function.

Diff:
---
 gas/ChangeLog        | 5 +++++
 gas/config/tc-i386.c | 4 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/gas/ChangeLog b/gas/ChangeLog
index a3854ab..96ceb58 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,8 @@
+2017-12-15  Jan Beulich  <jbeulich@suse.com>
+
+	* config/tc-i386.c (build_modrm_byte): Add missing ! to reg64
+	check leading to abort().
+
 2017-12-14  Nick Clifton  <nickc@redhat.com>
 
 	* config/tc-m32c.c: Update address of FSF in copyright notice.
diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index 092b99e..36e5b19 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -6437,7 +6437,7 @@ build_modrm_byte (void)
 	  if (i.tm.opcode_modifier.vexvvvv == VEXXDS)
 	    {
 	      /* For instructions with VexNDS, the register-only source
-		 operand must be 32/64bit integer, XMM, YMM or ZMM
+		 operand must be a 32/64bit integer, XMM, YMM, ZMM, or mask
 		 register.  It is encoded in VEX prefix.  We need to
 		 clear RegMem bit before calling operand_type_equal.  */
 
@@ -6459,7 +6459,7 @@ build_modrm_byte (void)
 	      op.bitfield.regmem = 0;
 	      if ((dest + 1) >= i.operands
 		  || (!op.bitfield.reg32
-		      && op.bitfield.reg64
+		      && !op.bitfield.reg64
 		      && !operand_type_equal (&op, &regxmm)
 		      && !operand_type_equal (&op, &regymm)
 		      && !operand_type_equal (&op, &regzmm)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]