[PATCH] D35998: [TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true

[Alexandru Guduleasa via Phabricator via llvm-commits]

alexandru.guduleasa updated this revision to Diff 109923.
alexandru.guduleasa added a comment.

I agree with the sift by one proposal and I apologize that we didn't catch this error on our end.
Apparently, there are some immediate operands that have the same value (e.g. 0).
For these, the OpIdx is not used, but it was computed anyway (and caused the out of bounds error).

I've increased the size of the array by one and I'm accessing the same index as the Operands vector.
All the operands that have 0 as the index (Tied_To, and those always-zero immediate) will no longer cause an error.


https://reviews.llvm.org/D35998

Files:
  utils/TableGen/AsmMatcherEmitter.cpp


Index: utils/TableGen/AsmMatcherEmitter.cpp
===================================================================
--- utils/TableGen/AsmMatcherEmitter.cpp
+++ utils/TableGen/AsmMatcherEmitter.cpp
@@ -1847,13 +1847,25 @@
   CvtOS << "  assert(Kind < CVT_NUM_SIGNATURES && \"Invalid signature!\");\n";
   CvtOS << "  const uint8_t *Converter = ConversionTable[Kind];\n";
   if (HasOptionalOperands) {
-    CvtOS << "  unsigned NumDefaults = 0;\n";
+    size_t MaxNumOperands = 0;
+    for (const auto &MI : Infos) {
+      MaxNumOperands = std::max(MaxNumOperands, MI->AsmOperands.size());
+    }
+    CvtOS << "  unsigned DefaultsOffset[" << (MaxNumOperands + 1)
+          << "] = { 0 };\n";
+    CvtOS << "  assert(OptionalOperandsMask.size() == " << (MaxNumOperands)
+          << ");\n";
+    CvtOS << "  for (unsigned i = 0, NumDefaults = 0; i < " << (MaxNumOperands)
+          << "; ++i) {\n";
+    CvtOS << "    DefaultsOffset[i + 1] = NumDefaults;\n";
+    CvtOS << "    NumDefaults += (OptionalOperandsMask[i] ? 1 : 0);\n";
+    CvtOS << "  }\n";
   }
   CvtOS << "  unsigned OpIdx;\n";
   CvtOS << "  Inst.setOpcode(Opcode);\n";
   CvtOS << "  for (const uint8_t *p = Converter; *p; p+= 2) {\n";
   if (HasOptionalOperands) {
-    CvtOS << "    OpIdx = *(p + 1) - NumDefaults;\n";
+    CvtOS << "    OpIdx = *(p + 1) - DefaultsOffset[*(p + 1)];\n";
   } else {
     CvtOS << "    OpIdx = *(p + 1);\n";
   }
@@ -1988,7 +2007,6 @@
                 << "        " << Op.Class->DefaultMethod << "()"
                 << "->" << Op.Class->RenderMethod << "(Inst, "
                 << OpInfo.MINumOperands << ");\n"
-                << "        ++NumDefaults;\n"
                 << "      } else {\n"
                 << "        static_cast<" << TargetOperandClass
                 << "&>(*Operands[OpIdx])." << Op.Class->RenderMethod


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D35998.109923.patch
Type: text/x-patch
Size: 1839 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170806/967737d8/attachment.bin>