[PATCH] D51867: [Diagnostics][NFC] Add error handling to FormatDiagnostic()

[Jan Korous via Phabricator via cfe-commits]

jkorous created this revision.
jkorous added reviewers: arphaman, vsapsai.
jkorous added a project: clang.
Herald added subscribers: cfe-commits, dexonsmith.

There seem to be couple implicit assumptions that might be better represented explicitly by asserts.


Repository:
  rC Clang

https://reviews.llvm.org/D51867

Files:
  lib/Basic/Diagnostic.cpp


Index: lib/Basic/Diagnostic.cpp
===================================================================
--- lib/Basic/Diagnostic.cpp
+++ lib/Basic/Diagnostic.cpp
@@ -768,6 +768,7 @@
 void Diagnostic::
 FormatDiagnostic(const char *DiagStr, const char *DiagEnd,
                  SmallVectorImpl<char> &OutStr) const {
+  assert(DiagStr <= DiagEnd && "Invalid DiagStr-DiagEnd range");
   // When the diagnostic string is only "%0", the entire string is being given
   // by an outside source.  Remove unprintable characters from this string
   // and skip all the other string processing.
@@ -798,17 +799,22 @@
     if (getArgKind(i) == DiagnosticsEngine::ak_qualtype)
       QualTypeVals.push_back(getRawArg(i));
 
-  while (DiagStr != DiagEnd) {
+  assert(DiagStr != nullptr && "DiagStr is nullptr");
+
+  while (DiagStr < DiagEnd) {
     if (DiagStr[0] != '%') {
       // Append non-%0 substrings to Str if we have one.
       const char *StrEnd = std::find(DiagStr, DiagEnd, '%');
       OutStr.append(DiagStr, StrEnd);
       DiagStr = StrEnd;
       continue;
-    } else if (isPunctuation(DiagStr[1])) {
+    } else if ((DiagStr + 1) < DiagEnd && isPunctuation(DiagStr[1])) {
       OutStr.push_back(DiagStr[1]);  // %% -> %.
       DiagStr += 2;
       continue;
+    } else if ((DiagStr + 1) >= DiagEnd) {
+      llvm_unreachable("DiagStr ends with '%'");
+      return;
     }
 
     // Skip the %.
@@ -825,34 +831,40 @@
     // Check to see if we have a modifier.  If so eat it.
     if (!isDigit(DiagStr[0])) {
       Modifier = DiagStr;
-      while (DiagStr[0] == '-' ||
-             (DiagStr[0] >= 'a' && DiagStr[0] <= 'z'))
+      while (DiagStr < DiagEnd &&
+             (DiagStr[0] == '-' || (DiagStr[0] >= 'a' && DiagStr[0] <= 'z')))
         ++DiagStr;
       ModifierLen = DiagStr-Modifier;
 
       // If we have an argument, get it next.
       if (DiagStr[0] == '{') {
         ++DiagStr; // Skip {.
+        assert(DiagStr < DiagEnd && "Invalid DiagStr");
         Argument = DiagStr;
 
         DiagStr = ScanFormat(DiagStr, DiagEnd, '}');
         assert(DiagStr != DiagEnd && "Mismatched {}'s in diagnostic string!");
         ArgumentLen = DiagStr-Argument;
         ++DiagStr;  // Skip }.
+        assert(DiagStr < DiagEnd && "Invalid DiagStr");
       }
     }
 
     assert(isDigit(*DiagStr) && "Invalid format for argument in diagnostic");
-    unsigned ArgNo = *DiagStr++ - '0';
+
+    unsigned ArgNo = *DiagStr - '0';
 
     // Only used for type diffing.
     unsigned ArgNo2 = ArgNo;
+    ++DiagStr;
 
     DiagnosticsEngine::ArgumentKind Kind = getArgKind(ArgNo);
     if (ModifierIs(Modifier, ModifierLen, "diff")) {
+      assert(DiagStr + 1 < DiagEnd && "Invalid diff modifier in DiagStr");
       assert(*DiagStr == ',' && isDigit(*(DiagStr + 1)) &&
              "Invalid format for diff modifier");
       ++DiagStr;  // Comma.
+      assert(DiagStr < DiagEnd && "Invalid DiagStr");
       ArgNo2 = *DiagStr++ - '0';
       DiagnosticsEngine::ArgumentKind Kind2 = getArgKind(ArgNo2);
       if (Kind == DiagnosticsEngine::ak_qualtype &&


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D51867.164692.patch
Type: text/x-patch
Size: 3079 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180910/2198e587/attachment.bin>