[ms] [llvm-ml] Accept whitespace around the dot operator

[epastor]

Summary

MASM allows arbitrary whitespace around the Intel dot operator, especially when used for struct field lookup

[rnk]

This code is already so convoluted that I don't feel like I can provide very good code review for it. More tech debt can't hurt here, this codepath is already overleveraged with tech debt and needs to be rewritten. But if you want to push on through and add the functionality, it has good tests, so go for it.

llvm/lib/MC/MCParser/MasmParser.cpp

1383

Please remote the stray whitespace change.

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

1706

My suggestion to try to simplify this code would be to use .slice to build two StringRefs, then check if they are non-empty, and unlex them if needed:

StringRef LHS = Identifer.slice(0, dotOffset);
StringRef RHS = Identifer.slice(dotOffset+1, Identifer.size());
if (LHS.empty())
  .. UnLex LHS
.. Unlex dot
if (RHS.empty())
  .. UnLex RHS

That keeps all the index math closer together, less spread out.

1707

Truly, breaking up identifiers into small chunks is the job of the lexer, but here we are anyway, breaking things up and "unlexing" them.

[epastor]

Clean up identifier slicing (which seems sadly necessary)

[epastor]

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

1706

Done, thanks! (It's RHS first, since UnLex treats the lexer as a stack.)

1707

Yep. *sigh*

[rnk]

lgtm, thanks!

[epastor]

Rebase on parent

llvm/lib/MC/MCParser/MasmParser.cpp

case AsmToken::Identifier: {
     }
     // Parse named bitwise negation.
     if (Identifier.equals_lower("not")) {
       if (parsePrimaryExpr(Res, EndLoc, nullptr))
         return true;
       Res = MCUnaryExpr::createNot(Res, getContext(), FirstTokenLoc);
       return false;
     }
     // Parse symbol variant.
     std::pair<StringRef, StringRef> Split;
     if (!MAI.useParensForSymbolVariant()) {
       if (FirstTokenKind == AsmToken::String) {
         if (Lexer.is(AsmToken::At)) {
           Lex(); // eat @
           SMLoc AtLoc = getLexer().getLoc();
           StringRef VName;
           if (parseIdentifier(VName))
bool MasmParser::lookUpField(const StructInfo &Structure, StringRef Member,
   const FieldInfo &Field = Structure.Fields[FieldIt->second];
   if (FieldMember.empty()) {
     Info.Offset += Field.Offset;
     Info.Type.Size = Field.SizeOf;
     Info.Type.ElementSize = Field.Type;
     Info.Type.Length = Field.LengthOf;
     if (Field.Contents.FT == FT_STRUCT)
       Info.Type.Name = Field.Contents.StructInfo.Structure.Name;
+    else
+      Info.Type.Name = "";
     return false;
   }
 
   if (Field.Contents.FT != FT_STRUCT)
     return true;
   const StructFieldInfo &StructInfo = Field.Contents.StructInfo;
 
   if (lookUpField(StructInfo.Structure, FieldMember, Info))

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

case AsmToken::EndOfStatement:
       Done = true;
       break;
     case AsmToken::Real:
       // DotOperator: [ebx].0
       UpdateLocLex = false;
       if (ParseIntelDotOperator(SM, End))
         return true;
       break;
+    case AsmToken::Dot:
+      if (!Parser.isParsingMasm()) {
+        if ((Done = SM.isValidEndState()))
+          break;
+        return Error(Tok.getLoc(), "unknown token in expression");
+      }
+      // MASM allows spaces around the dot operator (e.g., "var . x")
+      Lex();
+      UpdateLocLex = false;
+      if (ParseIntelDotOperator(SM, End))
+        return true;
+      break;
     case AsmToken::Dollar:
       if (!Parser.isParsingMasm()) {
         if ((Done = SM.isValidEndState()))
           break;
         return Error(Tok.getLoc(), "unknown token in expression");
       }
       LLVM_FALLTHROUGH;
     case AsmToken::At:
     case AsmToken::String:
     case AsmToken::Identifier: {
       SMLoc IdentLoc = Tok.getLoc();
       StringRef Identifier = Tok.getString();
       UpdateLocLex = false;
+      if (Parser.isParsingMasm()) {
+        size_t DotOffset = Identifier.find_first_of('.');
+        if (DotOffset != StringRef::npos) {
+          consumeToken();
+          StringRef LHS = Identifier.slice(0, DotOffset);
+          StringRef Dot = Identifier.slice(DotOffset, DotOffset + 1);
+          StringRef RHS = Identifier.slice(DotOffset + 1, StringRef::npos);
+          if (!RHS.empty()) {
+            getLexer().UnLex(AsmToken(AsmToken::Identifier, RHS));
+          }
+          getLexer().UnLex(AsmToken(AsmToken::Dot, Dot));
+          if (!LHS.empty()) {
+            getLexer().UnLex(AsmToken(AsmToken::Identifier, LHS));
+          }
+          break;
+        }
+      }
       // (MASM only) <TYPE> PTR operator
       if (Parser.isParsingMasm()) {
         const AsmToken &NextTok = getLexer().peekTok();
         if (NextTok.is(AsmToken::Identifier) &&
             NextTok.getIdentifier().equals_lower("ptr")) {
           AsmTypeInfo Info;
           if (Parser.lookUpType(Identifier, Info))
             return Error(Tok.getLoc(), "unknown type");
case AsmToken::Identifier: {
       bool ParseError = false;
       if (ParseIntelNamedOperator(Identifier, SM, ParseError, End)) {
         if (ParseError)
           return true;
         break;
       }
       // Symbol reference, when parsing assembly content
       InlineAsmIdentifierInfo Info;
-      AsmTypeInfo Type;
+      AsmFieldInfo FieldInfo;
       const MCExpr *Val;
       if (isParsingMSInlineAsm() || Parser.isParsingMasm()) {
         // MS Dot Operator expression
         if (Identifier.count('.') &&
             (PrevTK == AsmToken::RBrac || PrevTK == AsmToken::RParen)) {
           if (ParseIntelDotOperator(SM, End))
             return true;
           break;
         }
       }
       if (isParsingMSInlineAsm()) {
         // MS InlineAsm operators (TYPE/LENGTH/SIZE)
         if (unsigned OpKind = IdentifyIntelInlineAsmOperator(Identifier)) {
           if (int64_t Val = ParseIntelInlineAsmOperator(OpKind)) {
             if (SM.onInteger(Val, ErrMsg))
               return Error(IdentLoc, ErrMsg);
-          } else
+          } else {
             return true;
+          }
           break;
         }
         // MS InlineAsm identifier
         // Call parseIdentifier() to combine @ with the identifier behind it.
         if (TK == AsmToken::At && Parser.parseIdentifier(Identifier))
           return Error(IdentLoc, "expected identifier");
         if (ParseIntelInlineAsmIdentifier(Val, Identifier, Info, false, End))
           return true;
-        else if (SM.onIdentifierExpr(Val, Identifier, Info, Type, true, ErrMsg))
+        else if (SM.onIdentifierExpr(Val, Identifier, Info, FieldInfo.Type,
+                                     true, ErrMsg))
           return Error(IdentLoc, ErrMsg);
         break;
       }
       if (Parser.isParsingMasm()) {
         if (unsigned OpKind = IdentifyMasmOperator(Identifier)) {
           int64_t Val;
           if (ParseMasmOperator(OpKind, Val))
             return true;
           if (SM.onInteger(Val, ErrMsg))
             return Error(IdentLoc, ErrMsg);
           break;
         }
+        if (!getParser().lookUpType(Identifier, FieldInfo.Type)) {
+          // Field offset immediate; <TYPE>.<field specification>
+          Lex(); // eat type
+          bool EndDot = parseOptionalToken(AsmToken::Dot);
+          while (EndDot || (getTok().is(AsmToken::Identifier) &&
+                            getTok().getString().startswith("."))) {
+            getParser().parseIdentifier(Identifier);
+            if (!EndDot)
+              Identifier.consume_front(".");
+            EndDot = Identifier.consume_back(".");
+            if (getParser().lookUpField(FieldInfo.Type.Name, Identifier,
+                                        FieldInfo)) {
+              SMLoc IDEnd =
+                  SMLoc::getFromPointer(Identifier.data() + Identifier.size());
+              return Error(IdentLoc, "Unable to lookup field reference!",
+                           SMRange(IdentLoc, IDEnd));
             }
-      if (getParser().parsePrimaryExpr(Val, End, &Type)) {
+            if (!EndDot)
+              EndDot = parseOptionalToken(AsmToken::Dot);
+          }
+          if (SM.onInteger(FieldInfo.Offset, ErrMsg))
+            return Error(IdentLoc, ErrMsg);
+          break;
+        }
+      }
+      if (getParser().parsePrimaryExpr(Val, End, &FieldInfo.Type)) {
         return Error(Tok.getLoc(), "Unexpected identifier!");
-      } else if (SM.onIdentifierExpr(Val, Identifier, Info, Type, false,
-                                     ErrMsg)) {
+      } else if (SM.onIdentifierExpr(Val, Identifier, Info, FieldInfo.Type,
+                                     false, ErrMsg)) {
         return Error(IdentLoc, ErrMsg);
       }
       break;
     }
     case AsmToken::Integer: {
       // Look for 'b' or 'f' following an Integer as a directional label
       SMLoc Loc = getTok().getLoc();
       int64_t IntVal = getTok().getIntVal();
bool X86AsmParser::ParseIntelDotOperator(IntelExprStateMachine &SM,
                                          SMLoc &End) {
   const AsmToken &Tok = getTok();
   AsmFieldInfo Info;
 
   // Drop the optional '.'.
   StringRef DotDispStr = Tok.getString();
   if (DotDispStr.startswith("."))
     DotDispStr = DotDispStr.drop_front(1);
+  StringRef TrailingDot;
 
   // .Imm gets lexed as a real.
   if (Tok.is(AsmToken::Real)) {
     APInt DotDisp;
     DotDispStr.getAsInteger(10, DotDisp);
     Info.Offset = DotDisp.getZExtValue();
   } else if ((isParsingMSInlineAsm() || getParser().isParsingMasm()) &&
              Tok.is(AsmToken::Identifier)) {
+    if (DotDispStr.endswith(".")) {
+      TrailingDot = DotDispStr.substr(DotDispStr.size() - 1);
+      DotDispStr = DotDispStr.drop_back(1);
+    }
     const std::pair<StringRef, StringRef> BaseMember = DotDispStr.split('.');
     const StringRef Base = BaseMember.first, Member = BaseMember.second;
     if (getParser().lookUpField(SM.getType(), DotDispStr, Info) &&
         getParser().lookUpField(SM.getSymName(), DotDispStr, Info) &&
         getParser().lookUpField(DotDispStr, Info) &&
         (!SemaCallback ||
          SemaCallback->LookupInlineAsmField(Base, Member, Info.Offset)))
       return Error(Tok.getLoc(), "Unable to lookup field reference!");
   } else {
     return Error(Tok.getLoc(), "Unexpected token type!");
   }
 
   // Eat the DotExpression and update End
   End = SMLoc::getFromPointer(DotDispStr.data());
   const char *DotExprEndLoc = DotDispStr.data() + DotDispStr.size();
   while (Tok.getLoc().getPointer() < DotExprEndLoc)
     Lex();
+  if (!TrailingDot.empty())
+    getLexer().UnLex(AsmToken(AsmToken::Dot, TrailingDot));
   SM.addImm(Info.Offset);
   SM.setTypeInfo(Info.Type);
   return false;
 }
 
 /// Parse the 'offset' operator.
 /// This operator is used to specify the location of a given operand
 bool X86AsmParser::ParseIntelOffsetOperator(const MCExpr *&Val, StringRef &ID,

llvm/test/tools/llvm-ml/dot_operator.test

+# RUN: llvm-ml -filetype=asm %s | FileCheck %s
+
+.data
+
+FOO STRUCT
+  a BYTE ?
+  b BYTE ?
+  c BYTE ?
+  d BYTE ?
+FOO ENDS
+
+BAR STRUCT
+  e WORD ?
+  f WORD ?
+BAR ENDS
+
+var FOO <>
+
+.code
+
+t1:
+mov al, var.a
+mov al, var. b
+mov al, var .c
+mov al, var . d
+
+; CHECK-LABEL: t1:
+; CHECK: mov al, byte ptr [rip + var]
+; CHECK: mov al, byte ptr [rip + var+1]
+; CHECK: mov al, byte ptr [rip + var+2]
+; CHECK: mov al, byte ptr [rip + var+3]
+
+t2:
+mov eax, FOO.a
+mov ax, FOO. b
+mov al, FOO .c
+mov eax, FOO . d
+
+; CHECK-LABEL: t2:
+; CHECK: mov eax, 0
+; CHECK: mov ax, 1
+; CHECK: mov al, 2
+; CHECK: mov eax, 3
+
+t3:
+mov al, BYTE PTR var[FOO.c]
+
+; CHECK-LABEL: t3:
+; CHECK: mov al, byte ptr [rip + var+2]
+
+t4:
+mov ax, var.BAR.f
+mov ax, var .BAR.f
+mov ax, var. BAR.f
+mov ax, var.BAR .f
+mov ax, var.BAR. f
+mov ax, var . BAR . f
+
+; CHECK-LABEL: t4:
+; CHECK: mov ax, word ptr [rip + var+2]
+; CHECK: mov ax, word ptr [rip + var+2]
+; CHECK: mov ax, word ptr [rip + var+2]
+; CHECK: mov ax, word ptr [rip + var+2]
+; CHECK: mov ax, word ptr [rip + var+2]
+; CHECK: mov ax, word ptr [rip + var+2]
+
+END

llvm/test/tools/llvm-ml/struct.test

 mov al, [t2.FOOBAR].e.b
 mov al, [t2.FOOBAR.e].b
 mov al, [t2.FOOBAR.e.b]
 
 ; CHECK: t8:
 ; CHECK-NEXT: mov al, byte ptr [rip + t2+9]
 ; CHECK-NEXT: mov al, byte ptr [rip + t2+9]
 ; CHECK-NEXT: mov al, byte ptr [rip + t2+9]
-; CHECK-NEXT: mov al, byte ptr [rip + (t2+8)+1]
+; CHECK-NEXT: mov al, byte ptr [rip + t2+9]
 ; CHECK-NEXT: mov al, byte ptr [rip + t2+9]
 
 QUUX STRUCT
   u DWORD ?
   UNION
     v WORD ?
     w DWORD ?
     STRUCT