[DWARFv5] Fuss with asm syntax for conveying an MD5 checksum

[probinson]

Summary

Previously this used a quoted hex string; now it's a number with 0x prefix, just like the .octa directive.

[aprantl]

Especially if there is prior art this seems fine to me, especially if we keep the leading zeros.

[echristo]

The leading zeroes look weird to me, but I think in practice it's not going to be an issue anyhow so I think this is good to throw over the wall at the binutils list if you'd like?

Going to LGTM the patch for now as I think the syntax is fine as well.

[probinson]

The leading zeros are because an MD5 hash is defined to be a 128-bit value. MD5Result::digest() returns the full 32-character hex string. I didn't see any value in trying to suppress leading zeros, it's slow and more likely to be confusing than helpful.
The parsing should automatically zero-extend a short value, and as it's identical to .octa parsing (see test\MC\AsmParser\directive_values.s) I didn't bother with one for MD5.

llvm/trunk/lib/MC/MCAsmStreamer.cpp

static void printDwarfFileDirective(unsigned FileNo, StringRef Directory,
   }
 
   OS << "\t.file\t" << FileNo << ' ';
   if (!Directory.empty()) {
     PrintQuotedString(Directory, OS);
     OS << ' ';
   }
   PrintQuotedString(Filename, OS);
-  if (Checksum) {
-    OS << " md5 ";
-    PrintQuotedString(Checksum->digest(), OS);
-  }
+  if (Checksum)
+    OS << " md5 0x" << Checksum->digest();
   if (Source) {
     OS << " source ";
     PrintQuotedString(*Source, OS);
   }
 }
 
 Expected<unsigned> MCAsmStreamer::tryEmitDwarfFileDirective(
     unsigned FileNo, StringRef Directory, StringRef Filename,

llvm/trunk/lib/MC/MCParser/AsmParser.cpp

auto parseOp = [&]() -> bool {
     return false;
   };
 
   if (parseMany(parseOp))
     return addErrorSuffix(" in '" + Twine(IDVal) + "' directive");
   return false;
 }
 
-/// ParseDirectiveOctaValue
-///  ::= .octa [ hexconstant (, hexconstant)* ]
-
-bool AsmParser::parseDirectiveOctaValue(StringRef IDVal) {
-  auto parseOp = [&]() -> bool {
-    if (checkForValidSection())
-      return true;
-    if (getTok().isNot(AsmToken::Integer) && getTok().isNot(AsmToken::BigNum))
-      return TokError("unknown token in expression");
-    SMLoc ExprLoc = getTok().getLoc();
-    APInt IntValue = getTok().getAPIntVal();
-    uint64_t hi, lo;
-    Lex();
-    if (!IntValue.isIntN(128))
-      return Error(ExprLoc, "out of range literal value");
-    if (!IntValue.isIntN(64)) {
-      hi = IntValue.getHiBits(IntValue.getBitWidth() - 64).getZExtValue();
-      lo = IntValue.getLoBits(64).getZExtValue();
-    } else {
-      hi = 0;
-      lo = IntValue.getZExtValue();
-    }
+static bool parseHexOcta(AsmParser &Asm, uint64_t &hi, uint64_t &lo) {
+  if (Asm.getTok().isNot(AsmToken::Integer) &&
+      Asm.getTok().isNot(AsmToken::BigNum))
+    return Asm.TokError("unknown token in expression");
+  SMLoc ExprLoc = Asm.getTok().getLoc();
+  APInt IntValue = Asm.getTok().getAPIntVal();
+  Asm.Lex();
+    return Asm.Error(ExprLoc, "out of range literal value");
+  return false;
+}
+
+/// ParseDirectiveOctaValue
+///  ::= .octa [ hexconstant (, hexconstant)* ]
+
+bool AsmParser::parseDirectiveOctaValue(StringRef IDVal) {
+  auto parseOp = [&]() -> bool {
+    if (checkForValidSection())
+      return true;
+    uint64_t hi, lo;
+    if (parseHexOcta(*this, hi, lo))
+      return true;
     if (MAI.isLittleEndian()) {
       getStreamer().EmitIntValue(lo, 8);
       getStreamer().EmitIntValue(hi, 8);
     } else {
       getStreamer().EmitIntValue(hi, 8);
       getStreamer().EmitIntValue(lo, 8);
     }
     return false;
if (check(FileNumber == -1,
         parseEscapedString(FilenameData))
       return true;
     Filename = FilenameData;
     Directory = Path;
   } else {
     Filename = Path;
   }
 
-  std::string Checksum;
+  uint64_t MD5Hi, MD5Lo;
+  bool HasMD5 = false;
 
   Optional<StringRef> Source;
   bool HasSource = false;
   std::string SourceString;
 
   while (!parseOptionalToken(AsmToken::EndOfStatement)) {
     StringRef Keyword;
     if (check(getTok().isNot(AsmToken::Identifier),
               "unexpected token in '.file' directive") ||
         parseIdentifier(Keyword))
       return true;
     if (Keyword == "md5") {
+      HasMD5 = true;
       if (check(FileNumber == -1,
                 "MD5 checksum specified, but no file number") ||
-          check(getTok().isNot(AsmToken::String),
-                "unexpected token in '.file' directive") ||
-          parseEscapedString(Checksum) ||
-          check(Checksum.size() != 32, "invalid MD5 checksum specified"))
+          parseHexOcta(*this, MD5Hi, MD5Lo))
         return true;
     } else if (Keyword == "source") {
       HasSource = true;
       if (check(FileNumber == -1,
                 "source specified, but no file number") ||
           check(getTok().isNot(AsmToken::String),
                 "unexpected token in '.file' directive") ||
           parseEscapedString(SourceString))
         return true;
     } else {
       return TokError("unexpected token in '.file' directive");
     }
   }
 
   // In case there is a -g option as well as debug info from directive .file,
   // we turn off the -g option, directly use the existing debug info instead.
   getContext().setGenDwarfForAssembly(false);
 
   if (FileNumber == -1)
     getStreamer().EmitFileDirective(Filename);
   else {
     MD5::MD5Result *CKMem = nullptr;
-    if (!Checksum.empty()) {
-      Checksum = fromHex(Checksum);
-      if (check(Checksum.size() != 16, "invalid MD5 checksum specified"))
-        return true;
+    if (HasMD5) {
       CKMem = (MD5::MD5Result *)Ctx.allocate(sizeof(MD5::MD5Result), 1);
-      memcpy(&CKMem->Bytes, Checksum.data(), 16);
+      for (unsigned i = 0; i != 8; ++i) {
+        CKMem->Bytes[i] = uint8_t(MD5Hi >> ((7 - i) * 8));
+        CKMem->Bytes[i + 8] = uint8_t(MD5Lo >> ((7 - i) * 8));
+      }
     }
     if (HasSource) {
       char *SourceBuf = static_cast<char *>(Ctx.allocate(SourceString.size()));
       memcpy(SourceBuf, SourceString.data(), SourceString.size());
       Source = StringRef(SourceBuf, SourceString.size());
     }
     if (FileNumber == 0)
       getStreamer().emitDwarfFile0Directive(Directory, Filename, CKMem, Source);

llvm/trunk/test/CodeGen/Generic/dwarf-md5.ll

 ; MD5 checksums provided by IR should be passed through to asm.
 ; They'll be emitted to an object file only for DWARF 5 or later.
 
 ; Darwin clamps the line table at DWARF v2 so XFAIL this test.
 ; XFAIL: darwin
 
 ; REQUIRES: object-emission
 ; RUN: %llc_dwarf -dwarf-version 4 -filetype=asm -o - %s | FileCheck %s --check-prefixes=ASM,ASM-4
 ; RUN: %llc_dwarf -dwarf-version 5 -filetype=asm -o - %s | FileCheck %s --check-prefixes=ASM,ASM-5
 ; RUN: %llc_dwarf -dwarf-version 4 -filetype=obj -o %t4.o %s
 ; RUN: llvm-dwarfdump -debug-line %t4.o | FileCheck %s --check-prefix=OBJ
 ; RUN: %llc_dwarf -dwarf-version 5 -filetype=obj -o %t5.o %s
 ; RUN: llvm-dwarfdump -debug-line %t5.o | FileCheck %s --check-prefixes=OBJ,OBJ-5
 
 ; ASM-4-NOT: .file 0
-; ASM-5: .file 0 "/scratch{{.*[/\\]}}t.c" md5 "00000000000000000000000000000000"
+; ASM-5: .file 0 "/scratch{{.*[/\\]}}t.c" md5 0x00000000000000000000000000000000
 ; ASM: .file 1 "/scratch{{.*[/\\]}}t1.h"
 ; ASM-4-NOT:  md5
-; ASM-5-SAME: md5 "11111111111111111111111111111111"
+; ASM-5-SAME: md5 0x11111111111111111111111111111111
 ; ASM: .file 2 "/scratch{{.*[/\\]}}t2.h"
 ; ASM-4-NOT:  md5
-; ASM-5-SAME: md5 "22222222222222222222222222222222"
+; ASM-5-SAME: md5 0x22222222222222222222222222222222
 
 ; OBJ-5: file_names[ 0]:
 ; OBJ-5-NEXT: name: "t.c"
 ; OBJ-5-NEXT: dir_index: 0
 ; OBJ-5-NEXT: md5_checksum: 00000000000000000000000000000000
 ; OBJ: file_names[ 1]:
 ; OBJ-NEXT: name: "t1.h"
 ; OBJ-NEXT: dir_index: 0

llvm/trunk/test/MC/ELF/debug-file-options.s

 // RUN: llvm-mc -triple x86_64-unknown-unknown -dwarf-version 5 -filetype=obj %s -o -| llvm-dwarfdump --debug-line --debug-line-str -v - | FileCheck %s
 
 // Test combinations of options to the .file directive.
 
-        .file 1 "dir1/foo" md5 "ee87e05688663173cd6043a3a15bba6e" source "void foo() {}"
-        .file 2 "dir2/bar" source "void bar() {}" md5 "816225a0c90ca8948b70eb58be4d522f"
+        .file 1 "dir1/foo" md5 0xee87e05688663173cd6043a3a15bba6e source "void foo() {}"
+        .file 2 "dir2/bar" source "void bar() {}" md5 0x816225a0c90ca8948b70eb58be4d522f
         .loc 1 1 0
         nop
         .loc 2 1 0
         nop
 
 # CHECK: debug_line[0x00000000]
 # CHECK: version: 5
 # CHECK: include_directories[ 0] = .debug_line_str[0x[[DIR0:[0-9a-f]+]]] = "{{.+}}"

llvm/trunk/test/MC/ELF/debug-md5-err.s

 # RUN: not llvm-mc -triple x86_64-unknown-unknown -dwarf-version 5 -filetype=asm %s -o /dev/null 2>&1 | FileCheck %s
 # RUN: not llvm-mc -triple x86_64-unknown-unknown -dwarf-version 5 -filetype=obj %s -o /dev/null 2>&1 | FileCheck %s
 
 # This is syntactically legal, looks like no checksum provided.
 # CHECK-NOT: [[@LINE+1]]:{{[0-9]+}}: error:
         .file 1 "dir1/foo" "00112233445566778899aabbccddeeff"
 
 # Missing md5 keyword.
 # CHECK: [[@LINE+1]]:{{[0-9]+}}: error: unexpected token in '.file' directive
-        .file 2 "dir1" "foo" "00112233445566778899aabbccddeeff"
+        .file 2 "dir1" "foo" 0x00112233445566778899aabbccddeeff
 
-# Bad length.
-# CHECK: [[@LINE+1]]:{{[0-9]+}}: error: invalid MD5 checksum specified
+# Bad syntax.
+# CHECK: [[@LINE+1]]:{{[0-9]+}}: error: unknown token in expression
         .file 3 "dir2" "bar" md5 "ff"
 
-# Not a string.
-# CHECK: [[@LINE+1]]:{{[0-9]+}}: error: unexpected token in '.file' directive
+# No hex prefix.
+# CHECK: [[@LINE+1]]:{{[0-9]+}}: error: unknown token in expression
         .file 4 "dir3" "foo" md5 ffeeddccbbaa99887766554433221100
 
 # Non-DWARF .file syntax with checksum.
 # CHECK: [[@LINE+1]]:{{[0-9]+}}: error: MD5 checksum specified, but no file number
-        .file "baz" md5 "ffeeddccbbaa998877665544332211gg"
+        .file "baz" md5 0xffeeddccbbaa99887766554433221100
 
 # Inconsistent use of MD5 option. Note: .file 1 did not supply one.
 # CHECK: [[@LINE+1]]:{{[0-9]+}}: error: inconsistent use of MD5 checksums
-        .file 5 "bax" md5 "ffeeddccbbaa99887766554433221100"
+        .file 5 "bax" md5 0xffeeddccbbaa99887766554433221100

llvm/trunk/test/MC/ELF/debug-md5.s

 // RUN: llvm-mc -triple x86_64-unknown-unknown -dwarf-version 5 -fdebug-compilation-dir=/tmp -filetype=obj %s -o - | llvm-dwarfdump --debug-line --debug-line-str -v - | FileCheck %s
 
-        .file 1 "dir1/foo"   md5 "00112233445566778899aabbccddeeff"
-        .file 2 "dir2" "bar" md5 "ffeeddccbbaa99887766554433221100"
+        .file 1 "dir1/foo"   md5 0x00112233445566778899aabbccddeeff
+        .file 2 "dir2" "bar" md5 0xffeeddccbbaa99887766554433221100
         .loc 1 1 0
         nop
         .loc 2 1 0
         nop
 
 # CHECK: debug_line[0x00000000]
 # CHECK: version: 5
 # CHECK: include_directories[ 0] = .debug_line_str[0x[[DIR0:[0-9a-f]+]]] = "/tmp"