[PATCH] D35115: Clang's assembler crashes if Scale in lea is negative (pr33661)

[Andrew V. Tischenko via Phabricator via llvm-commits]

avt77 updated this revision to Diff 105645.
avt77 added a comment.

I removed NFCs from this patch.


https://reviews.llvm.org/D35115

Files:
  lib/Target/X86/AsmParser/X86AsmParser.cpp
  test/MC/X86/intel-syntax-invalid-scale.s


Index: test/MC/X86/intel-syntax-invalid-scale.s
===================================================================
--- test/MC/X86/intel-syntax-invalid-scale.s
+++ test/MC/X86/intel-syntax-invalid-scale.s
@@ -9,3 +9,7 @@
     lea rax, [rdi + rdx*32]
 // CHECK: error: scale factor in address must be 1, 2, 4 or 8
     lea rax, [rdi + rdx*16]
+// CHECK: error: unknown token in expression
+    lea rax, [rdi + rdx*-8]
+// CHECK: error: scale factor in address must be 1, 2, 4 or 8
+    lea rax, [rdi + -1*rdx]
Index: lib/Target/X86/AsmParser/X86AsmParser.cpp
===================================================================
--- lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -125,8 +125,8 @@
     int64_t popOperand() {
       assert (!PostfixStack.empty() && "Poped an empty stack!");
       ICToken Op = PostfixStack.pop_back_val();
-      assert ((Op.first == IC_IMM || Op.first == IC_REGISTER)
-              && "Expected and immediate or register!");
+      if (!(Op.first == IC_IMM || Op.first == IC_REGISTER))
+        return -1; // Return invalid value of Scale
       return Op.second;
     }
     void pushOperand(InfixCalculatorTok Op, int64_t Val = 0) {
@@ -475,7 +475,11 @@
         if (CurrState == IES_REGISTER || CurrState == IES_RPAREN ||
             CurrState == IES_INTEGER  || CurrState == IES_RBRAC)
           IC.pushOperator(IC_MINUS);
-        else
+        else if (PrevState == IES_REGISTER && CurrState == IES_MULTIPLY) {
+          // We have negate operator for Scale: it's illegal
+          State = IES_ERROR;
+          break;
+        } else
           IC.pushOperator(IC_NEG);
         if (CurrState == IES_REGISTER && PrevState != IES_MULTIPLY) {
           // If we already have a BaseReg, then assume this is the IndexReg with
@@ -517,7 +521,16 @@
       }
       PrevState = CurrState;
     }
-    void onRegister(unsigned Reg) {
+
+    bool checkScale(StringRef &ErrMsg) {
+      if (Scale != 1 && Scale != 2 && Scale != 4 && Scale != 8) {
+        ErrMsg = "scale factor in address must be 1, 2, 4 or 8";
+        return true;
+      }
+      return false;
+    }
+
+    bool onRegister(unsigned Reg, StringRef &ErrMsg) {
       IntelExprState CurrState = State;
       switch (State) {
       default:
@@ -537,14 +550,17 @@
           IndexReg = Reg;
           // Get the scale and replace the 'Scale * Register' with '0'.
           Scale = IC.popOperand();
+          if (checkScale(ErrMsg))
+            return true;
           IC.pushOperand(IC_IMM);
           IC.popOperator();
         } else {
           State = IES_ERROR;
         }
         break;
       }
       PrevState = CurrState;
+      return false;
     }
     void onIdentifierExpr(const MCExpr *SymRef, StringRef SymRefName) {
       PrevState = State;
@@ -586,10 +602,8 @@
           assert (!IndexReg && "IndexReg already set!");
           IndexReg = TmpReg;
           Scale = TmpInt;
-          if(Scale != 1 && Scale != 2 && Scale != 4 && Scale != 8) {
-            ErrMsg = "scale factor in address must be 1, 2, 4 or 8";
+          if (checkScale(ErrMsg))
             return true;
-          }
           // Get the scale and replace the 'Register * Scale' with '0'.
           IC.popOperator();
         } else {
@@ -1386,7 +1400,9 @@
       StringRef Identifier = Tok.getString();
       UpdateLocLex = false;
       if (TK != AsmToken::String && !ParseRegister(TmpReg, IdentLoc, End)) {
-        SM.onRegister(TmpReg);
+        StringRef ErrMsg;
+        if (SM.onRegister(TmpReg, ErrMsg))
+          return Error(Tok.getLoc(), ErrMsg);
       } else if (ParseIntelNamedOperator(Identifier, SM)) {
         UpdateLocLex = true;
       } else if (!isParsingInlineAsm()) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D35115.105645.patch
Type: text/x-patch
Size: 3745 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170707/8cba0435/attachment.bin>