[AsmParser] Check that addrspace fits within 24 bits

[luke]

Summary

Address spaces equal or larger than 1 << 24 don't fit and produce an
assertion during debug builds, or worse in release. This causes an error
to be reported during parsing instead.

[luke]

Adding tests that were forgotten

[arichardson]

Minor suggestions inline. Would you mind rebasing this on https://reviews.llvm.org/D138789 (which I will commit once approved). Should make the diff slightly smaller.

llvm/lib/AsmParser/LLParser.cpp

1824–1825

To match the logic in DataLayout.cpp. However, ideally we should have a shared constant somewhere instead of duplicating this knowledge.

[luke]

Rebased upon D138789

[asb]

llvm/lib/AsmParser/LLParser.cpp

1812

Nit: I think "24-bit" would be consistent with similar strings in LLVM.

[luke]

Adjust diagnostic string

[nikic]

llvm/test/Assembler/invalid-addrspace.ll

6

Does not match the updated diagnostic.

[luke]

Update test with new diagnostic

[nikic]

LGTM

llvm/lib/AsmParser/LLParser.cpp

if (Lex.getKind() == lltok::StringConstant) {
         AddrSpace = M->getDataLayout().getDefaultGlobalsAddressSpace();
       } else if (AddrSpaceStr == "P") {
         AddrSpace = M->getDataLayout().getProgramAddressSpace();
       } else {
         return tokError("invalid symbolic addrspace '" + AddrSpaceStr + "'");
       }
       Lex.Lex();
       return false;
-    } else if (Lex.getKind() != lltok::APSInt) {
-      return tokError("expected integer or string constant");
     }
-    return parseUInt32(AddrSpace);
+    if (Lex.getKind() != lltok::APSInt)
+      return tokError("expected integer or string constant");
+    SMLoc Loc = Lex.getLoc();
+    if (parseUInt32(AddrSpace))
+      return true;
+    if (!isUInt<24>(AddrSpace))
+      return error(Loc, "invalid address space, must be a 24-bit integer");
+    return false;
   };
 
   return parseToken(lltok::lparen, "expected '(' in address space") ||
          ParseAddrspaceValue(AddrSpace) ||
          parseToken(lltok::rparen, "expected ')' in address space");
 }
 
 /// parseStringAttribute
 ///   := StringConstant
 ///   := StringConstant '=' StringConstant
 bool LLParser::parseStringAttribute(AttrBuilder &B) {
   std::string Attr = Lex.getStrVal();
   Lex.Lex();
   std::string Val;
   if (EatIfPresent(lltok::equal) && parseStringConstant(Val))
     return true;
   B.addAttribute(Attr, Val);
   return false;
 }
 

llvm/test/Assembler/invalid-addrspace.ll

+; RUN: not llvm-as < %s 2>&1 | FileCheck %s
+; Check that parser rejects address spaces that are too large to fit in the 24 bits
+
+define void @f() {
+; CHECK: invalid address space, must be a 24-bit integer
+  %y = alloca i32, addrspace(16777216)
+  ret void
+}