[poppler] 2 commits - goo/GooString.h poppler/Error.h poppler/Hints.cc poppler/JBIG2Stream.cc poppler/PSOutputDev.cc poppler/SecurityHandler.cc poppler/StructTreeRoot.cc qt4/src qt5/src test/goostring-format-checker utils/HtmlOutputDev.cc

Albert Astals Cid aacid at kemper.freedesktop.org
Mon Feb 17 15:02:06 PST 2014


 goo/GooString.h                                           |   12 
 poppler/Error.h                                           |    4 
 poppler/Hints.cc                                          |    3 
 poppler/JBIG2Stream.cc                                    |    4 
 poppler/PSOutputDev.cc                                    |    4 
 poppler/SecurityHandler.cc                                |    3 
 poppler/StructTreeRoot.cc                                 |    3 
 qt4/src/poppler-annotation.cc                             |    6 
 qt5/src/poppler-annotation.cc                             |    6 
 test/goostring-format-checker/README                      |   16 
 test/goostring-format-checker/goostring-format-checker.cc |  369 ++++++++++++++
 utils/HtmlOutputDev.cc                                    |   11 
 12 files changed, 419 insertions(+), 22 deletions(-)

New commits:
commit 63e9c0b67fa2e64ca20258d873a849386c7eb295
Author: Fabio D'Urso <fabiodurso at hotmail.it>
Date:   Mon Feb 17 23:58:09 2014 +0100

    Some error() usage fixes

diff --git a/poppler/Hints.cc b/poppler/Hints.cc
index 3c2d070..20bc952 100644
--- a/poppler/Hints.cc
+++ b/poppler/Hints.cc
@@ -8,6 +8,7 @@
 // Copyright 2010, 2011, 2013 Albert Astals Cid <aacid at kde.org>
 // Copyright 2010, 2013 Pino Toscano <pino at kde.org>
 // Copyright 2013 Adrian Johnson <ajohnson at redneon.com>
+// Copyright 2014 Fabio D'Urso <fabiodurso at hotmail.it>
 //
 //========================================================================
 
@@ -38,7 +39,7 @@ Hints::Hints(BaseStream *str, Linearization *linearization, XRef *xref, Security
   pageObjectFirst = linearization->getObjectNumberFirst();
   if (pageObjectFirst < 0 || pageObjectFirst >= xref->getNumObjects()) {
     error(errSyntaxWarning, -1,
-      "Invalid reference for first page object (%d) in linearization table ",
+      "Invalid reference for first page object ({0:d}) in linearization table ",
       pageObjectFirst);
     pageObjectFirst = 0;
   }
diff --git a/poppler/JBIG2Stream.cc b/poppler/JBIG2Stream.cc
index bda7d52..3324143 100644
--- a/poppler/JBIG2Stream.cc
+++ b/poppler/JBIG2Stream.cc
@@ -21,7 +21,7 @@
 // Copyright (C) 2012 William Bader <williambader at hotmail.com>
 // Copyright (C) 2012 Thomas Freitag <Thomas.Freitag at alfa.de>
 // Copyright (C) 2013 Adrian Johnson <ajohnson at redneon.com>
-// Copyright (C) 2013 Fabio D'Urso <fabiodurso at hotmail.it>
+// Copyright (C) 2013, 2014 Fabio D'Urso <fabiodurso at hotmail.it>
 //
 // To see a description of the changes please see the Changelog file that
 // came with your tarball or type make ChangeLog if you are building from git
@@ -1840,7 +1840,7 @@ GBool JBIG2Stream::readSymbolDictSeg(Guint segNum, Guint length,
 	  }
 	  refBitmap = bitmaps[symID];
 	  if (unlikely(refBitmap == NULL)) {
-	    error(errSyntaxError, curStr->getPos(), "Invalid ref bitmap for symbol ID {0:d} in JBIG2 symbol dictionary", symID);
+	    error(errSyntaxError, curStr->getPos(), "Invalid ref bitmap for symbol ID {0:ud} in JBIG2 symbol dictionary", symID);
 	    goto syntaxError;
 	  }
 	  bitmaps[numInputSyms + i] =
diff --git a/poppler/PSOutputDev.cc b/poppler/PSOutputDev.cc
index a85fc24..cfe47af 100644
--- a/poppler/PSOutputDev.cc
+++ b/poppler/PSOutputDev.cc
@@ -26,7 +26,7 @@
 // Copyright (C) 2009, 2011, 2012 William Bader <williambader at hotmail.com>
 // Copyright (C) 2009 Kovid Goyal <kovid at kovidgoyal.net>
 // Copyright (C) 2009-2011, 2013 Adrian Johnson <ajohnson at redneon.com>
-// Copyright (C) 2012 Fabio D'Urso <fabiodurso at hotmail.it>
+// Copyright (C) 2012, 2014 Fabio D'Urso <fabiodurso at hotmail.it>
 // Copyright (C) 2012 Lu Wang <coolwanglu at gmail.com>
 //
 // To see a description of the changes please see the Changelog file that
@@ -2508,7 +2508,7 @@ void PSOutputDev::setupExternalCIDTrueTypeFont(GfxFont *font,
       gfree(codeToGID);
     } else {
       error(errSyntaxError, -1,
-	    "TrueType font '%s' does not allow embedding",
+	    "TrueType font '{0:s}' does not allow embedding",
 	    font->getName() ? font->getName()->getCString() : "(unnamed)");
 	    
     }
diff --git a/poppler/SecurityHandler.cc b/poppler/SecurityHandler.cc
index 8b85e60..d6f5599 100644
--- a/poppler/SecurityHandler.cc
+++ b/poppler/SecurityHandler.cc
@@ -15,6 +15,7 @@
 //
 // Copyright (C) 2010, 2012 Albert Astals Cid <aacid at kde.org>
 // Copyright (C) 2013 Adrian Johnson <ajohnson at redneon.com>
+// Copyright (C) 2014 Fabio D'Urso <fabiodurso at hotmail.it>
 //
 // To see a description of the changes please see the Changelog file that
 // came with your tarball or type make ChangeLog if you are building from git
@@ -295,7 +296,7 @@ StandardSecurityHandler::StandardSecurityHandler(PDFDoc *docA,
 	ok = gTrue;
       } else if (!(encVersion == -1 && encRevision == -1)) {
 	error(errUnimplemented, -1,
-	      "Unsupported version/revision (%d/%d) of Standard security handler",
+	      "Unsupported version/revision ({0:d}/{1:d}) of Standard security handler",
 	      encVersion, encRevision);
       }
     } else {
diff --git a/poppler/StructTreeRoot.cc b/poppler/StructTreeRoot.cc
index 59f017e..4fff297 100644
--- a/poppler/StructTreeRoot.cc
+++ b/poppler/StructTreeRoot.cc
@@ -5,6 +5,7 @@
 // This file is licensed under the GPLv2 or later
 //
 // Copyright 2013 Igalia S.L.
+// Copyright 2014 Fabio D'Urso <fabiodurso at hotmail.it>
 //
 //========================================================================
 
@@ -94,7 +95,7 @@ void StructTreeRoot::parse(Dict *root)
           index.free();
         }
       } else {
-        error(errSyntaxError, -1, "Nums array length is not a even ({0:i})", nums.arrayGetLength());
+        error(errSyntaxError, -1, "Nums array length is not a even ({0:d})", nums.arrayGetLength());
       }
     } else {
       error(errSyntaxError, -1, "Nums object is wrong type ({0:s})", nums.getTypeName());
diff --git a/qt4/src/poppler-annotation.cc b/qt4/src/poppler-annotation.cc
index 6d19065..297cd86 100644
--- a/qt4/src/poppler-annotation.cc
+++ b/qt4/src/poppler-annotation.cc
@@ -2,7 +2,7 @@
  * Copyright (C) 2006, 2009, 2012, 2013 Albert Astals Cid <aacid at kde.org>
  * Copyright (C) 2006, 2008, 2010 Pino Toscano <pino at kde.org>
  * Copyright (C) 2012, Guillermo A. Amaral B. <gamaral at kde.org>
- * Copyright (C) 2012, 2013 Fabio D'Urso <fabiodurso at hotmail.it>
+ * Copyright (C) 2012-2014 Fabio D'Urso <fabiodurso at hotmail.it>
  * Copyright (C) 2012, Tobias Koenig <tokoe at kdab.com>
  * Adapting code from
  *   Copyright (C) 2004 by Enrico Ros <eros.kde at email.it>
@@ -383,7 +383,7 @@ QList<Annotation*> AnnotationPrivate::findAnnotations(::Page *pdfPage, DocumentD
         Annot * ann = annots->getAnnot( j );
         if ( !ann )
         {
-            error(errInternal, -1, "Annot %u is null", j);
+            error(errInternal, -1, "Annot {0:ud} is null", j);
             continue;
         }
 
@@ -536,7 +536,7 @@ QList<Annotation*> AnnotationPrivate::findAnnotations(::Page *pdfPage, DocumentD
                     CASE_FOR_TYPE( TrapNet )
                     CASE_FOR_TYPE( Watermark )
                     CASE_FOR_TYPE( 3D )
-                    default: error(errUnimplemented, -1, "Annotation %u not supported", subType);
+                    default: error(errUnimplemented, -1, "Annotation {0:d} not supported", subType);
                 }
                 continue;
 #undef CASE_FOR_TYPE
diff --git a/qt5/src/poppler-annotation.cc b/qt5/src/poppler-annotation.cc
index 9a8b655..62ccaa5 100644
--- a/qt5/src/poppler-annotation.cc
+++ b/qt5/src/poppler-annotation.cc
@@ -2,7 +2,7 @@
  * Copyright (C) 2006, 2009, 2012, 2013 Albert Astals Cid <aacid at kde.org>
  * Copyright (C) 2006, 2008, 2010 Pino Toscano <pino at kde.org>
  * Copyright (C) 2012, Guillermo A. Amaral B. <gamaral at kde.org>
- * Copyright (C) 2012, 2013 Fabio D'Urso <fabiodurso at hotmail.it>
+ * Copyright (C) 2012-2014 Fabio D'Urso <fabiodurso at hotmail.it>
  * Copyright (C) 2012, Tobias Koenig <tokoe at kdab.com>
  * Adapting code from
  *   Copyright (C) 2004 by Enrico Ros <eros.kde at email.it>
@@ -383,7 +383,7 @@ QList<Annotation*> AnnotationPrivate::findAnnotations(::Page *pdfPage, DocumentD
         Annot * ann = annots->getAnnot( j );
         if ( !ann )
         {
-            error(errInternal, -1, "Annot %u is null", j);
+            error(errInternal, -1, "Annot {0:ud} is null", j);
             continue;
         }
 
@@ -536,7 +536,7 @@ QList<Annotation*> AnnotationPrivate::findAnnotations(::Page *pdfPage, DocumentD
                     CASE_FOR_TYPE( TrapNet )
                     CASE_FOR_TYPE( Watermark )
                     CASE_FOR_TYPE( 3D )
-                    default: error(errUnimplemented, -1, "Annotation %u not supported", subType);
+                    default: error(errUnimplemented, -1, "Annotation {0:d} not supported", subType);
                 }
                 continue;
 #undef CASE_FOR_TYPE
diff --git a/utils/HtmlOutputDev.cc b/utils/HtmlOutputDev.cc
index a3ae239..35e98b4 100644
--- a/utils/HtmlOutputDev.cc
+++ b/utils/HtmlOutputDev.cc
@@ -37,6 +37,7 @@
 // Copyright (C) 2013 Thomas Freitag <Thomas.Freitag at alfa.de>
 // Copyright (C) 2013 Julien Nabet <serval2412 at yahoo.fr>
 // Copyright (C) 2013 Johannes Brandstätter <jbrandstaetter at gmail.com>
+// Copyright (C) 2014 Fabio D'Urso <fabiodurso at hotmail.it>
 //
 // To see a description of the changes please see the Changelog file that
 // came with your tarball or type make ChangeLog if you are building from git
@@ -1327,7 +1328,7 @@ void HtmlOutputDev::drawJpegImage(GfxState *state, Stream *str)
   // open the image file
   GooString *fName=createImageFileName("jpg");
   if (!(f1 = fopen(fName->getCString(), "wb"))) {
-    error(errIO, -1, "Couldn't open image file '%s'", fName->getCString());
+    error(errIO, -1, "Couldn't open image file '{0:t}'", fName);
     delete fName;
     return;
   }
@@ -1361,7 +1362,7 @@ void HtmlOutputDev::drawPngImage(GfxState *state, Stream *str, int width, int he
   // open the image file
   GooString *fName=createImageFileName("png");
   if (!(f1 = fopen(fName->getCString(), "wb"))) {
-    error(errIO, -1, "Couldn't open image file '%s'", fName->getCString());
+    error(errIO, -1, "Couldn't open image file '{0:t}'", fName);
     delete fName;
     return;
   }
@@ -1369,7 +1370,7 @@ void HtmlOutputDev::drawPngImage(GfxState *state, Stream *str, int width, int he
   PNGWriter *writer = new PNGWriter( isMask ? PNGWriter::MONOCHROME : PNGWriter::RGB );
   // TODO can we calculate the resolution of the image?
   if (!writer->init(f1, width, height, 72, 72)) {
-    error(errInternal, -1, "Can't init PNG for image '%s'", fName->getCString());
+    error(errInternal, -1, "Can't init PNG for image '{0:t}'", fName);
     delete writer;
     fclose(f1);
     return;
@@ -1401,7 +1402,7 @@ void HtmlOutputDev::drawPngImage(GfxState *state, Stream *str, int width, int he
       }
 
       if (!writer->writeRow(row_pointer)) {
-        error(errIO, -1, "Failed to write into PNG '%s'", fName->getCString());
+        error(errIO, -1, "Failed to write into PNG '{0:t}'", fName);
         delete writer;
         delete imgStr;
         fclose(f1);
@@ -1438,7 +1439,7 @@ void HtmlOutputDev::drawPngImage(GfxState *state, Stream *str, int width, int he
 
       if (!writer->writeRow( &png_row ))
       {
-        error(errIO, -1, "Failed to write into PNG '%s'", fName->getCString());
+        error(errIO, -1, "Failed to write into PNG '{0:t}'", fName);
         delete writer;
         fclose(f1);
         gfree(png_row);
commit 8f2d847f1d0224a297e642944f8da9c1409732b6
Author: Fabio D'Urso <fabiodurso at hotmail.it>
Date:   Mon Feb 17 23:56:49 2014 +0100

    Clang++ plugin that checks for usage errors in GooString::format-style calls

diff --git a/goo/GooString.h b/goo/GooString.h
index 6bdcf06..5932be9 100644
--- a/goo/GooString.h
+++ b/goo/GooString.h
@@ -18,7 +18,7 @@
 // Copyright (C) 2006 Kristian Høgsberg <krh at redhat.com>
 // Copyright (C) 2006 Krzysztof Kowalczyk <kkowalczyk at gmail.com>
 // Copyright (C) 2008-2010, 2012 Albert Astals Cid <aacid at kde.org>
-// Copyright (C) 2012, 2013 Fabio D'Urso <fabiodurso at hotmail.it>
+// Copyright (C) 2012-2014 Fabio D'Urso <fabiodurso at hotmail.it>
 // Copyright (C) 2013 Jason Crain <jason at aquaticape.us>
 //
 // To see a description of the changes please see the Changelog file that
@@ -38,6 +38,12 @@
 #include <stdlib.h> // for NULL
 #include "gtypes.h"
 
+#ifdef __clang__
+# define GOOSTRING_FORMAT __attribute__((__annotate__("gooformat")))
+#else
+# define GOOSTRING_FORMAT
+#endif
+
 class GooString {
 public:
 
@@ -97,7 +103,7 @@ public:
   //     t -- GooString *
   //     w -- blank space; arg determines width
   // To get literal curly braces, use {{ or }}.
-  static GooString *format(const char *fmt, ...);
+  static GooString *format(const char *fmt, ...) GOOSTRING_FORMAT;
   static GooString *formatv(const char *fmt, va_list argList);
 
   // Destructor.
@@ -124,7 +130,7 @@ public:
   GooString *append(const char *str, int lengthA=CALC_STRING_LEN);
 
   // Append a formatted string.
-  GooString *appendf(const char *fmt, ...);
+  GooString *appendf(const char *fmt, ...) GOOSTRING_FORMAT;
   GooString *appendfv(const char *fmt, va_list argList);
 
   // Insert a character or string.
diff --git a/poppler/Error.h b/poppler/Error.h
index 88fc3ea..9e11733 100644
--- a/poppler/Error.h
+++ b/poppler/Error.h
@@ -17,6 +17,7 @@
 // Copyright (C) 2005 Albert Astals Cid <aacid at kde.org>
 // Copyright (C) 2005 Kristian Høgsberg <krh at redhat.com>
 // Copyright (C) 2013 Adrian Johnson <ajohnson at redneon.com>
+// Copyright (C) 2014 Fabio D'Urso <fabiodurso at hotmail.it>
 //
 // To see a description of the changes please see the Changelog file that
 // came with your tarball or type make ChangeLog if you are building from git
@@ -33,6 +34,7 @@
 #include <stdarg.h>
 #include "poppler-config.h"
 #include "goo/gtypes.h"
+#include "goo/GooString.h"
 
 enum ErrorCategory {
   errSyntaxWarning,    // PDF syntax error which can be worked around;
@@ -53,6 +55,6 @@ extern void setErrorCallback(void (*cbk)(void *data, ErrorCategory category,
 					 Goffset pos, char *msg),
 			     void *data);
 
-extern void CDECL error(ErrorCategory category, Goffset pos, const char *msg, ...);
+extern void CDECL error(ErrorCategory category, Goffset pos, const char *msg, ...) GOOSTRING_FORMAT;
 
 #endif
diff --git a/test/goostring-format-checker/README b/test/goostring-format-checker/README
new file mode 100644
index 0000000..cc58a5d
--- /dev/null
+++ b/test/goostring-format-checker/README
@@ -0,0 +1,16 @@
+== Clang++ compiler plugin that checks usage of GooString::format-like functions ==
+
+1) Compile the plugin with:
+	clang++ -shared -o goostring-format-checker.so goostring-format-checker.cc -D__STDC_CONSTANT_MACROS -D__STDC_LIMIT_MACROS
+
+2) Compile poppler and pass the following options to the clang++ compiler:
+	-Xclang -load -Xclang goostring-format-checker.so -Xclang -plugin -Xclang goostring-format-check
+
+Example:
+$ clang++ -shared -o goostring-format-checker.so goostring-format-checker.cc -D__STDC_CONSTANT_MACROS -D__STDC_LIMIT_MACROS
+$ export CXX=clang++
+$ export CXXFLAGS="-Xclang -load -Xclang $PWD/goostring-format-checker.so -Xclang -add-plugin -Xclang goostring-format-checker"
+$ mkdir build
+$ cd build
+$ cmake ../../..
+$ make
diff --git a/test/goostring-format-checker/goostring-format-checker.cc b/test/goostring-format-checker/goostring-format-checker.cc
new file mode 100644
index 0000000..eab9221
--- /dev/null
+++ b/test/goostring-format-checker/goostring-format-checker.cc
@@ -0,0 +1,369 @@
+/*
+ * goostring-format-checker.cc
+ *
+ * This file is licensed under the GPLv2 or later
+ *
+ * Clang++ compiler plugin that checks usage of GooString::format-like functions
+ *
+ * Copyright (C) 2014 Fabio D'Urso <fabiodurso at hotmail.it>
+ */
+
+#include <cctype>
+
+#include <clang/Frontend/FrontendPluginRegistry.h>
+#include <clang/AST/AST.h>
+#include <clang/AST/ASTConsumer.h>
+#include <clang/AST/Attr.h>
+#include <clang/AST/RecursiveASTVisitor.h>
+#include <clang/Frontend/CompilerInstance.h>
+
+using namespace clang;
+
+namespace
+{
+
+class GooStringFormatCheckerVisitor : public RecursiveASTVisitor<GooStringFormatCheckerVisitor> {
+public:
+	explicit GooStringFormatCheckerVisitor(CompilerInstance *compInst);
+
+	bool VisitFunctionDecl(FunctionDecl *funcDecl);
+	bool VisitCallExpr(CallExpr *callExpr);
+
+private:
+	/* Returns the index of the format argument, or -1 if the function must
+	 * not be checked */
+	int findFormatArgumentIndex(const FunctionDecl *funcDecl) const;
+
+	/* Returns the SourceLocation of the n-th character */
+	SourceLocation getLocationOfCharacter(const StringLiteral *strLiteral, unsigned n);
+
+	/* Validates usage of a placeholder and returns the corresponding
+	 * argument index, or -1 in case of errors */
+	int verifyPlaceholder(const CallExpr *callExpr, const SourceLocation &placeholderLocation,
+		std::string &placeholderText, int baseArgIdx) const;
+
+	CompilerInstance *compInst;
+	DiagnosticsEngine *diag;
+	unsigned diag_badFuncZeroArgs;
+	unsigned diag_badFuncNonVariadic;
+	unsigned diag_badFuncLastArgInvalidType;
+	unsigned diag_notStringLiteral;
+	unsigned diag_notPlainASCII;
+	unsigned diag_wrongOrder;
+	unsigned diag_unescapedBracket;
+	unsigned diag_unterminatedPlaceholder;
+	unsigned diag_unconsumedArgs;
+	unsigned diag_missingColon;
+	unsigned diag_missingArgNumber;
+	unsigned diag_badArgNumber;
+	unsigned diag_argumentNotPresent;
+	unsigned diag_badPrecision;
+	unsigned diag_badType;
+	unsigned diag_wrongArgExprType;
+};
+
+GooStringFormatCheckerVisitor::GooStringFormatCheckerVisitor(CompilerInstance *compInst)
+: compInst(compInst) {
+	diag = &compInst->getDiagnostics();
+
+	diag_badFuncZeroArgs = diag->getCustomDiagID(DiagnosticsEngine::Error, "Cannot enforce format string checks on a function that takes no arguments");
+	diag_badFuncNonVariadic = diag->getCustomDiagID(DiagnosticsEngine::Error, "Cannot enforce format string checks on a non-variadic function");
+	diag_badFuncLastArgInvalidType = diag->getCustomDiagID(DiagnosticsEngine::Error, "Cannot enforce format string checks if the last non-variadic argument is not const char *");
+	diag_notStringLiteral = diag->getCustomDiagID(DiagnosticsEngine::Warning, "Format string is not a string literal. Skipping format checks");
+	diag_notPlainASCII = diag->getCustomDiagID(DiagnosticsEngine::Warning, "Format string contains non-ASCII or NUL characters. Skipping format checks");
+	diag_wrongOrder = diag->getCustomDiagID(DiagnosticsEngine::Error, "Argument %0 must be consumed before argument %1");
+	diag_unescapedBracket = diag->getCustomDiagID(DiagnosticsEngine::Error, "Unescaped '}' character");
+	diag_unterminatedPlaceholder = diag->getCustomDiagID(DiagnosticsEngine::Error, "Unterminated placeholder");
+	diag_unconsumedArgs = diag->getCustomDiagID(DiagnosticsEngine::Warning, "Unconsumed argument(s)");
+	diag_missingColon = diag->getCustomDiagID(DiagnosticsEngine::Error, "Invalid placeholder '{%0}': missing colon character");
+	diag_missingArgNumber = diag->getCustomDiagID(DiagnosticsEngine::Error, "Invalid placeholder '{%0}': missing <arg> number");
+	diag_badArgNumber = diag->getCustomDiagID(DiagnosticsEngine::Error, "Invalid placeholder '{%0}': bad <arg> number");
+	diag_argumentNotPresent = diag->getCustomDiagID(DiagnosticsEngine::Error, "Argument for placeholder '{%0}' is not present");
+	diag_badPrecision = diag->getCustomDiagID(DiagnosticsEngine::Error, "Invalid placeholder '{%0}': bad <precision> value");
+	diag_badType = diag->getCustomDiagID(DiagnosticsEngine::Error, "Invalid placeholder '{%0}': bad <type> specifier");
+	diag_wrongArgExprType = diag->getCustomDiagID(DiagnosticsEngine::Error, "Expected %0 for placeholder '{%1}', found %2");
+}
+
+bool GooStringFormatCheckerVisitor::VisitFunctionDecl(FunctionDecl *funcDecl) {
+	findFormatArgumentIndex(funcDecl); // Spot misuse of the "gooformat" annotation
+	return true;
+}
+
+bool GooStringFormatCheckerVisitor::VisitCallExpr(CallExpr *callExpr) {
+	/*** Locate format argument or skip calls that needn't be checked ***/
+
+	const int formatArgIdx = findFormatArgumentIndex(callExpr->getDirectCallee());
+	if (formatArgIdx == -1)
+		return true;
+
+	/*** Obtain format string value ***/
+
+	const Expr *formatArgExpr = callExpr->getArg(formatArgIdx);
+	while (formatArgExpr->getStmtClass() == Stmt::ImplicitCastExprClass) {
+		formatArgExpr = static_cast<const ImplicitCastExpr*>(formatArgExpr)->getSubExpr();
+	}
+	if (formatArgExpr->getStmtClass() != Stmt::StringLiteralClass) {
+		diag->Report(formatArgExpr->getExprLoc(), diag_notStringLiteral);
+		return true;
+	}
+	const StringLiteral *formatArgStrLiteral = static_cast<const StringLiteral*>(formatArgExpr);
+	if (formatArgStrLiteral->containsNonAsciiOrNull()) {
+		diag->Report(formatArgExpr->getExprLoc(), diag_notPlainASCII);
+		return true;
+	}
+
+	/*** Parse format string and verify arguments ***/
+
+	const std::string format = formatArgStrLiteral->getString().str();
+
+	/* Keeps track of whether we are currently parsing a character contained
+	 * within '{' ... '}'. If set, current_placeholder contains the contents
+	 * parsed so far (without brackets) */
+	bool in_placeholder = false;
+	std::string current_placeholder;
+
+	// Source location of the current placeholder's opening bracket
+	SourceLocation placeholderLoc;
+
+	/* Keeps track of the next expected argument number, to check that
+	 * arguments are first consumed in order (eg {0:d}{2:d}{1:d} is wrong).
+	 * Note that it's possible to "look back" at already consumed
+	 * arguments (eg {0:d}{1:d}{0:d} is OK) */
+	int nextExpectedArgNum = 0;
+
+	for (unsigned i = 0; i < format.length(); i++) {
+		if (in_placeholder) {
+			// Have we reached the end of the placeholder?
+			if (format[i] == '}') {
+				in_placeholder = false;
+
+				// Verifies the placeholder and returns the argument number
+				const int foundArgNum = verifyPlaceholder(callExpr, placeholderLoc, current_placeholder, formatArgIdx+1);
+
+				// If the placeholder wasn't valid, disable argument order checks
+				if (foundArgNum == -1) {
+					nextExpectedArgNum = -1;
+				}
+
+				// If argument order checks are enabled, let's check!
+				if (nextExpectedArgNum != -1) {
+					if (foundArgNum == nextExpectedArgNum) {
+						nextExpectedArgNum++;
+					} else if (foundArgNum > nextExpectedArgNum) {
+						diag->Report(placeholderLoc, diag_wrongOrder) << nextExpectedArgNum << foundArgNum;
+						nextExpectedArgNum = -1; // disable further checks
+					}
+				}
+			} else {
+				current_placeholder += format[i];
+			}
+		} else if (format[i] == '{') {
+			// If we find a '{' then a placeholder is starting...
+			in_placeholder = true;
+			current_placeholder = "";
+			placeholderLoc = getLocationOfCharacter(formatArgStrLiteral, i);
+
+			// ...unless it's followed by another '{' (escape sequence)
+			if (i+1 < format.length() && format[i+1] == '{') {
+				i++; // skip next '{' character
+				in_placeholder = false;
+			}
+		} else if (format[i] == '}') {
+			/* If we have found a '}' and we're not in a placeholder,
+			 * then it *MUST* be followed by another '}' (escape sequence) */
+			if (i+1 >= format.length() || format[i+1] != '}') {
+				diag->Report(getLocationOfCharacter(formatArgStrLiteral, i), diag_unescapedBracket);
+			} else {
+				i++; // skip next '}' character
+			}
+		}
+	}
+
+	/* If we've reached the end of the format string and in_placeholder is
+	 * still set, then the last placeholder wasn't terminated properly */
+	if (in_placeholder)
+		diag->Report(placeholderLoc, diag_unterminatedPlaceholder);
+
+	int unconsumedArgs = callExpr->getNumArgs() - (formatArgIdx + 1 + nextExpectedArgNum);
+	if (unconsumedArgs > 0)
+		diag->Report(callExpr->getArg(callExpr->getNumArgs() - unconsumedArgs)->getExprLoc(), diag_unconsumedArgs);
+
+	return true;
+}
+
+int GooStringFormatCheckerVisitor::findFormatArgumentIndex(const FunctionDecl *funcDecl) const {
+	if (!funcDecl)
+		return -1;
+
+	AnnotateAttr *annotation = NULL;
+	for (specific_attr_iterator<AnnotateAttr> it = funcDecl->specific_attr_begin<AnnotateAttr>();
+		it != funcDecl->specific_attr_end<AnnotateAttr>() && !annotation; ++it) {
+		if (it->getAnnotation() == "gooformat")
+			annotation = *it;
+	}
+
+	// If this function hasn't got the "gooformat" annotation on it
+	if (!annotation)
+		return -1;
+
+	if (funcDecl->getNumParams() == 0) {
+		diag->Report(annotation->getLocation(), diag_badFuncZeroArgs);
+		return -1;
+	}
+
+	if (!funcDecl->isVariadic()) {
+		diag->Report(annotation->getLocation(), diag_badFuncNonVariadic);
+		return -1;
+	}
+
+	// Assume the last non-variadic argument is the format specifier
+	const int formatArgIdx = funcDecl->getNumParams() - 1;
+	const QualType formatArgType = funcDecl->getParamDecl(formatArgIdx)->getType();
+	if (formatArgType.getAsString() != "const char *") {
+		diag->Report(annotation->getLocation(), diag_badFuncLastArgInvalidType);
+		return -1;
+	}
+
+	return formatArgIdx;
+}
+
+SourceLocation GooStringFormatCheckerVisitor::getLocationOfCharacter(const StringLiteral *strLiteral, unsigned n)
+{
+	return strLiteral->getLocationOfByte(n, compInst->getSourceManager(),
+		compInst->getLangOpts(), compInst->getTarget());
+}
+
+int GooStringFormatCheckerVisitor::verifyPlaceholder(const CallExpr *callExpr, const SourceLocation &placeholderLocation,
+		std::string &placeholderText, int baseArgIdx) const
+{
+	// Find the colon that separates the argument number and the format specifier
+	const size_t delim = placeholderText.find(':');
+	if (delim == std::string::npos) {
+		diag->Report(placeholderLocation, diag_missingColon) << placeholderText;
+		return -1;
+	}
+	if (delim == 0) {
+		diag->Report(placeholderLocation, diag_missingArgNumber) << placeholderText;
+		return -1;
+	}
+	for (unsigned int i = 0; i < delim; i++) {
+		if (!isdigit(placeholderText[i])) {
+			diag->Report(placeholderLocation, diag_badArgNumber) << placeholderText;
+			return -1;
+		}
+	}
+
+	// Extract argument number and its actual position in the call's argument list
+	const int argNum = atoi(placeholderText.substr(0, delim).c_str());
+	const int argIdx = baseArgIdx + argNum;
+	if (argIdx >= callExpr->getNumArgs()) {
+		diag->Report(placeholderLocation, diag_argumentNotPresent) << placeholderText;
+		return argNum;
+	}
+
+	// Check and strip width/precision specifiers
+	std::string format = placeholderText.substr(delim + 1);
+	bool dot_found = false;
+	while (isdigit(format[0]) || format[0] == '.') {
+		if (format[0] == '.') {
+			if (dot_found) {
+				diag->Report(placeholderLocation, diag_badPrecision) << placeholderText;
+				return argNum;
+			}
+			dot_found = true;
+		}
+		format = format.substr(1);
+	}
+
+	const Expr *argExpr = callExpr->getArg(argIdx);
+	const QualType qualType = argExpr->getType();
+	const Type *valueType = qualType->getUnqualifiedDesugaredType();
+
+	if (format == "d" || format == "x" || format == "X" || format == "o" || format == "b" || format == "w") {
+		if (!valueType->isSpecificBuiltinType(BuiltinType::Int)) {
+			diag->Report(argExpr->getExprLoc(), diag_wrongArgExprType) << "int" << placeholderText << qualType.getAsString();
+		}
+	} else if (format == "ud" || format == "ux" || format == "uX" || format == "uo" || format == "ub") {
+		if (!valueType->isSpecificBuiltinType(BuiltinType::UInt)) {
+			diag->Report(argExpr->getExprLoc(), diag_wrongArgExprType) << "unsigned int" << placeholderText << qualType.getAsString();
+		}
+	} else if (format == "ld" || format == "lx" || format == "lX" || format == "lo" || format == "lb") {
+		if (!valueType->isSpecificBuiltinType(BuiltinType::Long)) {
+			diag->Report(argExpr->getExprLoc(), diag_wrongArgExprType) << "long" << placeholderText << qualType.getAsString();
+		}
+	} else if (format == "uld" || format == "ulx" || format == "ulX" || format == "ulo" || format == "ulb") {
+		if (!valueType->isSpecificBuiltinType(BuiltinType::ULong)) {
+			diag->Report(argExpr->getExprLoc(), diag_wrongArgExprType) << "unsigned long" << placeholderText << qualType.getAsString();
+		}
+	} else if (format == "lld" || format == "llx" || format == "llX" || format == "llo" || format == "llb") {
+		if (!valueType->isSpecificBuiltinType(BuiltinType::LongLong)) {
+			diag->Report(argExpr->getExprLoc(), diag_wrongArgExprType) << "long long" << placeholderText << qualType.getAsString();
+		}
+	} else if (format == "ulld" || format == "ullx" || format == "ullX" || format == "ullo" || format == "ullb") {
+		if (!valueType->isSpecificBuiltinType(BuiltinType::ULongLong)) {
+			diag->Report(argExpr->getExprLoc(), diag_wrongArgExprType) << "unsigned long long" << placeholderText << qualType.getAsString();
+		}
+	} else if (format == "f" || format == "g" || format == "gs") {
+		if (!valueType->isSpecificBuiltinType(BuiltinType::Double)) {
+			diag->Report(argExpr->getExprLoc(), diag_wrongArgExprType) << "float or double" << placeholderText << qualType.getAsString();
+		}
+	} else if (format == "c") {
+		if (!valueType->isSpecificBuiltinType(BuiltinType::UInt) &&
+			!valueType->isSpecificBuiltinType(BuiltinType::Int)) {
+			diag->Report(argExpr->getExprLoc(), diag_wrongArgExprType) << "char, short or int" << placeholderText << qualType.getAsString();
+		}
+	} else if (format == "s") {
+		if (!valueType->isPointerType()
+			|| !valueType->getPointeeType()->getUnqualifiedDesugaredType()->isCharType()) {
+			diag->Report(argExpr->getExprLoc(), diag_wrongArgExprType) << "char *" << placeholderText << qualType.getAsString();
+		}
+	} else if (format == "t") {
+		const CXXRecordDecl *pointeeType = valueType->isPointerType() ?
+			valueType->getPointeeType()->getAsCXXRecordDecl() : 0;
+		if (pointeeType == 0 || pointeeType->getQualifiedNameAsString() != "GooString") {
+			diag->Report(argExpr->getExprLoc(), diag_wrongArgExprType) << "GooString *" << placeholderText << qualType.getAsString();
+		}
+	} else {
+		diag->Report(placeholderLocation, diag_badType) << placeholderText;
+		return argNum;
+	}
+
+	return argNum;
+}
+
+class GooStringFormatCheckerConsumer : public clang::ASTConsumer {
+public:
+	GooStringFormatCheckerConsumer(CompilerInstance *compInst)
+	: visitor(compInst) {
+	}
+
+	virtual void HandleTranslationUnit(clang::ASTContext &ctx) {
+		visitor.TraverseDecl(ctx.getTranslationUnitDecl());
+	}
+
+private:
+	GooStringFormatCheckerVisitor visitor;
+};
+
+class GooStringFormatCheckerAction : public PluginASTAction
+{
+protected:
+	ASTConsumer *CreateASTConsumer(CompilerInstance &compInst, llvm::StringRef inFile) {
+		return new GooStringFormatCheckerConsumer(&compInst);
+	}
+
+	bool ParseArgs(const CompilerInstance &compInst, const std::vector<std::string>& args) {
+		if (args.size() != 0) {
+			DiagnosticsEngine &D = compInst.getDiagnostics();
+			D.Report(D.getCustomDiagID(DiagnosticsEngine::Error, "goostring-format-checker takes no arguments"));
+			return false;
+		} else {
+			return true;
+		}
+	}
+};
+
+}
+
+static FrontendPluginRegistry::Add<GooStringFormatCheckerAction>
+X("goostring-format-checker", "Checks usage of GooString::format-like functions");


More information about the poppler mailing list