[IR/AsmWriter] Output escape sequences if the first character isdigit()

[filcab]

Summary

If the first character in a metadata attachment's name is a digit, it has
to be output using an escape sequence, otherwise it's not valid text IR.

Removed an over-zealous assert from LLVMContext which didn't allow this.
The rule should only apply to text IR. Actual names can have any sequence
of non-NUL bytes.

Also added some documentation on accepted names.

Bug found with AFL fuzz.

[filcab]

I could add some more stuff to FileCheck in that test, but am not 100% sure on what the best way to do it.

If it's safe to assume the output it currently has (!{} becomes !2, and some other "abbreviations" appear), I can do a verbatim check, but it would basically be checking copy+paste.
Comments on this?

[• rafael]

docs/LangRef.rst

840

Maybe mention that you can still use escapes, so in the end we support any name but have a different syntax in the .ll

lib/IR/AsmWriter.cpp

2218–2219

Start with a lowercase letter.

2223–2224

This is just clang-format, right? Please commit it first.

lib/IR/LLVMContext.cpp

246–248

Commit the clang-format bits first.

[filcab]

• clang-format a few functions. NFC
• [IR/AsmWriter] Output escape sequences if the first character isdigit()

Split the patch in two and addressed comments.

[filcab]

• [IR/AsmWriter] Output escape sequences if the first character isdigit()

printMetadataIdentifier now actually starts with a lowercase letter

[filcab]

• CHECK-LABEL-ize test. NFC

Move test to test/Assembler/metadata.ll

[kschimpf]

LGTM.

[filcab]

This got committed as r238865, r238866, and r238867.
Phab didn't pick it up, though. Closing.

docs/LangRef.rst

 
 Named Metadata
 --------------
 
 Named metadata is a collection of metadata. :ref:`Metadata
 nodes <metadata>` (but not metadata strings) are the only valid
 operands for a named metadata.
 
+#. Named metadata are represented as a string of characters with the
+   metadata prefix. The rules for metadata names are the same as for
+   identifiers, but quoted names are not allowed. ``"\xx"`` type escapes
+   are still valid, which allows any character to be part of a name.
+
 Syntax::
 
     ; Some unnamed metadata nodes, which are referenced by the named metadata.
     !0 = !{!"zero"}
     !1 = !{!"one"}
     !2 = !{!"two"}
     ; A named metadata.
     !name = !{!0, !1, !2}

lib/IR/AsmWriter.cpp

void AssemblyWriter::printModule(const Module *M) {
 
   // Output metadata.
   if (!Machine.mdn_empty()) {
     Out << '\n';
     writeAllMDNodes();
   }
 }
 
-void AssemblyWriter::printNamedMDNode(const NamedMDNode *NMD) {
-  Out << '!';
-  StringRef Name = NMD->getName();
+static void printMetadataIdentifier(StringRef Name,
+                                    formatted_raw_ostream &Out) {
   if (Name.empty()) {
     Out << "<empty name> ";
   } else {
-    if (isalpha(static_cast<unsigned char>(Name[0])) ||
-        Name[0] == '-' || Name[0] == '$' ||
-        Name[0] == '.' || Name[0] == '_')
+    if (isalpha(static_cast<unsigned char>(Name[0])) || Name[0] == '-' ||
+        Name[0] == '$' || Name[0] == '.' || Name[0] == '_')
       Out << Name[0];
     else
       Out << '\\' << hexdigit(Name[0] >> 4) << hexdigit(Name[0] & 0x0F);
     for (unsigned i = 1, e = Name.size(); i != e; ++i) {
       unsigned char C = Name[i];
       if (isalnum(static_cast<unsigned char>(C)) || C == '-' || C == '$' ||
           C == '.' || C == '_')
         Out << C;
       else
         Out << '\\' << hexdigit(C >> 4) << hexdigit(C & 0x0F);
     }
   }
+}
+
+void AssemblyWriter::printNamedMDNode(const NamedMDNode *NMD) {
+  Out << '!';
+  printMetadataIdentifier(NMD->getName(), Out);
   Out << " = !{";
   for (unsigned i = 0, e = NMD->getNumOperands(); i != e; ++i) {
-    if (i) Out << ", ";
+    if (i)
+      Out << ", ";
     int Slot = Machine.getMetadataSlot(NMD->getOperand(i));
     if (Slot == -1)
       Out << "<badref>";
     else
       Out << '!' << Slot;
   }
   Out << "}\n";
 }
 
-
 static void PrintLinkage(GlobalValue::LinkageTypes LT,
                          formatted_raw_ostream &Out) {
   switch (LT) {
   case GlobalValue::ExternalLinkage: break;
   case GlobalValue::PrivateLinkage:       Out << "private ";        break;
   case GlobalValue::InternalLinkage:      Out << "internal ";       break;
   case GlobalValue::LinkOnceAnyLinkage:   Out << "linkonce ";       break;
   case GlobalValue::LinkOnceODRLinkage:   Out << "linkonce_odr ";   break;
   case GlobalValue::WeakAnyLinkage:       Out << "weak ";           break;
   case GlobalValue::WeakODRLinkage:       Out << "weak_odr ";       break;
   case GlobalValue::CommonLinkage:        Out << "common ";         break;
   case GlobalValue::AppendingLinkage:     Out << "appending ";      break;
   case GlobalValue::ExternalWeakLinkage:  Out << "extern_weak ";    break;
   case GlobalValue::AvailableExternallyLinkage:
     Out << "available_externally ";
     break;
   }
 }
 
-
 static void PrintVisibility(GlobalValue::VisibilityTypes Vis,
                             formatted_raw_ostream &Out) {
   switch (Vis) {
   case GlobalValue::DefaultVisibility: break;
   case GlobalValue::HiddenVisibility:    Out << "hidden "; break;
   case GlobalValue::ProtectedVisibility: Out << "protected "; break;
   }
 }
if (MDs.empty())
     return;
 
   if (MDNames.empty())
     TheModule->getMDKindNames(MDNames);
 
   for (const auto &I : MDs) {
     unsigned Kind = I.first;
     Out << Separator;
-    if (Kind < MDNames.size())
-      Out << "!" << MDNames[Kind];
-    else
+    if (Kind < MDNames.size()) {
+      Out << "!";
+      printMetadataIdentifier(MDNames[Kind], Out);
+    } else
       Out << "!<unknown kind #" << Kind << ">";
     Out << ' ';
     WriteAsOperandInternal(Out, I.second, &TypePrinter, &Machine, TheModule);
   }
 }
 
 void AssemblyWriter::writeMDNode(unsigned Slot, const MDNode *Node) {
   Out << '!' << Slot << " = ";

lib/IR/LLVMContext.cpp

 void LLVMContext::emitError(unsigned LocCookie, const Twine &ErrorStr) {
   diagnose(DiagnosticInfoInlineAsm(LocCookie, ErrorStr));
 }
 
 //===----------------------------------------------------------------------===//
 // Metadata Kind Uniquing
 //===----------------------------------------------------------------------===//
 
-/// getMDKindID - Return a unique non-zero ID for the specified metadata kind.
+/// Return a unique non-zero ID for the specified metadata kind.
 unsigned LLVMContext::getMDKindID(StringRef Name) const {
-  assert(!std::isdigit(Name.front()) &&
-         "Named metadata may not start with a digit");
-
   // If this is new, assign it its ID.
-  return pImpl->CustomMDKindNames.insert(std::make_pair(
-                                             Name,
-                                             pImpl->CustomMDKindNames.size()))
+  return pImpl->CustomMDKindNames.insert(
+                                     std::make_pair(
+                                         Name, pImpl->CustomMDKindNames.size()))
       .first->second;
 }
 
 /// getHandlerNames - Populate client supplied smallvector using custome
 /// metadata name and ID.
 void LLVMContext::getMDKindNames(SmallVectorImpl<StringRef> &Names) const {
   Names.resize(pImpl->CustomMDKindNames.size());
   for (StringMap<unsigned>::const_iterator I = pImpl->CustomMDKindNames.begin(),
        E = pImpl->CustomMDKindNames.end(); I != E; ++I)
     Names[I->second] = I->first();
 }

test/Assembler/metadata.ll

 ; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | FileCheck %s
 ; RUN: verify-uselistorder %s
 
-; CHECK: @test
+; CHECK-LABEL: @test
 ; CHECK: ret void, !bar !1, !foo !0
 define void @test() {
   add i32 2, 1, !bar !0
   add i32 1, 2, !foo !1
   call void @llvm.dbg.func.start(metadata !"foo")
   extractvalue {{i32, i32}, i32} undef, 0, 1, !foo !0
   ret void, !foo !0, !bar !1
 }
 
-; CHECK: define void @test2() !foo !2 !baz !3
+; CHECK-LABEL: define void @test2() !foo !2 !baz !3
 define void @test2() !foo !2 !baz !3 {
   unreachable
 }
 
-; CHECK: define void @test3() !bar !3
+; CHECK-LABEL: define void @test3() !bar !3
 ; CHECK: unreachable, !bar !4
 define void @test3() !bar !3 {
   unreachable, !bar !4
 }
 
+; CHECK-LABEL: define void @test_attachment_name() {
+; CHECK:   unreachable, !\342abc !4
+define void @test_attachment_name() {
+  ;; Escape the first character when printing text IR, since it's a digit
+  unreachable, !\34\32abc !4
+}
+
 !0 = !DILocation(line: 662302, column: 26, scope: !1)
 !1 = !DISubprogram(name: "foo")
 !2 = distinct !{}
 !3 = distinct !{}
 !4 = distinct !{}
 
 declare void @llvm.dbg.func.start(metadata) nounwind readnone
 
 !foo = !{ !0 }
 !bar = !{ !1 }