[PATCH] D43152: [DWARFv5] Turn an assert into a diagnostic.

[Paul Robinson via Phabricator via llvm-commits]

probinson created this revision.
probinson added reviewers: dblaikie, aprantl.
probinson added a project: debug-info.
Herald added subscribers: llvm-commits, JDevlieghere, hiraditya, arichardson, emaste.

Hand-coded assembler files should not trigger assertions.

I'm not really keen on my choice of error indicators, which is why I'm
posting this for review; suggestions welcome.  The thing is, there's a
moderate amount of plumbing in between the AsmParser and the DWARF
file table management that would be affected, that is also used by
DwarfDebug, and changing it would have largely no benefit.
I contemplated ErrorOr<> for a while, but it's not that easy to use
for a non-syscall diagnostic.


Repository:
  rL LLVM

https://reviews.llvm.org/D43152

Files:
  llvm/lib/MC/MCDwarf.cpp
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/test/MC/ELF/debug-md5-err.s


Index: llvm/test/MC/ELF/debug-md5-err.s
===================================================================
--- llvm/test/MC/ELF/debug-md5-err.s
+++ llvm/test/MC/ELF/debug-md5-err.s
@@ -19,3 +19,7 @@
 # Non-DWARF .file syntax with checksum.
 # CHECK: [[@LINE+1]]:{{[0-9]+}}: error: MD5 checksum specified, but no file number
         .file "baz" md5 "ffeeddccbbaa998877665544332211gg"
+
+# 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"
Index: llvm/lib/MC/MCParser/AsmParser.cpp
===================================================================
--- llvm/lib/MC/MCParser/AsmParser.cpp
+++ llvm/lib/MC/MCParser/AsmParser.cpp
@@ -3367,9 +3367,15 @@
     // we turn off -g option, directly use the existing debug info instead.
     if (getContext().getGenDwarfForAssembly())
       getContext().setGenDwarfForAssembly(false);
-    else if (getStreamer().EmitDwarfFileDirective(FileNumber, Directory,
-                                                  Filename, CKMem) == 0)
-      return Error(FileNumberLoc, "file number already allocated");
+    else {
+      FileNumber = getStreamer().EmitDwarfFileDirective(FileNumber, Directory,
+                                                        Filename, CKMem);
+      if (check(FileNumber == 0, FileNumberLoc,
+                "file number already allocated") ||
+          check(FileNumber == UINT_MAX, DirectiveLoc,
+                "inconsistent use of MD5 checksums"))
+        return true;
+    }
   }
 
   return false;
Index: llvm/lib/MC/MCDwarf.cpp
===================================================================
--- llvm/lib/MC/MCDwarf.cpp
+++ llvm/lib/MC/MCDwarf.cpp
@@ -514,6 +514,9 @@
     Directory = "";
   }
   assert(!FileName.empty());
+  // If any files have an MD5 checksum, they all must.
+  if (MCDwarfFiles.empty())
+    HasMD5 = (Checksum != nullptr);
   if (FileNumber == 0) {
     // File numbers start with 1 and/or after any file numbers
     // allocated by inline-assembler .file directives.
@@ -537,8 +540,8 @@
     return 0;
 
   // If any files have an MD5 checksum, they all must.
-  if (FileNumber > 1)
-    assert(HasMD5 == (Checksum != nullptr));
+  if (HasMD5 != (Checksum != nullptr))
+    return UINT_MAX;
 
   if (Directory.empty()) {
     // Separate the directory part from the basename of the FileName.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D43152.133730.patch
Type: text/x-patch
Size: 2453 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180210/e203c204/attachment.bin>