[SystemZ] Improve AsmParser handling of invalid instructions
authorRichard Sandiford <rsandifo@linux.vnet.ibm.com>
Fri, 24 May 2013 14:26:46 +0000 (14:26 +0000)
committerRichard Sandiford <rsandifo@linux.vnet.ibm.com>
Fri, 24 May 2013 14:26:46 +0000 (14:26 +0000)
Previously, an invalid instruction like:

foo     %r1, %r0

would generate the rather odd error message:

....: error: unknown token in expression
foo     %r1, %r0
^

We now get the more informative:

....: error: invalid instruction
foo     %r1, %r0
^

The same would happen if an address were used where a register was expected.
We now get "invalid operand for instruction" instead.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@182644 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Target/SystemZ/AsmParser/SystemZAsmParser.cpp
test/MC/SystemZ/regs-bad.s
test/MC/SystemZ/tokens.s [new file with mode: 0644]

index 600834e74494a2cb0d6ebc6b56e817bc2d12bb85..7c28abd8b1d6cd56324ca60751ebf86b9873a3a3 100644 (file)
@@ -44,6 +44,7 @@ public:
 
 private:
   enum OperandKind {
+    KindInvalid,
     KindToken,
     KindReg,
     KindAccessReg,
@@ -108,6 +109,9 @@ private:
 
 public:
   // Create particular kinds of operand.
+  static SystemZOperand *createInvalid(SMLoc StartLoc, SMLoc EndLoc) {
+    return new SystemZOperand(KindInvalid, StartLoc, EndLoc);
+  }
   static SystemZOperand *createToken(StringRef Str, SMLoc Loc) {
     SystemZOperand *Op = new SystemZOperand(KindToken, Loc, Loc);
     Op->Token.Data = Str.data();
@@ -279,9 +283,8 @@ private:
 
   bool parseRegister(Register &Reg);
 
-  OperandMatchResultTy
-  parseRegister(Register &Reg, RegisterGroup Group, const unsigned *Regs,
-                bool IsAddress = false);
+  bool parseRegister(Register &Reg, RegisterGroup Group, const unsigned *Regs,
+                     bool IsAddress = false);
 
   OperandMatchResultTy
   parseRegister(SmallVectorImpl<MCParsedAsmOperand*> &Operands,
@@ -289,6 +292,11 @@ private:
                 SystemZOperand::RegisterKind Kind,
                 bool IsAddress = false);
 
+  bool parseAddress(unsigned &Base, const MCExpr *&Disp,
+                    unsigned &Index, const unsigned *Regs,
+                    SystemZOperand::RegisterKind RegKind,
+                    bool HasIndex);
+
   OperandMatchResultTy
   parseAddress(SmallVectorImpl<MCParsedAsmOperand*> &Operands,
                const unsigned *Regs, SystemZOperand::RegisterKind RegKind,
@@ -411,22 +419,22 @@ bool SystemZAsmParser::parseRegister(Register &Reg) {
 
   // Eat the % prefix.
   if (Parser.getTok().isNot(AsmToken::Percent))
-    return true;
+    return Error(Parser.getTok().getLoc(), "register expected");
   Parser.Lex();
 
   // Expect a register name.
   if (Parser.getTok().isNot(AsmToken::Identifier))
-    return true;
+    return Error(Reg.StartLoc, "invalid register");
 
   // Check that there's a prefix.
   StringRef Name = Parser.getTok().getString();
   if (Name.size() < 2)
-    return true;
+    return Error(Reg.StartLoc, "invalid register");
   char Prefix = Name[0];
 
   // Treat the rest of the register name as a register number.
   if (Name.substr(1).getAsInteger(10, Reg.Num))
-    return true;
+    return Error(Reg.StartLoc, "invalid register");
 
   // Look for valid combinations of prefix and number.
   if (Prefix == 'r' && Reg.Num < 16)
@@ -436,7 +444,7 @@ bool SystemZAsmParser::parseRegister(Register &Reg) {
   else if (Prefix == 'a' && Reg.Num < 16)
     Reg.Group = RegAccess;
   else
-    return true;
+    return Error(Reg.StartLoc, "invalid register");
 
   Reg.EndLoc = Parser.getTok().getLoc();
   Parser.Lex();
@@ -447,30 +455,19 @@ bool SystemZAsmParser::parseRegister(Register &Reg) {
 // the raw register number to LLVM numbering, with zero entries indicating
 // an invalid register.  IsAddress says whether the register appears in an
 // address context.
-SystemZAsmParser::OperandMatchResultTy
-SystemZAsmParser::parseRegister(Register &Reg, RegisterGroup Group,
-                                const unsigned *Regs, bool IsAddress) {
-  if (Parser.getTok().isNot(AsmToken::Percent))
-    return MatchOperand_NoMatch;
-  if (parseRegister(Reg)) {
-    Error(Reg.StartLoc, "invalid register");
-    return MatchOperand_ParseFail;
-  }
-  if (Reg.Group != Group) {
-    Error(Reg.StartLoc, "invalid operand for instruction");
-    return MatchOperand_ParseFail;
-  }
-  if (Regs && Regs[Reg.Num] == 0) {
-    Error(Reg.StartLoc, "invalid register pair");
-    return MatchOperand_ParseFail;
-  }
-  if (Reg.Num == 0 && IsAddress) {
-    Error(Reg.StartLoc, "%r0 used in an address");
-    return MatchOperand_ParseFail;
-  }
+bool SystemZAsmParser::parseRegister(Register &Reg, RegisterGroup Group,
+                                     const unsigned *Regs, bool IsAddress) {
+  if (parseRegister(Reg))
+    return true;
+  if (Reg.Group != Group)
+    return Error(Reg.StartLoc, "invalid operand for instruction");
+  if (Regs && Regs[Reg.Num] == 0)
+    return Error(Reg.StartLoc, "invalid register pair");
+  if (Reg.Num == 0 && IsAddress)
+    return Error(Reg.StartLoc, "%r0 used in an address");
   if (Regs)
     Reg.Num = Regs[Reg.Num];
-  return MatchOperand_Success;
+  return false;
 }
 
 // Parse a register and add it to Operands.  The other arguments are as above.
@@ -479,64 +476,75 @@ SystemZAsmParser::parseRegister(SmallVectorImpl<MCParsedAsmOperand*> &Operands,
                                 RegisterGroup Group, const unsigned *Regs,
                                 SystemZOperand::RegisterKind Kind,
                                 bool IsAddress) {
+  if (Parser.getTok().isNot(AsmToken::Percent))
+    return MatchOperand_NoMatch;
+
   Register Reg;
-  OperandMatchResultTy Result = parseRegister(Reg, Group, Regs, IsAddress);
-  if (Result == MatchOperand_Success)
-    Operands.push_back(SystemZOperand::createReg(Kind, Reg.Num,
-                                                 Reg.StartLoc, Reg.EndLoc));
-  return Result;
-}
+  if (parseRegister(Reg, Group, Regs, IsAddress))
+    return MatchOperand_ParseFail;
 
-// Parse a memory operand and add it to Operands.  Regs maps asm register
-// numbers to LLVM address registers and RegKind says what kind of address
-// register we're using (ADDR32Reg or ADDR64Reg).  HasIndex says whether
-// the address allows index registers.
-SystemZAsmParser::OperandMatchResultTy
-SystemZAsmParser::parseAddress(SmallVectorImpl<MCParsedAsmOperand*> &Operands,
-                               const unsigned *Regs,
-                               SystemZOperand::RegisterKind RegKind,
-                               bool HasIndex) {
-  SMLoc StartLoc = Parser.getTok().getLoc();
+  Operands.push_back(SystemZOperand::createReg(Kind, Reg.Num,
+                                               Reg.StartLoc, Reg.EndLoc));
+  return MatchOperand_Success;
+}
 
+// Parse a memory operand into Base, Disp and Index.  Regs maps asm
+// register numbers to LLVM register numbers and RegKind says what kind
+// of address register we're using (ADDR32Reg or ADDR64Reg).  HasIndex
+// says whether the address allows index registers.
+bool SystemZAsmParser::parseAddress(unsigned &Base, const MCExpr *&Disp,
+                                    unsigned &Index, const unsigned *Regs,
+                                    SystemZOperand::RegisterKind RegKind,
+                                    bool HasIndex) {
   // Parse the displacement, which must always be present.
-  const MCExpr *Disp;
   if (getParser().parseExpression(Disp))
-    return MatchOperand_NoMatch;
+    return true;
 
   // Parse the optional base and index.
-  unsigned Index = 0;
-  unsigned Base = 0;
+  Index = 0;
+  Base = 0;
   if (getLexer().is(AsmToken::LParen)) {
     Parser.Lex();
 
     // Parse the first register.
     Register Reg;
-    OperandMatchResultTy Result = parseRegister(Reg, RegGR, Regs, true);
-    if (Result != MatchOperand_Success)
-      return Result;
+    if (parseRegister(Reg, RegGR, Regs, RegKind))
+      return true;
 
     // Check whether there's a second register.  If so, the one that we
     // just parsed was the index.
     if (getLexer().is(AsmToken::Comma)) {
       Parser.Lex();
 
-      if (!HasIndex) {
-        Error(Reg.StartLoc, "invalid use of indexed addressing");
-        return MatchOperand_ParseFail;
-      }
+      if (!HasIndex)
+        return Error(Reg.StartLoc, "invalid use of indexed addressing");
 
       Index = Reg.Num;
-      Result = parseRegister(Reg, RegGR, Regs, true);
-      if (Result != MatchOperand_Success)
-        return Result;
+      if (parseRegister(Reg, RegGR, Regs, RegKind))
+        return true;
     }
     Base = Reg.Num;
 
     // Consume the closing bracket.
     if (getLexer().isNot(AsmToken::RParen))
-      return MatchOperand_NoMatch;
+      return Error(Parser.getTok().getLoc(), "unexpected token in address");
     Parser.Lex();
   }
+  return false;
+}
+
+// Parse a memory operand and add it to Operands.  The other arguments
+// are as above.
+SystemZAsmParser::OperandMatchResultTy
+SystemZAsmParser::parseAddress(SmallVectorImpl<MCParsedAsmOperand*> &Operands,
+                               const unsigned *Regs,
+                               SystemZOperand::RegisterKind RegKind,
+                               bool HasIndex) {
+  SMLoc StartLoc = Parser.getTok().getLoc();
+  unsigned Base, Index;
+  const MCExpr *Disp;
+  if (parseAddress(Base, Disp, Index, Regs, RegKind, HasIndex))
+    return MatchOperand_ParseFail;
 
   SMLoc EndLoc =
     SMLoc::getFromPointer(Parser.getTok().getLoc().getPointer() - 1);
@@ -551,11 +559,9 @@ bool SystemZAsmParser::ParseDirective(AsmToken DirectiveID) {
 
 bool SystemZAsmParser::ParseRegister(unsigned &RegNo, SMLoc &StartLoc,
                                      SMLoc &EndLoc) {
-  if (Parser.getTok().isNot(AsmToken::Percent))
-    return Error(Parser.getTok().getLoc(), "register expected");
   Register Reg;
   if (parseRegister(Reg))
-    return Error(Reg.StartLoc, "invalid register");
+    return true;
   if (Reg.Group == RegGR)
     RegNo = SystemZMC::GR64Regs[Reg.Num];
   else if (Reg.Group == RegFP)
@@ -616,15 +622,34 @@ parseOperand(SmallVectorImpl<MCParsedAsmOperand*> &Operands,
   if (ResTy == MatchOperand_ParseFail)
     return true;
 
-  // The only other type of operand is an immediate.
-  const MCExpr *Expr;
+  // Check for a register.  All real register operands should have used
+  // a context-dependent parse routine, which gives the required register
+  // class.  The code is here to mop up other cases, like those where
+  // the instruction isn't recognized.
+  if (Parser.getTok().is(AsmToken::Percent)) {
+    Register Reg;
+    if (parseRegister(Reg))
+      return true;
+    Operands.push_back(SystemZOperand::createInvalid(Reg.StartLoc, Reg.EndLoc));
+    return false;
+  }
+
+  // The only other type of operand is an immediate or address.  As above,
+  // real address operands should have used a context-dependent parse routine,
+  // so we treat any plain expression as an immediate.
   SMLoc StartLoc = Parser.getTok().getLoc();
-  if (getParser().parseExpression(Expr))
+  unsigned Base, Index;
+  const MCExpr *Expr;
+  if (parseAddress(Base, Expr, Index, SystemZMC::GR64Regs,
+                   SystemZOperand::ADDR64Reg, true))
     return true;
 
   SMLoc EndLoc =
     SMLoc::getFromPointer(Parser.getTok().getLoc().getPointer() - 1);
-  Operands.push_back(SystemZOperand::createImm(Expr, StartLoc, EndLoc));
+  if (Base || Index)
+    Operands.push_back(SystemZOperand::createInvalid(StartLoc, EndLoc));
+  else
+    Operands.push_back(SystemZOperand::createImm(Expr, StartLoc, EndLoc));
   return false;
 }
 
@@ -683,13 +708,17 @@ MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
 
 SystemZAsmParser::OperandMatchResultTy SystemZAsmParser::
 parseAccessReg(SmallVectorImpl<MCParsedAsmOperand*> &Operands) {
+  if (Parser.getTok().isNot(AsmToken::Percent))
+    return MatchOperand_NoMatch;
+
   Register Reg;
-  OperandMatchResultTy Result = parseRegister(Reg, RegAccess, 0);
-  if (Result == MatchOperand_Success)
-    Operands.push_back(SystemZOperand::createAccessReg(Reg.Num,
-                                                       Reg.StartLoc,
-                                                       Reg.EndLoc));
-  return Result;
+  if (parseRegister(Reg, RegAccess, 0))
+    return MatchOperand_ParseFail;
+
+  Operands.push_back(SystemZOperand::createAccessReg(Reg.Num,
+                                                     Reg.StartLoc,
+                                                     Reg.EndLoc));
+  return MatchOperand_Success;
 }
 
 SystemZAsmParser::OperandMatchResultTy SystemZAsmParser::
index ec6751af840ace0a3d0c363f9dc9f88bbdb96afe..65720578ff8fa0482e761358d2b62226947e16c7 100644 (file)
@@ -13,7 +13,7 @@
 #CHECK: lr     %r0,%a1
 #CHECK: error: invalid operand for instruction
 #CHECK: lr     %r0,0
-#CHECK: error: unexpected token in argument list
+#CHECK: error: invalid operand for instruction
 #CHECK: lr     %r0,0(%r1)
 
        lr      %f0,%r1
@@ -35,7 +35,7 @@
 #CHECK: lgr    %r0,%a1
 #CHECK: error: invalid operand for instruction
 #CHECK: lgr    %r0,0
-#CHECK: error: unexpected token in argument list
+#CHECK: error: invalid operand for instruction
 #CHECK: lgr    %r0,0(%r1)
 
        lgr     %f0,%r1
@@ -73,7 +73,7 @@
 #CHECK: dlr    %r0,%a1
 #CHECK: error: invalid operand for instruction
 #CHECK: dlr    %r0,0
-#CHECK: error: unexpected token in argument list
+#CHECK: error: invalid operand for instruction
 #CHECK: dlr    %r0,0(%r1)
 
        dlr     %r1,%r0
 #CHECK: ler    %f0,%a1
 #CHECK: error: invalid operand for instruction
 #CHECK: ler    %f0,0
-#CHECK: error: unexpected token in argument list
+#CHECK: error: invalid operand for instruction
 #CHECK: ler    %f0,0(%r1)
 
        ler     %r0,%f1
 #CHECK: ldr    %f0,%a1
 #CHECK: error: invalid operand for instruction
 #CHECK: ldr    %f0,0
-#CHECK: error: unexpected token in argument list
+#CHECK: error: invalid operand for instruction
 #CHECK: ldr    %f0,0(%r1)
 
        ldr     %r0,%f1
 #CHECK: lxr    %f0,%a1
 #CHECK: error: invalid operand for instruction
 #CHECK: lxr    %f0,0
-#CHECK: error: unexpected token in argument list
+#CHECK: error: invalid operand for instruction
 #CHECK: lxr    %f0,0(%r1)
 
        lxr     %f2,%f0
 #CHECK: ear    %r0,%f0
 #CHECK: error: invalid operand for instruction
 #CHECK: ear    %r0,0
-#CHECK: error: unexpected token in argument list
+#CHECK: error: invalid operand for instruction
 #CHECK: ear    %r0,0(%r1)
 
        ear     %r0,%r0
diff --git a/test/MC/SystemZ/tokens.s b/test/MC/SystemZ/tokens.s
new file mode 100644 (file)
index 0000000..07b29d8
--- /dev/null
@@ -0,0 +1,70 @@
+# RUN: not llvm-mc -triple s390x-linux-gnu < %s 2> %t
+# RUN: FileCheck < %t %s
+
+#CHECK: error: invalid instruction
+#CHECK: foo    100, 200
+#CHECK: error: register expected
+#CHECK: foo    100(, 200
+#CHECK: error: register expected
+#CHECK: foo    100(0), 200
+#CHECK: error: invalid operand
+#CHECK: foo    100(%a0), 200
+#CHECK: error: %r0 used in an address
+#CHECK: foo    100(%r0), 200
+#CHECK: error: invalid operand
+#CHECK: foo    100(%r1,%a0), 200
+#CHECK: error: %r0 used in an address
+#CHECK: foo    100(%r1,%r0), 200
+#CHECK: error: unexpected token in address
+#CHECK: foo    100(%r1,%r2, 200
+#CHECK: error: invalid instruction
+#CHECK: foo    100(%r1,%r2), 200
+#CHECK: error: unexpected token in argument list
+#CHECK: foo    100(%r1,%r2)(, 200
+#CHECK: error: invalid instruction
+#CHECK: foo    %r0, 200
+#CHECK: error: invalid instruction
+#CHECK: foo    %r15, 200
+#CHECK: error: invalid register
+#CHECK: foo    %r16, 200
+#CHECK: error: invalid instruction
+#CHECK: foo    %f0, 200
+#CHECK: error: invalid instruction
+#CHECK: foo    %f15, 200
+#CHECK: error: invalid register
+#CHECK: foo    %f16, 200
+#CHECK: error: invalid instruction
+#CHECK: foo    %a0, 200
+#CHECK: error: invalid instruction
+#CHECK: foo    %a15, 200
+#CHECK: error: invalid register
+#CHECK: foo    %a16, 200
+#CHECK: error: invalid register
+#CHECK: foo    %c, 200
+#CHECK: error: invalid register
+#CHECK: foo    %, 200
+#CHECK: error: unknown token in expression
+#CHECK: foo    {, 200
+
+       foo     100, 200
+       foo     100(, 200
+       foo     100(0), 200
+       foo     100(%a0), 200
+       foo     100(%r0), 200
+       foo     100(%r1,%a0), 200
+       foo     100(%r1,%r0), 200
+       foo     100(%r1,%r2, 200
+       foo     100(%r1,%r2), 200
+       foo     100(%r1,%r2)(, 200
+       foo     %r0, 200
+       foo     %r15, 200
+       foo     %r16, 200
+       foo     %f0, 200
+       foo     %f15, 200
+       foo     %f16, 200
+       foo     %a0, 200
+       foo     %a15, 200
+       foo     %a16, 200
+       foo     %c, 200
+       foo     %, 200
+       foo     {, 200