[ARM64] Check for proper immediate in shift/extend operands

[bsmith]

Summary

This fixes an assertion failure when using a negative immediate after a shift or extend operand.

There is no testcase for this fix since it is part of porting over MC/AArch64/basic-a64-diagnostics.s, which will be enabled for ARM64 once everything is fixed in it.

[t.p.northover]

Hi Bradley,

This looks reasonable to me.

Cheers.

Tim.

lib/Target/ARM64/AsmParser/ARM64AsmParser.cpp

private:
 
   SMLoc getLoc() const { return Parser.getTok().getLoc(); }
 
   bool parseSysAlias(StringRef Name, SMLoc NameLoc, OperandVector &Operands);
   unsigned parseCondCodeString(StringRef Cond);
   bool parseCondCode(OperandVector &Operands, bool invertCondCode);
   int tryParseRegister();
   int tryMatchVectorRegister(StringRef &Kind, bool expected);
-  bool parseOptionalShift(OperandVector &Operands);
-  bool parseOptionalExtend(OperandVector &Operands);
   bool parseRegister(OperandVector &Operands);
   bool parseMemory(OperandVector &Operands);
   bool parseSymbolicImmVal(const MCExpr *&ImmVal);
   bool parseVectorList(OperandVector &Operands);
   bool parseOperand(OperandVector &Operands, bool isCondCode,
                     bool invertCondCode);
 
   void Warning(SMLoc L, const Twine &Msg) { Parser.Warning(L, Msg); }

 /// @name Auto-generated Match Functions
 /// {
 
 #define GET_ASSEMBLER_HEADER
 #include "ARM64GenAsmMatcher.inc"
 
   /// }
 
+  OperandMatchResultTy tryParseOptionalShift(OperandVector &Operands);
+  OperandMatchResultTy tryParseOptionalExtend(OperandVector &Operands);
   OperandMatchResultTy tryParseNoIndexMemory(OperandVector &Operands);
   OperandMatchResultTy tryParseBarrierOperand(OperandVector &Operands);
   OperandMatchResultTy tryParseMRSSystemRegister(OperandVector &Operands);
   OperandMatchResultTy tryParseSysReg(OperandVector &Operands);
   OperandMatchResultTy tryParseSysCROperand(OperandVector &Operands);
   OperandMatchResultTy tryParsePrefetch(OperandVector &Operands);
   OperandMatchResultTy tryParseAdrpLabel(OperandVector &Operands);
   OperandMatchResultTy tryParseAdrLabel(OperandVector &Operands);
if (invertCondCode)
     CC = ARM64CC::getInvertedCondCode(ARM64CC::CondCode(CC));
 
   const MCExpr *CCExpr = MCConstantExpr::Create(CC, getContext());
   Operands.push_back(
       ARM64Operand::CreateImm(CCExpr, S, getLoc(), getContext()));
   return false;
 }
 
-/// ParseOptionalShift - Some operands take an optional shift argument. Parse
+/// tryParseOptionalShift - Some operands take an optional shift argument. Parse
 /// them if present.
-bool ARM64AsmParser::parseOptionalShift(OperandVector &Operands) {
+ARM64AsmParser::OperandMatchResultTy
+ARM64AsmParser::tryParseOptionalShift(OperandVector &Operands) {
   const AsmToken &Tok = Parser.getTok();
-  ARM64_AM::ShiftType ShOp = StringSwitch<ARM64_AM::ShiftType>(Tok.getString())
+  std::string LowerID = Tok.getString().lower();
+  ARM64_AM::ShiftType ShOp = StringSwitch<ARM64_AM::ShiftType>(LowerID)
                                  .Case("lsl", ARM64_AM::LSL)
                                  .Case("lsr", ARM64_AM::LSR)
                                  .Case("asr", ARM64_AM::ASR)
                                  .Case("ror", ARM64_AM::ROR)
                                  .Case("msl", ARM64_AM::MSL)
-                                 .Case("LSL", ARM64_AM::LSL)
-                                 .Case("LSR", ARM64_AM::LSR)
-                                 .Case("ASR", ARM64_AM::ASR)
-                                 .Case("ROR", ARM64_AM::ROR)
-                                 .Case("MSL", ARM64_AM::MSL)
                                  .Default(ARM64_AM::InvalidShift);
   if (ShOp == ARM64_AM::InvalidShift)
-    return true;
+    return MatchOperand_NoMatch;
 
   SMLoc S = Tok.getLoc();
   Parser.Lex();
 
   // We expect a number here.
   bool Hash = getLexer().is(AsmToken::Hash);
-  if (!Hash && getLexer().isNot(AsmToken::Integer))
-    return TokError("immediate value expected for shifter operand");
+  if (!Hash && getLexer().isNot(AsmToken::Integer)) {
+    TokError("immediate value expected for shifter operand");
+    return MatchOperand_ParseFail;
+  }
 
   if (Hash)
     Parser.Lex(); // Eat the '#'.
 
+  // Make sure we do actually have a number
+  if (!Parser.getTok().is(AsmToken::Integer)) {
+    Error(Parser.getTok().getLoc(),
+          "expected integer shift amount");
+    return MatchOperand_ParseFail;
+  }
+
   SMLoc ExprLoc = getLoc();
   const MCExpr *ImmVal;
   if (getParser().parseExpression(ImmVal))
-    return true;
+    return MatchOperand_ParseFail;
 
   const MCConstantExpr *MCE = dyn_cast<MCConstantExpr>(ImmVal);
-  if (!MCE)
-    return TokError("immediate value expected for shifter operand");
+  if (!MCE) {
+    TokError("immediate value expected for shifter operand");
+    return MatchOperand_ParseFail;
+  }
 
-  if ((MCE->getValue() & 0x3f) != MCE->getValue())
-    return Error(ExprLoc, "immediate value too large for shifter operand");
+  if ((MCE->getValue() & 0x3f) != MCE->getValue()) {
+    Error(ExprLoc, "immediate value too large for shifter operand");
+    return MatchOperand_ParseFail;
+  }
 
   SMLoc E = SMLoc::getFromPointer(getLoc().getPointer() - 1);
   Operands.push_back(
       ARM64Operand::CreateShifter(ShOp, MCE->getValue(), S, E, getContext()));
-  return false;
+  return MatchOperand_Success;
 }
 
-/// parseOptionalExtend - Some operands take an optional extend argument. Parse
+/// tryParseOptionalExtend - Some operands take an optional extend argument. Parse
 /// them if present.
-bool ARM64AsmParser::parseOptionalExtend(OperandVector &Operands) {
+ARM64AsmParser::OperandMatchResultTy
+ARM64AsmParser::tryParseOptionalExtend(OperandVector &Operands) {
   const AsmToken &Tok = Parser.getTok();
+  std::string LowerID = Tok.getString().lower();
   ARM64_AM::ExtendType ExtOp =
-      StringSwitch<ARM64_AM::ExtendType>(Tok.getString())
+      StringSwitch<ARM64_AM::ExtendType>(LowerID)
           .Case("uxtb", ARM64_AM::UXTB)
           .Case("uxth", ARM64_AM::UXTH)
           .Case("uxtw", ARM64_AM::UXTW)
           .Case("uxtx", ARM64_AM::UXTX)
           .Case("lsl", ARM64_AM::UXTX) // Alias for UXTX
           .Case("sxtb", ARM64_AM::SXTB)
           .Case("sxth", ARM64_AM::SXTH)
           .Case("sxtw", ARM64_AM::SXTW)
           .Case("sxtx", ARM64_AM::SXTX)
-          .Case("UXTB", ARM64_AM::UXTB)
-          .Case("UXTH", ARM64_AM::UXTH)
-          .Case("UXTW", ARM64_AM::UXTW)
-          .Case("UXTX", ARM64_AM::UXTX)
-          .Case("LSL", ARM64_AM::UXTX) // Alias for UXTX
-          .Case("SXTB", ARM64_AM::SXTB)
-          .Case("SXTH", ARM64_AM::SXTH)
-          .Case("SXTW", ARM64_AM::SXTW)
-          .Case("SXTX", ARM64_AM::SXTX)
           .Default(ARM64_AM::InvalidExtend);
   if (ExtOp == ARM64_AM::InvalidExtend)
-    return true;
+    return MatchOperand_NoMatch;
 
   SMLoc S = Tok.getLoc();
   Parser.Lex();
 
   if (getLexer().is(AsmToken::EndOfStatement) ||
       getLexer().is(AsmToken::Comma)) {
     SMLoc E = SMLoc::getFromPointer(getLoc().getPointer() - 1);
     Operands.push_back(
         ARM64Operand::CreateExtend(ExtOp, 0, S, E, getContext()));
-    return false;
+    return MatchOperand_Success;
   }
 
   bool Hash = getLexer().is(AsmToken::Hash);
   if (!Hash && getLexer().isNot(AsmToken::Integer)) {
     SMLoc E = SMLoc::getFromPointer(getLoc().getPointer() - 1);
     Operands.push_back(
         ARM64Operand::CreateExtend(ExtOp, 0, S, E, getContext()));
-    return false;
+    return MatchOperand_Success;
   }
 
   if (Hash)
     Parser.Lex(); // Eat the '#'.
 
+  // Make sure we do actually have a number
+  if (!Parser.getTok().is(AsmToken::Integer)) {
+    Error(Parser.getTok().getLoc(),
+          "expected integer shift amount");
+    return MatchOperand_ParseFail;
+  }
+
   const MCExpr *ImmVal;
   if (getParser().parseExpression(ImmVal))
-    return true;
+    return MatchOperand_ParseFail;
 
   const MCConstantExpr *MCE = dyn_cast<MCConstantExpr>(ImmVal);
-  if (!MCE)
-    return TokError("immediate value expected for extend operand");
+  if (!MCE) {
+    TokError("immediate value expected for extend operand");
+    return MatchOperand_ParseFail;
+  }
 
   SMLoc E = SMLoc::getFromPointer(getLoc().getPointer() - 1);
   Operands.push_back(
       ARM64Operand::CreateExtend(ExtOp, MCE->getValue(), S, E, getContext()));
-  return false;
+  return MatchOperand_Success;
 }
 
 /// parseSysAlias - The IC, DC, AT, and TLBI instructions are simple aliases for
 /// the SYS instruction. Parse them specially so that we create a SYS MCInst.
 bool ARM64AsmParser::parseSysAlias(StringRef Name, SMLoc NameLoc,
                                    OperandVector &Operands) {
   if (Name.find('.') != StringRef::npos)
     return TokError("invalid operand");
case AsmToken::Identifier: {
     if (isCondCode)
       return parseCondCode(Operands, invertCondCode);
 
     // If it's a register name, parse it.
     if (!parseRegister(Operands))
       return false;
 
     // This could be an optional "shift" operand.
-    if (!parseOptionalShift(Operands))
-      return false;
+    OperandMatchResultTy GotShift = tryParseOptionalShift(Operands);
+    // We can only continue if no tokens were eaten.
+    if (GotShift != MatchOperand_NoMatch)
+      return GotShift;
 
     // Or maybe it could be an optional "extend" operand.
-    if (!parseOptionalExtend(Operands))
-      return false;
+    OperandMatchResultTy GotExtend = tryParseOptionalExtend(Operands);
+    // We can only continue if no tokens were eaten.
+    if (GotExtend != MatchOperand_NoMatch)
+      return GotExtend;
 
     // This was not a register so parse other operands that start with an
     // identifier (like labels) as expressions and create them as immediates.
     const MCExpr *IdVal;
     S = getLoc();
     if (getParser().parseExpression(IdVal))
       return true;