r200132 - Enforce safe usage of DiagnosticsEngine::getCustomDiagID()

Alp Toker alp at nuanti.com
Sat Jan 25 22:17:38 PST 2014


Author: alp
Date: Sun Jan 26 00:17:37 2014
New Revision: 200132

URL: http://llvm.org/viewvc/llvm-project?rev=200132&view=rev
Log:
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.

Modified:
    cfe/trunk/include/clang/Basic/Diagnostic.h
    cfe/trunk/include/clang/Basic/DiagnosticIDs.h
    cfe/trunk/lib/ARCMigrate/FileRemapper.cpp
    cfe/trunk/lib/ARCMigrate/ObjCMT.cpp
    cfe/trunk/lib/CodeGen/CodeGenModule.cpp
    cfe/trunk/lib/CodeGen/CodeGenPGO.cpp
    cfe/trunk/lib/Lex/PTHLexer.cpp

Modified: cfe/trunk/include/clang/Basic/Diagnostic.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Diagnostic.h?rev=200132&r1=200131&r2=200132&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Diagnostic.h (original)
+++ cfe/trunk/include/clang/Basic/Diagnostic.h Sun Jan 26 00:17:37 2014
@@ -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

Modified: cfe/trunk/include/clang/Basic/DiagnosticIDs.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticIDs.h?rev=200132&r1=200131&r2=200132&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticIDs.h (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticIDs.h Sun Jan 26 00:17:37 2014
@@ -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.

Modified: cfe/trunk/lib/ARCMigrate/FileRemapper.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/FileRemapper.cpp?rev=200132&r1=200131&r2=200132&view=diff
==============================================================================
--- cfe/trunk/lib/ARCMigrate/FileRemapper.cpp (original)
+++ cfe/trunk/lib/ARCMigrate/FileRemapper.cpp Sun Jan 26 00:17:37 2014
@@ -271,9 +271,7 @@ void FileRemapper::resetTarget(Target &t
 }
 
 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;
 }

Modified: cfe/trunk/lib/ARCMigrate/ObjCMT.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/ObjCMT.cpp?rev=200132&r1=200131&r2=200132&view=diff
==============================================================================
--- cfe/trunk/lib/ARCMigrate/ObjCMT.cpp (original)
+++ cfe/trunk/lib/ARCMigrate/ObjCMT.cpp Sun Jan 26 00:17:37 2014
@@ -1845,11 +1845,9 @@ void ObjCMigrateASTConsumer::HandleTrans
    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;
 }
 

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=200132&r1=200131&r2=200132&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Sun Jan 26 00:17:37 2014
@@ -339,9 +339,9 @@ void CodeGenModule::DecorateInstruction(
     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

Modified: cfe/trunk/lib/CodeGen/CodeGenPGO.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenPGO.cpp?rev=200132&r1=200131&r2=200132&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenPGO.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenPGO.cpp Sun Jan 26 00:17:37 2014
@@ -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)

Modified: cfe/trunk/lib/Lex/PTHLexer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PTHLexer.cpp?rev=200132&r1=200131&r2=200132&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/PTHLexer.cpp (original)
+++ cfe/trunk/lib/Lex/PTHLexer.cpp Sun Jan 26 00:17:37 2014
@@ -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,





More information about the cfe-commits mailing list