IR: Disallow complicated function-local metadata

Disallow complex types of function-local metadata.  The only valid
function-local metadata is an `MDNode` whose sole argument is a
non-metadata function-local value.

Part of PR21532.

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

lib/AsmParser/LLParser.cpp

@@ -4668,7 +4668,11 @@ bool LLParser::ParseMDNodeVector(SmallVectorImpl<Value*> &Elts,
   if (Lex.getKind() == lltok::rbrace)
     return false;
 
+  bool IsLocal = false;
   do {
+    if (IsLocal)
+      return TokError("unexpected operand after function-local metadata");
+
     // Null is a special case since it is typeless.
     if (EatIfPresent(lltok::kw_null)) {
       Elts.push_back(nullptr);
@@ -4678,6 +4682,15 @@ bool LLParser::ParseMDNodeVector(SmallVectorImpl<Value*> &Elts,
     Value *V = nullptr;
     if (ParseTypeAndValue(V, PFS)) return true;
     Elts.push_back(V);
+
+    if (isa<MDNode>(V) && cast<MDNode>(V)->isFunctionLocal())
+      return TokError("unexpected nested function-local metadata");
+    if (!V->getType()->isMetadataTy() && !isa<Constant>(V)) {
+      assert(PFS && "Unexpected function-local metadata without PFS");
+      if (Elts.size() > 1)
+        return TokError("unexpected function-local metadata");
+      IsLocal = true;
+    }
   } while (EatIfPresent(lltok::comma));
 
   return false;

lib/Bitcode/Reader/BitcodeReader.cpp

@@ -1072,7 +1072,6 @@ std::error_code BitcodeReader::ParseMetadata() {
       break;
     }
 
-    bool IsFunctionLocal = false;
     // Read a record.
     Record.clear();
     unsigned Code = Stream.readRecord(Entry.ID, Record);
@@ -1100,9 +1099,34 @@ std::error_code BitcodeReader::ParseMetadata() {
       }
       break;
     }
-    case bitc::METADATA_FN_NODE:
-      IsFunctionLocal = true;
-      // fall-through
+    case bitc::METADATA_FN_NODE: {
+      // This is a function-local node.
+      if (Record.size() % 2 == 1)
+        return Error(BitcodeError::InvalidRecord);
+
+      // If this isn't a single-operand node that directly references
+      // non-metadata, we're dropping it.  This used to be legal, but there's
+      // no upgrade path.
+      auto dropRecord = [&] {
+        MDValueList.AssignValue(MDNode::get(Context, None), NextMDValueNo++);
+      };
+      if (Record.size() != 2) {
+        dropRecord();
+        break;
+      }
+
+      Type *Ty = getTypeByID(Record[0]);
+      if (Ty->isMetadataTy() || Ty->isVoidTy()) {
+        dropRecord();
+        break;
+      }
+
+      Value *Elts[] = {ValueList.getValueFwdRef(Record[1], Ty)};
+      Value *V = MDNode::getWhenValsUnresolved(Context, Elts,
+                                               /*IsFunctionLocal*/ true);
+      MDValueList.AssignValue(V, NextMDValueNo++);
+      break;
+    }
     case bitc::METADATA_NODE: {
       if (Record.size() % 2 == 1)
         return Error(BitcodeError::InvalidRecord);
@@ -1120,8 +1144,8 @@ std::error_code BitcodeReader::ParseMetadata() {
         else
           Elts.push_back(nullptr);
       }
-      Value *V = MDNode::getWhenValsUnresolved(Context, Elts, IsFunctionLocal);
-      IsFunctionLocal = false;
+      Value *V = MDNode::getWhenValsUnresolved(Context, Elts,
+                                               /*IsFunctionLocal*/ false);
       MDValueList.AssignValue(V, NextMDValueNo++);
       break;
     }

lib/IR/Metadata.cpp

@@ -185,43 +185,20 @@ static const Function *getFunctionForValue(Value *V) {
   return nullptr;
 }
 
-#ifndef NDEBUG
-static const Function *assertLocalFunction(const MDNode *N) {
-  if (!N->isFunctionLocal()) return nullptr;
-
-  // FIXME: This does not handle cyclic function local metadata.
-  const Function *F = nullptr, *NewF = nullptr;
-  for (unsigned i = 0, e = N->getNumOperands(); i != e; ++i) {
-    if (Value *V = N->getOperand(i)) {
-      if (MDNode *MD = dyn_cast<MDNode>(V))
-        NewF = assertLocalFunction(MD);
-      else
-        NewF = getFunctionForValue(V);
-    }
-    if (!F)
-      F = NewF;
-    else
-      assert((NewF == nullptr || F == NewF) &&
-             "inconsistent function-local metadata");
-  }
-  return F;
-}
-#endif
-
 // getFunction - If this metadata is function-local and recursively has a
 // function-local operand, return the first such operand's parent function.
 // Otherwise, return null. getFunction() should not be used for performance-
 // critical code because it recursively visits all the MDNode's operands.  
 const Function *MDNode::getFunction() const {
-#ifndef NDEBUG
-  return assertLocalFunction(this);
-#else
-  if (!isFunctionLocal()) return nullptr;
-  for (unsigned i = 0, e = getNumOperands(); i != e; ++i)
-    if (const Function *F = getFunctionForValue(getOperand(i)))
-      return F;
-  return nullptr;
-#endif
+  if (!isFunctionLocal())
+    return nullptr;
+  assert(getNumOperands() == 1 &&
+         "Expected one operand for function-local metadata");
+  assert(getOperand(0) &&
+         "Expected non-null operand for function-local metadata");
+  assert(!getOperand(0)->getType()->isMetadataTy() &&
+         "Expected non-metadata as operand of function-local metadata");
+  return getFunctionForValue(getOperand(0));
 }
 
 /// \brief Check if the Value  would require a function-local MDNode.
@@ -260,6 +237,14 @@ MDNode *MDNode::getMDNode(LLVMContext &Context, ArrayRef<Value*> Vals,
     break;
   }
 
+  if (isFunctionLocal) {
+    assert(Vals.size() == 1 &&
+           "Expected exactly one operand for function-local metadata");
+    assert(Vals[0] && "Expected non-null operand for function-local metadata");
+    assert(!Vals[0]->getType()->isMetadataTy() &&
+           "Expected non-metadata as operand of function-local metadata");
+  }
+
   // Coallocate space for the node and Operands together, then placement new.
   GenericMDNode *N =
       new (Vals.size()) GenericMDNode(Context, Vals, isFunctionLocal);
@@ -323,6 +308,8 @@ void MDNode::replaceOperand(MDNodeOperand *Op, Value *To) {
   // Handle this case by implicitly dropping the MDNode reference to null.
   // Likewise if the MDNode is function-local but for a different function.
   if (To && isFunctionLocalValue(To)) {
+    assert(!To->getType()->isMetadataTy() &&
+           "Expected non-metadata as operand of function-local metadata");
     if (!isFunctionLocal())
       To = nullptr;
     else {
@@ -338,6 +325,14 @@ void MDNode::replaceOperand(MDNodeOperand *Op, Value *To) {
   if (From == To)
     return;
 
+  // If this MDValue was previously function-local but no longer is, clear
+  // its function-local flag.
+  if (isFunctionLocal() && !(To && isFunctionLocalValue(To))) {
+    assert(getNumOperands() == 1 &&
+           "Expected function-local metadata to have exactly one operand");
+    setValueSubclassData(getSubclassDataFromValue() & ~FunctionLocalBit);
+  }
+
   // If this node is already not being uniqued (because one of the operands
   // already went to null), then there is nothing else to do here.
   if (isNotUniqued()) {
@@ -377,22 +372,6 @@ void MDNode::replaceOperand(MDNodeOperand *Op, Value *To) {
 
   N->Hash = Key.Hash;
   Store.insert(N);
-
-  // If this MDValue was previously function-local but no longer is, clear
-  // its function-local flag.
-  if (isFunctionLocal() && !isFunctionLocalValue(To)) {
-    bool isStillFunctionLocal = false;
-    for (unsigned i = 0, e = getNumOperands(); i != e; ++i) {
-      Value *V = getOperand(i);
-      if (!V) continue;
-      if (isFunctionLocalValue(V)) {
-        isStillFunctionLocal = true;
-        break;
-      }
-    }
-    if (!isStillFunctionLocal)
-      setValueSubclassData(getSubclassDataFromValue() & ~FunctionLocalBit);
-  }
 }
 
 MDNode *MDNode::concatenate(MDNode *A, MDNode *B) {

test/Assembler/functionlocal-metadata.ll

@@ -13,19 +13,17 @@ entry:
 ; CHECK: call void @llvm.dbg.declare(metadata !{i32* %1}, metadata !{i32* %1}, metadata {{.*}})
   call void @llvm.dbg.declare(metadata !{i32 %two}, metadata !{i32 %0}, metadata !{metadata !"0x102"})
 ; CHECK: call void @llvm.dbg.declare(metadata !{i32 %two}, metadata !{i32 %0}, metadata {{.*}})
-  call void @llvm.dbg.declare(metadata !{i32 %0}, metadata !{i32* %1, i32 %0}, metadata !{metadata !"0x102"})
-; CHECK: call void @llvm.dbg.declare(metadata !{i32 %0}, metadata !{i32* %1, i32 %0}, metadata {{.*}})
-  call void @llvm.dbg.declare(metadata !{i32* %1}, metadata !{i32 %b, i32 %0}, metadata !{metadata !"0x102"})
-; CHECK: call void @llvm.dbg.declare(metadata !{i32* %1}, metadata !{i32 %b, i32 %0}, metadata {{.*}})
-  call void @llvm.dbg.declare(metadata !{i32 %a}, metadata !{i32 %a, metadata !"foo"}, metadata !{metadata !"0x102"})
-; CHECK: call void @llvm.dbg.declare(metadata !{i32 %a}, metadata !{i32 %a, metadata !"foo"}, metadata {{.*}})
-  call void @llvm.dbg.declare(metadata !{i32 %b}, metadata !{metadata !0, i32 %two}, metadata !{metadata !"0x102"})
-; CHECK: call void @llvm.dbg.declare(metadata !{i32 %b}, metadata !{metadata ![[ID0:[0-9]+]], i32 %two}, metadata {{.*}})
+  call void @llvm.dbg.declare(metadata !{i32* %1}, metadata !{i32 %b}, metadata !{metadata !"0x102"})
+; CHECK: call void @llvm.dbg.declare(metadata !{i32* %1}, metadata !{i32 %b}, metadata {{.*}})
+  call void @llvm.dbg.declare(metadata !{i32 %a}, metadata !{i32 %a}, metadata !{metadata !"0x102"})
+; CHECK: call void @llvm.dbg.declare(metadata !{i32 %a}, metadata !{i32 %a}, metadata {{.*}})
+  call void @llvm.dbg.declare(metadata !{i32 %b}, metadata !{i32 %two}, metadata !{metadata !"0x102"})
+; CHECK: call void @llvm.dbg.declare(metadata !{i32 %b}, metadata !{i32 %two}, metadata {{.*}})
 
   call void @llvm.dbg.value(metadata !{ i32 %a }, i64 0, metadata !1, metadata !{metadata !"0x102"})
 ; CHECK: call void @llvm.dbg.value(metadata !{i32 %a}, i64 0, metadata ![[ID1:[0-9]+]], metadata {{.*}})
   call void @llvm.dbg.value(metadata !{ i32 %0 }, i64 25, metadata !0, metadata !{metadata !"0x102"})
-; CHECK: call void @llvm.dbg.value(metadata !{i32 %0}, i64 25, metadata ![[ID0]], metadata {{.*}})
+; CHECK: call void @llvm.dbg.value(metadata !{i32 %0}, i64 25, metadata ![[ID0:[0-9]+]], metadata {{.*}})
   call void @llvm.dbg.value(metadata !{ i32* %1 }, i64 16, metadata !3, metadata !{metadata !"0x102"})
 ; CHECK: call void @llvm.dbg.value(metadata !{i32* %1}, i64 16, metadata ![[ID3:[0-9]+]], metadata {{.*}})
   call void @llvm.dbg.value(metadata !3, i64 12, metadata !2, metadata !{metadata !"0x102"})

test/Feature/metadata.ll

@@ -4,7 +4,7 @@
 define void @foo(i32 %x) {
   call void @llvm.zonk(metadata !1, i64 0, metadata !1)
   store i32 0, i32* null, !whatever !0, !whatever_else !{}, !more !{metadata !"hello"}
-  store i32 0, i32* null, !whatever !{i32 %x, metadata !"hello", metadata !1, metadata !{}, metadata !2}
+  store i32 0, i32* null, !whatever !{metadata !"hello", metadata !1, metadata !{}, metadata !2}
   ret void, !whatever !{i32 %x}
 }
 

test/Transforms/GlobalOpt/metadata.ll

@@ -13,14 +13,15 @@ define i32 @main(i32 %argc, i8** %argv) {
 }
 
 define void @foo(i32 %x) {
-  call void @llvm.foo(metadata !{i8*** @G, i32 %x})
-; CHECK: call void @llvm.foo(metadata !{null, i32 %x})
+  call void @llvm.foo(metadata !{i8*** @G})
+; CHECK: call void @llvm.foo(metadata !0)
   ret void
 }
 
 declare void @llvm.foo(metadata) nounwind readnone
 
 !named = !{!0}
+; CHECK: !named = !{!0}
 
 !0 = metadata !{i8*** @G}
 ; CHECK: !0 = metadata !{null}