Enforce safe usage of DiagnosticsEngine::getCustomDiagID()

Replace the last incorrect uses and templatize the function to require a
compile-time constant string preventing further misuse.

The diagnostic formatter expects well-formed input and has undefined behaviour
with arbitrary input or crafted user strings in source files. Accepting user
input would also have caused unbounded generation of new diagnostic IDs which
can be problematic in long-running sessions or language bindings.

This completes the work to fix several incorrect callers that passed user
input or raw messages to the diagnostics engine where a constant format string
was expected.

llvm-svn: 200132

clang/include/clang/Basic/Diagnostic.h

@@ -587,15 +587,18 @@ public:
     this->NumWarnings = NumWarnings;
   }
 
-  /// \brief Return an ID for a diagnostic with the specified message and level.
+  /// \brief Return an ID for a diagnostic with the specified format string and
+  /// level.
   ///
   /// If this is the first request for this diagnostic, it is registered and
   /// created, otherwise the existing ID is returned.
   ///
   /// \param Message A fixed diagnostic format string that will be hashed and
   /// mapped to a unique DiagID.
-  unsigned getCustomDiagID(Level L, StringRef Message) {
-    return Diags->getCustomDiagID((DiagnosticIDs::Level)L, Message);
+  template <unsigned N>
+  unsigned getCustomDiagID(Level L, const char (&FormatString)[N]) {
+    return Diags->getCustomDiagID((DiagnosticIDs::Level)L,
+                                  StringRef(FormatString, N));
   }
 
   /// \brief Converts a diagnostic argument (as an intptr_t) into the string

clang/include/clang/Basic/DiagnosticIDs.h

@@ -124,11 +124,16 @@ public:
   DiagnosticIDs();
   ~DiagnosticIDs();
 
-  /// \brief Return an ID for a diagnostic with the specified message and level.
+  /// \brief Return an ID for a diagnostic with the specified format string and
+  /// level.
   ///
   /// If this is the first request for this diagnostic, it is registered and
   /// created, otherwise the existing ID is returned.
-  unsigned getCustomDiagID(Level L, StringRef Message);
+
+  // FIXME: Replace this function with a create-only facilty like
+  // createCustomDiagIDFromFormatString() to enforce safe usage. At the time of
+  // writing, nearly all callers of this function were invalid.
+  unsigned getCustomDiagID(Level L, StringRef FormatString);
 
   //===--------------------------------------------------------------------===//
   // Diagnostic classification and reporting interfaces.

clang/lib/ARCMigrate/FileRemapper.cpp

@@ -271,9 +271,7 @@ void FileRemapper::resetTarget(Target &targ) {
 }
 
 bool FileRemapper::report(const Twine &err, DiagnosticsEngine &Diag) {
-  SmallString<128> buf;
-  unsigned ID = Diag.getDiagnosticIDs()->getCustomDiagID(DiagnosticIDs::Error,
-                                                         err.toStringRef(buf));
-  Diag.Report(ID);
+  Diag.Report(Diag.getCustomDiagID(DiagnosticsEngine::Error, "%0"))
+      << err.str();
   return true;
 }

clang/lib/ARCMigrate/ObjCMT.cpp

@@ -1845,11 +1845,9 @@ void ObjCMigrateASTConsumer::HandleTranslationUnit(ASTContext &Ctx) {
    std::string Error;
    llvm::raw_fd_ostream OS(MigrateDir.c_str(), Error, llvm::sys::fs::F_Binary);
     if (!Error.empty()) {
-      // FIXME: It's not safe to pass arbitrary user-generated strings into
-      // getCustomDiagID(). Use a constant diagnostic ID instead.
-      unsigned ID = Ctx.getDiagnostics().getDiagnosticIDs()->
-          getCustomDiagID(DiagnosticIDs::Error, Error);
-      Ctx.getDiagnostics().Report(ID);
+      DiagnosticsEngine &Diags = Ctx.getDiagnostics();
+      Diags.Report(Diags.getCustomDiagID(DiagnosticsEngine::Error, "%0"))
+          << Error;
       return;
     }
 
@@ -2062,12 +2060,8 @@ private:
 }
 
 static bool reportDiag(const Twine &Err, DiagnosticsEngine &Diag) {
-  SmallString<128> Buf;
-  // FIXME: It's not safe to pass arbitrary user-generated strings into
-  // getCustomDiagID(). Use a constant diagnostic ID instead.
-  unsigned ID = Diag.getDiagnosticIDs()->getCustomDiagID(DiagnosticIDs::Error,
-                                                         Err.toStringRef(Buf));
-  Diag.Report(ID);
+  Diag.Report(Diag.getCustomDiagID(DiagnosticsEngine::Error, "%0"))
+      << Err.str();
   return true;
 }
 

clang/lib/CodeGen/CodeGenModule.cpp

@@ -339,9 +339,9 @@ void CodeGenModule::DecorateInstruction(llvm::Instruction *Inst,
     Inst->setMetadata(llvm::LLVMContext::MD_tbaa, TBAAInfo);
 }
 
-void CodeGenModule::Error(SourceLocation loc, StringRef error) {
-  unsigned diagID = getDiags().getCustomDiagID(DiagnosticsEngine::Error, error);
-  getDiags().Report(Context.getFullLoc(loc), diagID);
+void CodeGenModule::Error(SourceLocation loc, StringRef message) {
+  unsigned diagID = getDiags().getCustomDiagID(DiagnosticsEngine::Error, "%0");
+  getDiags().Report(Context.getFullLoc(loc), diagID) << message;
 }
 
 /// ErrorUnsupported - Print out an error that codegen doesn't support the

clang/lib/CodeGen/CodeGenPGO.cpp

@@ -24,8 +24,8 @@ using namespace CodeGen;
 
 static void ReportBadPGOData(CodeGenModule &CGM, const char *Message) {
   DiagnosticsEngine &Diags = CGM.getDiags();
-  unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error, Message);
-  Diags.Report(DiagID);
+  unsigned diagID = Diags.getCustomDiagID(DiagnosticsEngine::Error, "%0");
+  Diags.Report(diagID) << Message;
 }
 
 PGOProfileData::PGOProfileData(CodeGenModule &CGM, std::string Path)

clang/lib/Lex/PTHLexer.cpp

@@ -419,7 +419,7 @@ PTHManager::~PTHManager() {
 }
 
 static void InvalidPTH(DiagnosticsEngine &Diags, const char *Msg) {
-  Diags.Report(Diags.getCustomDiagID(DiagnosticsEngine::Error, Msg));
+  Diags.Report(Diags.getCustomDiagID(DiagnosticsEngine::Error, "%0")) << Msg;
 }
 
 PTHManager *PTHManager::Create(const std::string &file,