From a3459ddf08a198e6a0add44754bb0f8e97aac729 Mon Sep 17 00:00:00 2001
From: Robert Marsh <rdmarsh@semmle.com>
Date: Wed, 1 Aug 2018 12:25:29 -0700
Subject: [PATCH 01/18] C++: add support for custom wide character sizes

Certain Microsoft projects, such as CoreCLR and ChakraCore, use a
library called the PAL, which enables two-byte strings in the printf
family of functions, even when built on a platform with four-byte
strings. This adds support for determining the size of a wide character
from the definitions of such functions, rather than assuming that they
match the compiler's wchar_t.
---
 cpp/ql/src/semmle/code/cpp/commons/Printf.qll | 17 ++++++-
 .../cpp/models/implementations/Printf.qll     | 36 +++++++++----
 .../WrongTypeFormatArguments.expected         |  2 +
 .../WrongTypeFormatArguments.qlref            |  1 +
 .../Linux_two_byte_wprintf/printf.cpp         | 51 +++++++++++++++++++
 5 files changed, 96 insertions(+), 11 deletions(-)
 create mode 100644 cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/WrongTypeFormatArguments.expected
 create mode 100644 cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/WrongTypeFormatArguments.qlref
 create mode 100644 cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/printf.cpp

diff --git a/cpp/ql/src/semmle/code/cpp/commons/Printf.qll b/cpp/ql/src/semmle/code/cpp/commons/Printf.qll
index 5125c972f4bb..542e47a79c6b 100644
--- a/cpp/ql/src/semmle/code/cpp/commons/Printf.qll
+++ b/cpp/ql/src/semmle/code/cpp/commons/Printf.qll
@@ -28,6 +28,15 @@ class AttributeFormattingFunction extends FormattingFunction {
   }
 }
 
+Type getAPrimitiveVariadicFormatterWideType() {
+  exists(TopLevelFunction f, int formatParamIndex |
+    primitiveVariadicFormatter(f, formatParamIndex, true) and
+    result = f.getParameter(formatParamIndex).getType().getUnspecifiedType() and
+    result.(PointerType).getBaseType().getSize() != 1 and
+    f.hasDefinition()
+  )
+}
+
 /**
  * A standard function such as `vprintf` that has a format parameter
  * and a variable argument list of type `va_arg`.
@@ -722,7 +731,13 @@ class FormatLiteral extends Literal {
 
   private Type getConversionType5(int n) {
     exists(string cnv | cnv = this.getEffectiveStringConversionChar(n) |
-      cnv="S" and result.(PointerType).getBaseType().hasName("wchar_t") 
+      cnv="S" and
+      (
+        result = getAPrimitiveVariadicFormatterWideType()
+        or
+        not exists(getAPrimitiveVariadicFormatterWideType()) and
+        result.(PointerType).getBaseType().hasName("wchar_t")
+      )
     )
   }
 
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll
index 06c6a86e4a10..8458a7165af9 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll
@@ -12,7 +12,8 @@ class Printf extends FormattingFunction {
       hasGlobalName("wprintf") or
       hasGlobalName("wprintf_s") or
       hasGlobalName("g_printf")
-    )
+    ) and
+    not hasDefinition()
   }
 
   override int getFormatParameterIndex() { result=0 }
@@ -26,7 +27,15 @@ class Printf extends FormattingFunction {
  * The standard functions `fprintf`, `fwprintf` and their glib variants.
  */
 class Fprintf extends FormattingFunction {
-  Fprintf() { this instanceof TopLevelFunction and (hasGlobalName("fprintf") or hasGlobalName("fwprintf") or hasGlobalName("g_fprintf"))}
+  Fprintf() {
+    this instanceof TopLevelFunction and
+    (
+      hasGlobalName("fprintf") or
+      hasGlobalName("fwprintf") or
+      hasGlobalName("g_fprintf")
+    ) and
+    not hasDefinition()
+  }
 
   override int getFormatParameterIndex() { result=1 }
   override predicate isWideCharDefault() { hasGlobalName("fwprintf") }
@@ -47,7 +56,8 @@ class Sprintf extends FormattingFunction {
       hasGlobalName("g_strdup_printf") or
       hasGlobalName("g_sprintf") or
       hasGlobalName("__builtin___sprintf_chk")
-    )
+    ) and
+    not hasDefinition()
   }
 
   override predicate isWideCharDefault() {
@@ -100,7 +110,8 @@ class Snprintf extends FormattingFunction {
       or hasGlobalName("g_snprintf")
       or hasGlobalName("wnsprintf")
       or hasGlobalName("__builtin___snprintf_chk")
-    )
+    ) and
+    not hasDefinition()
   }
 
   override int getFormatParameterIndex() {
@@ -133,10 +144,13 @@ class Snprintf extends FormattingFunction {
    * in the buffer.
    */
   predicate returnsFullFormatLength() {
-    hasGlobalName("snprintf") or
-    hasGlobalName("g_snprintf") or
-    hasGlobalName("__builtin___snprintf_chk") or
-    hasGlobalName("snprintf_s")
+    (
+      hasGlobalName("snprintf") or
+      hasGlobalName("g_snprintf") or
+      hasGlobalName("__builtin___snprintf_chk") or
+      hasGlobalName("snprintf_s")
+    ) and
+    not hasDefinition()
   }
 
   override int getSizeParameterIndex() {
@@ -158,7 +172,8 @@ class StringCchPrintf extends FormattingFunction {
       or hasGlobalName("StringCbPrintfEx")
       or hasGlobalName("StringCbPrintf_l")
       or hasGlobalName("StringCbPrintf_lEx")
-    )
+    ) and
+    not hasDefinition()
   }
 
   override int getFormatParameterIndex() {
@@ -187,7 +202,8 @@ class Syslog extends FormattingFunction {
   Syslog() {
     this instanceof TopLevelFunction and (
       hasGlobalName("syslog")
-    )
+    ) and
+    not hasDefinition()
   }
 
   override int getFormatParameterIndex() { result=1 }
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/WrongTypeFormatArguments.expected
new file mode 100644
index 000000000000..6b772112fe8c
--- /dev/null
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/WrongTypeFormatArguments.expected	
@@ -0,0 +1,2 @@
+| printf.cpp:43:29:43:35 | test | This argument should be of type 'char *' but is of type 'char16_t *' |
+| printf.cpp:50:29:50:35 | test | This argument should be of type 'char16_t *' but is of type 'wchar_t *' |
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/WrongTypeFormatArguments.qlref b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/WrongTypeFormatArguments.qlref
new file mode 100644
index 000000000000..6f557ace55a5
--- /dev/null
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/WrongTypeFormatArguments.qlref	
@@ -0,0 +1 @@
+Likely Bugs/Format/WrongTypeFormatArguments.ql
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/printf.cpp b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/printf.cpp
new file mode 100644
index 000000000000..e738b10470ef
--- /dev/null
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/printf.cpp	
@@ -0,0 +1,51 @@
+/*
+ * Test for custom definitions of *wprintf using different types than the
+ * platform wide character type.
+ */
+
+#define WCHAR char16_t
+typedef void *va_list;
+#define va_start(va, other)
+#define va_end(args)
+
+int	vswprintf(WCHAR *dest, WCHAR *format, va_list args) {
+	return 0;
+}
+
+int swprintf(WCHAR *dest, WCHAR *format, ...) {
+	va_list args;
+	va_start(args, format);
+
+	int ret = vswprintf(dest, format, args);
+
+	va_end(args);
+
+	return ret;
+}
+
+int sprintf(char *dest, char *format, ...);
+
+void test1() {
+	WCHAR string[20];
+
+	swprintf(string, u"test %s", u"test");
+}
+
+void test2() {
+	char string[20];
+
+	sprintf(string, "test %S", u"test");
+}
+
+void test3() {
+	char string[20];
+
+	sprintf(string, "test %s", u"test");
+}
+
+
+void test4() {
+	char string[20];
+
+	sprintf(string, "test %S", L"test");
+}

From 5b8925c6993789749ed426983f110a73cdce73a6 Mon Sep 17 00:00:00 2001
From: Robert Marsh <rdmarsh@semmle.com>
Date: Wed, 1 Aug 2018 12:45:52 -0700
Subject: [PATCH 02/18] C++: document new predicate

---
 cpp/ql/src/semmle/code/cpp/commons/Printf.qll | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/cpp/ql/src/semmle/code/cpp/commons/Printf.qll b/cpp/ql/src/semmle/code/cpp/commons/Printf.qll
index 542e47a79c6b..c887d5b69d65 100644
--- a/cpp/ql/src/semmle/code/cpp/commons/Printf.qll
+++ b/cpp/ql/src/semmle/code/cpp/commons/Printf.qll
@@ -28,6 +28,10 @@ class AttributeFormattingFunction extends FormattingFunction {
   }
 }
 
+/**
+ * A type that is used as a format string by a wide variadic formatter such as
+ * `vwprintf`.
+ */
 Type getAPrimitiveVariadicFormatterWideType() {
   exists(TopLevelFunction f, int formatParamIndex |
     primitiveVariadicFormatter(f, formatParamIndex, true) and

From fe8f7e96249157fe91d810c807706544d41833ee Mon Sep 17 00:00:00 2001
From: Robert Marsh <rdmarsh@semmle.com>
Date: Tue, 21 Aug 2018 12:00:33 -0700
Subject: [PATCH 03/18] C++: consider attributes when finding wide string
 functions

---
 cpp/ql/src/semmle/code/cpp/commons/Printf.qll      | 14 ++++++++++----
 .../code/cpp/models/implementations/Printf.qll     | 12 ++++++------
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/commons/Printf.qll b/cpp/ql/src/semmle/code/cpp/commons/Printf.qll
index c887d5b69d65..04a4b7c62784 100644
--- a/cpp/ql/src/semmle/code/cpp/commons/Printf.qll
+++ b/cpp/ql/src/semmle/code/cpp/commons/Printf.qll
@@ -30,15 +30,21 @@ class AttributeFormattingFunction extends FormattingFunction {
 
 /**
  * A type that is used as a format string by a wide variadic formatter such as
- * `vwprintf`.
+ * `vwprintf` or by a user-defined formatting function with the GNU `format`
+ * attribute.
  */
-Type getAPrimitiveVariadicFormatterWideType() {
+Type getAFormatterWideType() {
   exists(TopLevelFunction f, int formatParamIndex |
     primitiveVariadicFormatter(f, formatParamIndex, true) and
     result = f.getParameter(formatParamIndex).getType().getUnspecifiedType() and
     result.(PointerType).getBaseType().getSize() != 1 and
     f.hasDefinition()
   )
+  or
+  exists(AttributeFormattingFunction f, int formatParamIndex |
+    result = f.getParameter(formatParamIndex).getType().getUnspecifiedType() and
+    result.(PointerType).getBaseType().getSize() != 1
+  )
 }
 
 /**
@@ -737,9 +743,9 @@ class FormatLiteral extends Literal {
     exists(string cnv | cnv = this.getEffectiveStringConversionChar(n) |
       cnv="S" and
       (
-        result = getAPrimitiveVariadicFormatterWideType()
+        result = getAFormatterWideType()
         or
-        not exists(getAPrimitiveVariadicFormatterWideType()) and
+        not exists(getAFormatterWideType()) and
         result.(PointerType).getBaseType().hasName("wchar_t")
       )
     )
diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll
index 8458a7165af9..2c79014da0e4 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll
@@ -13,7 +13,7 @@ class Printf extends FormattingFunction {
       hasGlobalName("wprintf_s") or
       hasGlobalName("g_printf")
     ) and
-    not hasDefinition()
+    not exists(getADeclarationEntry().getFile().getRelativePath())
   }
 
   override int getFormatParameterIndex() { result=0 }
@@ -34,7 +34,7 @@ class Fprintf extends FormattingFunction {
       hasGlobalName("fwprintf") or
       hasGlobalName("g_fprintf")
     ) and
-    not hasDefinition()
+    not exists(getADeclarationEntry().getFile().getRelativePath())
   }
 
   override int getFormatParameterIndex() { result=1 }
@@ -57,7 +57,7 @@ class Sprintf extends FormattingFunction {
       hasGlobalName("g_sprintf") or
       hasGlobalName("__builtin___sprintf_chk")
     ) and
-    not hasDefinition()
+    not exists(getADeclarationEntry().getFile().getRelativePath())
   }
 
   override predicate isWideCharDefault() {
@@ -111,7 +111,7 @@ class Snprintf extends FormattingFunction {
       or hasGlobalName("wnsprintf")
       or hasGlobalName("__builtin___snprintf_chk")
     ) and
-    not hasDefinition()
+    not exists(getADeclarationEntry().getFile().getRelativePath())
   }
 
   override int getFormatParameterIndex() {
@@ -150,7 +150,7 @@ class Snprintf extends FormattingFunction {
       hasGlobalName("__builtin___snprintf_chk") or
       hasGlobalName("snprintf_s")
     ) and
-    not hasDefinition()
+    not exists(getADeclarationEntry().getFile().getRelativePath())
   }
 
   override int getSizeParameterIndex() {
@@ -173,7 +173,7 @@ class StringCchPrintf extends FormattingFunction {
       or hasGlobalName("StringCbPrintf_l")
       or hasGlobalName("StringCbPrintf_lEx")
     ) and
-    not hasDefinition()
+    not exists(getADeclarationEntry().getFile().getRelativePath())
   }
 
   override int getFormatParameterIndex() {

From 6e5207ce3c2a66b7150e1f4b0305f196c495dd82 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 7 Sep 2018 10:25:24 +0100
Subject: [PATCH 04/18] CPP: Allow declarations of library printf functions in
 source (repairs most of the tests).

---
 .../code/cpp/models/implementations/Printf.qll       | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll
index 2c79014da0e4..bd8b6d87cca8 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll
@@ -13,7 +13,7 @@ class Printf extends FormattingFunction {
       hasGlobalName("wprintf_s") or
       hasGlobalName("g_printf")
     ) and
-    not exists(getADeclarationEntry().getFile().getRelativePath())
+    not exists(getDefinition().getFile().getRelativePath())
   }
 
   override int getFormatParameterIndex() { result=0 }
@@ -34,7 +34,7 @@ class Fprintf extends FormattingFunction {
       hasGlobalName("fwprintf") or
       hasGlobalName("g_fprintf")
     ) and
-    not exists(getADeclarationEntry().getFile().getRelativePath())
+    not exists(getDefinition().getFile().getRelativePath())
   }
 
   override int getFormatParameterIndex() { result=1 }
@@ -57,7 +57,7 @@ class Sprintf extends FormattingFunction {
       hasGlobalName("g_sprintf") or
       hasGlobalName("__builtin___sprintf_chk")
     ) and
-    not exists(getADeclarationEntry().getFile().getRelativePath())
+    not exists(getDefinition().getFile().getRelativePath())
   }
 
   override predicate isWideCharDefault() {
@@ -111,7 +111,7 @@ class Snprintf extends FormattingFunction {
       or hasGlobalName("wnsprintf")
       or hasGlobalName("__builtin___snprintf_chk")
     ) and
-    not exists(getADeclarationEntry().getFile().getRelativePath())
+    not exists(getDefinition().getFile().getRelativePath())
   }
 
   override int getFormatParameterIndex() {
@@ -150,7 +150,7 @@ class Snprintf extends FormattingFunction {
       hasGlobalName("__builtin___snprintf_chk") or
       hasGlobalName("snprintf_s")
     ) and
-    not exists(getADeclarationEntry().getFile().getRelativePath())
+    not exists(getDefinition().getFile().getRelativePath())
   }
 
   override int getSizeParameterIndex() {
@@ -173,7 +173,7 @@ class StringCchPrintf extends FormattingFunction {
       or hasGlobalName("StringCbPrintf_l")
       or hasGlobalName("StringCbPrintf_lEx")
     ) and
-    not exists(getADeclarationEntry().getFile().getRelativePath())
+    not exists(getDefinition().getFile().getRelativePath())
   }
 
   override int getFormatParameterIndex() {

From e74721e3a40826c58ddf787c120a3693de25a37e Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 7 Sep 2018 18:10:30 +0100
Subject: [PATCH 05/18] CPP: Test fixes as a result of changes.

---
 .../TooManyFormatArguments.expected                           | 1 -
 .../WrongNumberOfFormatArguments.expected                     | 1 -
 .../Format/WrongNumberOfFormatArguments/custom_printf.cpp     | 4 ++--
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/TooManyFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/TooManyFormatArguments.expected
index 3c94ae894346..0d9d84285f16 100644
--- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/TooManyFormatArguments.expected	
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/TooManyFormatArguments.expected	
@@ -1,5 +1,4 @@
 | custom_printf.cpp:31:5:31:12 | call to myPrintf | Format expects 2 arguments but given 3 |
-| custom_printf.cpp:44:2:44:7 | call to printf | Format expects 0 arguments but given 2 |
 | macros.cpp:12:2:12:31 | call to printf | Format expects 2 arguments but given 3 |
 | macros.cpp:16:2:16:30 | call to printf | Format expects 2 arguments but given 3 |
 | test.c:7:2:7:7 | call to printf | Format expects 0 arguments but given 1 |
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/WrongNumberOfFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/WrongNumberOfFormatArguments.expected
index e320187584f8..db4318a4b96b 100644
--- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/WrongNumberOfFormatArguments.expected	
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/WrongNumberOfFormatArguments.expected	
@@ -1,5 +1,4 @@
 | custom_printf.cpp:29:5:29:12 | call to myPrintf | Format expects 2 arguments but given 1 |
-| custom_printf.cpp:45:2:45:7 | call to printf | Format expects 2 arguments but given 0 |
 | macros.cpp:14:2:14:37 | call to printf | Format expects 4 arguments but given 3 |
 | macros.cpp:21:2:21:36 | call to printf | Format expects 4 arguments but given 3 |
 | test.c:9:2:9:7 | call to printf | Format expects 1 arguments but given 0 |
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/custom_printf.cpp b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/custom_printf.cpp
index ef7e54b468ba..b80e14e7410e 100644
--- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/custom_printf.cpp	
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/custom_printf.cpp	
@@ -41,6 +41,6 @@ void test_custom_printf2()
 {
 	//     notTheFormat  format    ...
 	printf(0,            "%i %i",  100, 200); // GOOD
-	printf("",           "%i %i",  100, 200); // GOOD [FALSE POSITIVE]
-	printf("%i %i",      ""        );         // GOOD [FALSE POSITIVE]
+	printf("",           "%i %i",  100, 200); // GOOD
+	printf("%i %i",      ""        );         // GOOD
 }

From 39f030b8f7458105b50f7258718993a1862006e3 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 7 Sep 2018 09:26:40 +0100
Subject: [PATCH 06/18] CPP: Annotate test.

---
 .../WrongTypeFormatArguments.expected                  |  4 ++--
 .../Linux_two_byte_wprintf/printf.cpp                  | 10 ++++++----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/WrongTypeFormatArguments.expected
index 6b772112fe8c..420a97ac72f1 100644
--- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/WrongTypeFormatArguments.expected	
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/WrongTypeFormatArguments.expected	
@@ -1,2 +1,2 @@
-| printf.cpp:43:29:43:35 | test | This argument should be of type 'char *' but is of type 'char16_t *' |
-| printf.cpp:50:29:50:35 | test | This argument should be of type 'char16_t *' but is of type 'wchar_t *' |
+| printf.cpp:45:29:45:35 | test | This argument should be of type 'char *' but is of type 'char16_t *' |
+| printf.cpp:52:29:52:35 | test | This argument should be of type 'char16_t *' but is of type 'wchar_t *' |
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/printf.cpp b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/printf.cpp
index e738b10470ef..6e0a7d3b383b 100644
--- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/printf.cpp	
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/printf.cpp	
@@ -25,27 +25,29 @@ int swprintf(WCHAR *dest, WCHAR *format, ...) {
 
 int sprintf(char *dest, char *format, ...);
 
+// ---
+
 void test1() {
 	WCHAR string[20];
 
-	swprintf(string, u"test %s", u"test");
+	swprintf(string, u"test %s", u"test"); // GOOD
 }
 
 void test2() {
 	char string[20];
 
-	sprintf(string, "test %S", u"test");
+	sprintf(string, "test %S", u"test"); // GOOD
 }
 
 void test3() {
 	char string[20];
 
-	sprintf(string, "test %s", u"test");
+	sprintf(string, "test %s", u"test"); // BAD: `char16_t` string parameter read as `char` string
 }
 
 
 void test4() {
 	char string[20];
 
-	sprintf(string, "test %S", L"test");
+	sprintf(string, "test %S", L"test"); // BAD: `wchar_t` string parameter read as `char16_t` string
 }

From 2af56b89b12e1281ba0c776c3d9081d0f2eb7f49 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 7 Sep 2018 10:47:57 +0100
Subject: [PATCH 07/18] CPP: Add a test where different wide types are present.

---
 .../WrongTypeFormatArguments.expected         | 12 ++++++
 .../WrongTypeFormatArguments.qlref            |  1 +
 .../Linux_mixed_byte_wprintf/tests.cpp        | 40 +++++++++++++++++++
 3 files changed, 53 insertions(+)
 create mode 100644 cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected
 create mode 100644 cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.qlref
 create mode 100644 cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp

diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected
new file mode 100644
index 000000000000..ea24e2970780
--- /dev/null
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected	
@@ -0,0 +1,12 @@
+| tests.cpp:18:15:18:22 | Hello | This argument should be of type 'char *' but is of type 'char16_t *' |
+| tests.cpp:19:15:19:22 | Hello | This argument should be of type 'char *' but is of type 'wchar_t *' |
+| tests.cpp:21:15:21:21 | Hello | This argument should be of type 'wchar_t *' but is of type 'char *' |
+| tests.cpp:22:15:22:22 | Hello | This argument should be of type 'wchar_t *' but is of type 'char16_t *' |
+| tests.cpp:25:17:25:23 | Hello | This argument should be of type 'wchar_t *' but is of type 'char *' |
+| tests.cpp:26:17:26:24 | Hello | This argument should be of type 'wchar_t *' but is of type 'char16_t *' |
+| tests.cpp:30:17:30:24 | Hello | This argument should be of type 'char *' but is of type 'char16_t *' |
+| tests.cpp:31:17:31:24 | Hello | This argument should be of type 'char *' but is of type 'wchar_t *' |
+| tests.cpp:33:36:33:42 | Hello | This argument should be of type 'wchar_t *' but is of type 'char *' |
+| tests.cpp:34:36:34:43 | Hello | This argument should be of type 'wchar_t *' but is of type 'char16_t *' |
+| tests.cpp:38:36:38:43 | Hello | This argument should be of type 'char *' but is of type 'char16_t *' |
+| tests.cpp:39:36:39:43 | Hello | This argument should be of type 'char *' but is of type 'wchar_t *' |
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.qlref b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.qlref
new file mode 100644
index 000000000000..6f557ace55a5
--- /dev/null
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.qlref	
@@ -0,0 +1 @@
+Likely Bugs/Format/WrongTypeFormatArguments.ql
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp
new file mode 100644
index 000000000000..aac1b4cbb92f
--- /dev/null
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp	
@@ -0,0 +1,40 @@
+/*
+ * Test for custom definitions of *wprintf using different types than the
+ * platform wide character type.
+ */
+
+typedef unsigned int size_t;
+
+int printf(const char * format, ...);
+int wprintf(const wchar_t * format, ...); // on wchar_t
+int swprintf(char16_t * s, size_t n, const char16_t * format, ...); // on char16_t
+
+#define BUF_SIZE (4096)
+
+void tests() {
+	char16_t buffer[BUF_SIZE];
+
+	printf("%s", "Hello"); // GOOD
+	printf("%s", u"Hello"); // BAD: expecting char
+	printf("%s", L"Hello"); // BAD: expecting char
+
+	printf("%S", "Hello"); // BAD: expecting wchar_t or char16_t
+	printf("%S", u"Hello"); // GOOD [FALSE POSITIVE]
+	printf("%S", L"Hello"); // GOOD
+
+	wprintf(L"%s", "Hello"); // BAD: expecting wchar_t
+	wprintf(L"%s", u"Hello"); // BAD: expecting wchar_t
+	wprintf(L"%s", L"Hello"); // GOOD
+
+	wprintf(L"%S", "Hello"); // GOOD
+	wprintf(L"%S", u"Hello"); // BAD: expecting char
+	wprintf(L"%S", L"Hello"); // BAD: expecting char
+
+	swprintf(buffer, BUF_SIZE, u"%s", "Hello"); // BAD: expecting char16_t
+	swprintf(buffer, BUF_SIZE, u"%s", u"Hello"); // GOOD [FALSE POSITIVE]
+	swprintf(buffer, BUF_SIZE, u"%s", L"Hello"); // BAD: expecting char16_t [NOT DETECTED]
+
+	swprintf(buffer, BUF_SIZE, u"%S", "Hello"); // GOOD
+	swprintf(buffer, BUF_SIZE, u"%S", u"Hello"); // BAD: expecting char
+	swprintf(buffer, BUF_SIZE, u"%S", L"Hello"); // BAD: expecting char
+}

From 800555865a4792ed9fc04ae7f49a3b581fbf3d38 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Thu, 13 Sep 2018 14:41:43 +0100
Subject: [PATCH 08/18] CPP: More test cases.

---
 .../WrongTypeFormatArguments.expected         |  1 +
 .../Microsoft_no_wchar/printf1.h              | 28 +++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/WrongTypeFormatArguments.expected
index fa245ce45bf5..01ad5d8f33a0 100644
--- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/WrongTypeFormatArguments.expected	
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/WrongTypeFormatArguments.expected	
@@ -17,6 +17,7 @@
 | printf1.h:74:19:74:22 | C_ST | This argument should be of type 'ssize_t' but is of type 'unsigned long long' |
 | printf1.h:75:19:75:28 | sizeof(<expr>) | This argument should be of type 'ssize_t' but is of type 'unsigned long long' |
 | printf1.h:84:23:84:35 | ... - ... | This argument should be of type 'ssize_t' but is of type 'long long' |
+| printf1.h:130:18:130:18 | 0 | This argument should be of type 'void *' but is of type 'int' |
 | real_world.h:46:36:46:43 | filename | This argument should be of type 'wchar_t *' but is of type 'char16_t *' |
 | real_world.h:61:21:61:22 | & ... | This argument should be of type 'int *' but is of type 'short *' |
 | real_world.h:62:22:62:23 | & ... | This argument should be of type 'short *' but is of type 'int *' |
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/printf1.h b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/printf1.h
index 3907977df3f0..c41719ff3c63 100644
--- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/printf1.h	
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/printf1.h	
@@ -101,3 +101,31 @@ void fun1(unsigned char* a, unsigned char* b) {
   printf("%td\n", pdt); // GOOD
   printf("%td\n", a-b); // GOOD
 }
+
+typedef wchar_t WCHAR_T; // WCHAR_T -> wchar_t -> int
+typedef int MYCHAR; // MYCHAR -> int (notably not via the wchar_t typedef)
+
+void fun2() {
+  wchar_t *myString1;
+  WCHAR_T *myString2;
+  int *myString3;
+  MYCHAR *myString4;
+
+  printf("%S", myString1); // GOOD
+  printf("%S", myString2); // GOOD
+  printf("%S", myString3); // GOOD
+  printf("%S", myString4); // GOOD
+}
+
+typedef void *VOIDPTR;
+typedef int (*FUNPTR)(int);
+
+void fun3(void *p1, VOIDPTR p2, FUNPTR p3, char *p4)
+{
+  printf("%p\n", p1); // GOOD
+  printf("%p\n", p2); // GOOD
+  printf("%p\n", p3); // GOOD
+  printf("%p\n", p4); // GOOD
+  printf("%p\n", p4 + 1); // GOOD
+  printf("%p\n", 0); // GOOD [FALSE POSITIVE]
+}

From 1af6c10888b6f73d59d57dba0da4b7de34139d2a Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 14 Sep 2018 11:14:29 +0100
Subject: [PATCH 09/18] CPP: Add a test where different word sizes are present.

---
 .../WrongTypeFormatArguments.expected           |  4 ++++
 .../WrongTypeFormatArguments.qlref              |  1 +
 .../Linux_mixed_word_size/tests_32.cpp          | 17 +++++++++++++++++
 .../Linux_mixed_word_size/tests_64.cpp          | 17 +++++++++++++++++
 4 files changed, 39 insertions(+)
 create mode 100644 cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.expected
 create mode 100644 cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.qlref
 create mode 100644 cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_32.cpp
 create mode 100644 cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_64.cpp

diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.expected
new file mode 100644
index 000000000000..225d725b628b
--- /dev/null
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.expected	
@@ -0,0 +1,4 @@
+| tests_32.cpp:14:16:14:23 | void_ptr | This argument should be of type 'long' but is of type 'void *' |
+| tests_32.cpp:15:15:15:15 | l | This argument should be of type 'void *' but is of type 'long' |
+| tests_64.cpp:14:16:14:23 | void_ptr | This argument should be of type 'long' but is of type 'void *' |
+| tests_64.cpp:15:15:15:15 | l | This argument should be of type 'void *' but is of type 'long' |
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.qlref b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.qlref
new file mode 100644
index 000000000000..6f557ace55a5
--- /dev/null
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.qlref	
@@ -0,0 +1 @@
+Likely Bugs/Format/WrongTypeFormatArguments.ql
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_32.cpp b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_32.cpp
new file mode 100644
index 000000000000..3c9b802a7a74
--- /dev/null
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_32.cpp	
@@ -0,0 +1,17 @@
+// semmle-extractor-options: --edg --target --edg linux_i686
+/*
+ * Test for printf in a snapshot that contains multiple word/pointer sizes.
+ */
+
+int printf(const char * format, ...);
+
+void test_32()
+{
+	long l;
+	void *void_ptr;
+
+	printf("%li", l); // GOOD
+	printf("%li", void_ptr); // BAD
+	printf("%p", l); // BAD
+	printf("%p", void_ptr); // GOOD
+}
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_64.cpp b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_64.cpp
new file mode 100644
index 000000000000..6b38c4e0245c
--- /dev/null
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_64.cpp	
@@ -0,0 +1,17 @@
+// semmle-extractor-options: --edg --target --edg linux_x86_64
+/*
+ * Test for printf in a snapshot that contains multiple word/pointer sizes.
+ */
+
+int printf(const char * format, ...);
+
+void test_64()
+{
+	long l;
+	void *void_ptr;
+
+	printf("%li", l); // GOOD
+	printf("%li", void_ptr); // BAD
+	printf("%p", l); // BAD
+	printf("%p", void_ptr); // GOOD
+}

From e2be19b555405b4c8c7cef73b7278c8ef5064f31 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 7 Sep 2018 17:32:15 +0100
Subject: [PATCH 10/18] CPP: New mechanism for string types in printf.qll.

---
 cpp/ql/src/semmle/code/cpp/commons/Printf.qll | 81 ++++++++++++-------
 .../models/interfaces/FormattingFunction.qll  | 49 +++++++++++
 .../WrongTypeFormatArguments.expected         |  4 +-
 .../Linux_mixed_byte_wprintf/tests.cpp        |  4 +-
 4 files changed, 104 insertions(+), 34 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/commons/Printf.qll b/cpp/ql/src/semmle/code/cpp/commons/Printf.qll
index 04a4b7c62784..78dcdfe75747 100644
--- a/cpp/ql/src/semmle/code/cpp/commons/Printf.qll
+++ b/cpp/ql/src/semmle/code/cpp/commons/Printf.qll
@@ -239,6 +239,32 @@ class FormatLiteral extends Literal {
     getUse().getTarget().(FormattingFunction).isWideCharDefault()
   }
 
+  /**
+   * Gets the default character type expected for `%s` by this format literal.  Typically
+   * `char` or `wchar_t`.
+   */
+  Type getDefaultCharType() {
+    result = getUse().getTarget().(FormattingFunction).getDefaultCharType()
+  }
+
+  /**
+   * Gets the non-default character type expected for `%S` by this format literal.  Typically
+   * `wchar_t` or `char`.  On some snapshots there may be multiple results where we can't tell
+   * which is correct for a particular function.
+   */
+  Type getNonDefaultCharType() {
+    result = getUse().getTarget().(FormattingFunction).getNonDefaultCharType()
+  }
+
+  /**
+   * Gets the wide character type for this format literal.  This is usually `wchar_t`.  On some
+   * snapshots there may be multiple results where we can't tell which is correct for a
+   * particular function.
+   */
+  Type getWideCharType() {
+    result = getUse().getTarget().(FormattingFunction).getWideCharType()
+  }
+
   /**
    * Holds if this `FormatLiteral` is in a context that supports
    * Microsoft rules and extensions.
@@ -648,7 +674,6 @@ class FormatLiteral extends Literal {
     result = getConversionType2(n) or
     result = getConversionType3(n) or
     result = getConversionType4(n) or
-    result = getConversionType5(n) or
     result = getConversionType6(n) or
     result = getConversionType7(n) or
     result = getConversionType8(n) or
@@ -715,38 +740,34 @@ class FormatLiteral extends Literal {
   }
 
   /**
-   * Gets the 'effective' string type character, that is, 's' (meaning a char string) or
-   * 'S' (meaning a wide string).
-   *  - in the base case this is the same as the format type character.
-   *  - for a `wprintf` or similar function call, the meanings are reversed.
-   *  - the size prefixes 'l'/'w' (long) and 'h' (short) override the
-   *    type character to effectively 'S' or 's' respectively.
+   * Gets the string type required by the nth conversion specifier.
+   *  - in the base case this is the default for the formatting function
+   *    (e.g. `char` for `printf`, `wchar_t` for `wprintf`).
+   *  - the `%S` format character reverses wideness.
+   *  - the size prefixes 'l'/'w' and 'h' override the type character
+   *    to wide or single-byte characters respectively.
    */
-  private string getEffectiveStringConversionChar(int n) {
-    exists(string len, string conv | this.parseConvSpec(n, _, _, _, _, _, len, conv) and (conv = "s" or conv = "S") |
-      (len = "l" and result = "S") or
-      (len = "w" and result = "S") or
-      (len = "h" and result = "s") or
-      (len != "l" and len != "w" and len != "h" and (result = "s" or result = "S") and (if isWideCharDefault() then result != conv else result = conv))
-    )
-  }
-
   private Type getConversionType4(int n) {
-    exists(string cnv, CharType t | cnv = this.getEffectiveStringConversionChar(n) |
-      cnv="s" and t = result.(PointerType).getBaseType()
-      and not t.isExplicitlySigned()
-      and not t.isExplicitlyUnsigned()
-    )
-  }
-
-  private Type getConversionType5(int n) {
-    exists(string cnv | cnv = this.getEffectiveStringConversionChar(n) |
-      cnv="S" and
+    exists(string len, string conv |
+      this.parseConvSpec(n, _, _, _, _, _, len, conv) and
       (
-        result = getAFormatterWideType()
-        or
-        not exists(getAFormatterWideType()) and
-        result.(PointerType).getBaseType().hasName("wchar_t")
+        (
+          (conv = "s" or conv = "S") and
+          len = "h" and
+          result.(PointerType).getBaseType() instanceof PlainCharType
+        ) or (
+          (conv = "s" or conv = "S") and
+          (len = "l" or len = "w") and
+          result.(PointerType).getBaseType() = getWideCharType()
+        ) or (
+          conv = "s" and
+          (len != "l" and len != "w" and len != "h") and
+          result.(PointerType).getBaseType() = getDefaultCharType()
+        ) or (
+          conv = "S" and
+          (len != "l" and len != "w" and len != "h") and
+          result.(PointerType).getBaseType() = getNonDefaultCharType()
+        )
       )
     )
   }
diff --git a/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll b/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll
index 2712348d880d..c4d21933e2b7 100644
--- a/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll
@@ -7,6 +7,18 @@
  */
 
 import semmle.code.cpp.Function
+/**
+ * A type that is used as a format string by any formatting function, or `wchar_t` if
+ * there is none.
+ */
+private Type getAFormatterWideTypeOrDefault() {
+  result = getAFormatterWideType().(PointerType).getBaseType() or
+  (
+    not exists(getAFormatterWideType().(PointerType).getBaseType()) and
+    result instanceof Wchar_t
+  )
+}
+
 /**
  * A standard library function that uses a `printf`-like formatting string.
  */
@@ -20,6 +32,43 @@ abstract class FormattingFunction extends Function {
    */
   predicate isWideCharDefault() { none() }
 
+  /**
+   * Gets the default character type expected for `%s` by this function.  Typically
+   * `char` or `wchar_t`.
+   */
+  Type getDefaultCharType() {
+    result = getParameter(getFormatParameterIndex()).getType().
+      getUnderlyingType().(PointerType).getBaseType().stripTopLevelSpecifiers()
+  }
+
+  /**
+   * Gets the non-default character type expected for `%S` by this function.  Typically
+   * `wchar_t` or `char`.  On some snapshots there may be multiple results where we can't tell
+   * which is correct for a particular function.
+   */
+  Type getNonDefaultCharType() {
+  	(
+  	  getDefaultCharType().getSize() = 1 and
+  	  result = getAFormatterWideTypeOrDefault()
+  	) or (
+  	  getDefaultCharType().getSize() > 1 and
+  	  result instanceof PlainCharType
+  	)
+  }
+
+  /**
+   * Gets the wide character type for this function.  This is usually `wchar_t`.  On some
+   * snapshots there may be multiple results where we can't tell which is correct for a
+   * particular function.
+   */
+  Type getWideCharType() {
+    (
+      result = getDefaultCharType() or
+      result = getNonDefaultCharType()
+    ) and
+    result.getSize() > 1
+  }
+
   /**
    * Gets the position at which the output parameter, if any, occurs.
    */
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected
index ea24e2970780..498ed9a84ddc 100644
--- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected	
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected	
@@ -6,7 +6,7 @@
 | tests.cpp:26:17:26:24 | Hello | This argument should be of type 'wchar_t *' but is of type 'char16_t *' |
 | tests.cpp:30:17:30:24 | Hello | This argument should be of type 'char *' but is of type 'char16_t *' |
 | tests.cpp:31:17:31:24 | Hello | This argument should be of type 'char *' but is of type 'wchar_t *' |
-| tests.cpp:33:36:33:42 | Hello | This argument should be of type 'wchar_t *' but is of type 'char *' |
-| tests.cpp:34:36:34:43 | Hello | This argument should be of type 'wchar_t *' but is of type 'char16_t *' |
+| tests.cpp:33:36:33:42 | Hello | This argument should be of type 'char16_t *' but is of type 'char *' |
+| tests.cpp:35:36:35:43 | Hello | This argument should be of type 'char16_t *' but is of type 'wchar_t *' |
 | tests.cpp:38:36:38:43 | Hello | This argument should be of type 'char *' but is of type 'char16_t *' |
 | tests.cpp:39:36:39:43 | Hello | This argument should be of type 'char *' but is of type 'wchar_t *' |
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp
index aac1b4cbb92f..f208faded648 100644
--- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp	
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp	
@@ -31,8 +31,8 @@ void tests() {
 	wprintf(L"%S", L"Hello"); // BAD: expecting char
 
 	swprintf(buffer, BUF_SIZE, u"%s", "Hello"); // BAD: expecting char16_t
-	swprintf(buffer, BUF_SIZE, u"%s", u"Hello"); // GOOD [FALSE POSITIVE]
-	swprintf(buffer, BUF_SIZE, u"%s", L"Hello"); // BAD: expecting char16_t [NOT DETECTED]
+	swprintf(buffer, BUF_SIZE, u"%s", u"Hello"); // GOOD
+	swprintf(buffer, BUF_SIZE, u"%s", L"Hello"); // BAD: expecting char16_t
 
 	swprintf(buffer, BUF_SIZE, u"%S", "Hello"); // GOOD
 	swprintf(buffer, BUF_SIZE, u"%S", u"Hello"); // BAD: expecting char

From 89c56486b5e04d187f6220a38b524886232235c1 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Tue, 11 Sep 2018 12:37:25 +0100
Subject: [PATCH 11/18] CPP: Test getDefaultCharType etc.

---
 .../Linux_mixed_byte_wprintf/formattingFunction.expected  | 3 +++
 .../Linux_mixed_byte_wprintf/formattingFunction.ql        | 8 ++++++++
 .../Linux_two_byte_wprintf/formattingFunction.expected    | 2 ++
 .../Linux_two_byte_wprintf/formattingFunction.ql          | 8 ++++++++
 .../Linux_unsigned_chars/formattingFunction.expected      | 4 ++++
 .../Linux_unsigned_chars/formattingFunction.ql            | 8 ++++++++
 6 files changed, 33 insertions(+)
 create mode 100644 cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/formattingFunction.expected
 create mode 100644 cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/formattingFunction.ql
 create mode 100644 cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/formattingFunction.expected
 create mode 100644 cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/formattingFunction.ql
 create mode 100644 cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/formattingFunction.expected
 create mode 100644 cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/formattingFunction.ql

diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/formattingFunction.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/formattingFunction.expected
new file mode 100644
index 000000000000..2096c6aa8caf
--- /dev/null
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/formattingFunction.expected	
@@ -0,0 +1,3 @@
+| tests.cpp:8:5:8:10 | printf | char | wchar_t | wchar_t |
+| tests.cpp:9:5:9:11 | wprintf | wchar_t | char | wchar_t |
+| tests.cpp:10:5:10:12 | swprintf | char16_t | char | char16_t |
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/formattingFunction.ql b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/formattingFunction.ql
new file mode 100644
index 000000000000..45f12aca07f2
--- /dev/null
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/formattingFunction.ql	
@@ -0,0 +1,8 @@
+import cpp
+
+from FormattingFunction f
+select
+  f,
+  concat(f.getDefaultCharType().toString(), ", "),
+  concat(f.getNonDefaultCharType().toString(), ", "),
+  concat(f.getWideCharType().toString(), ", ")
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/formattingFunction.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/formattingFunction.expected
new file mode 100644
index 000000000000..01f6a5ed7530
--- /dev/null
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/formattingFunction.expected	
@@ -0,0 +1,2 @@
+| printf.cpp:15:5:15:12 | swprintf | char16_t | char | char16_t |
+| printf.cpp:26:5:26:11 | sprintf | char | char16_t | char16_t |
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/formattingFunction.ql b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/formattingFunction.ql
new file mode 100644
index 000000000000..45f12aca07f2
--- /dev/null
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/formattingFunction.ql	
@@ -0,0 +1,8 @@
+import cpp
+
+from FormattingFunction f
+select
+  f,
+  concat(f.getDefaultCharType().toString(), ", "),
+  concat(f.getNonDefaultCharType().toString(), ", "),
+  concat(f.getWideCharType().toString(), ", ")
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/formattingFunction.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/formattingFunction.expected
new file mode 100644
index 000000000000..75906fa0aed6
--- /dev/null
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/formattingFunction.expected	
@@ -0,0 +1,4 @@
+| common.h:12:12:12:17 | printf | char | wchar_t | wchar_t |
+| format.h:4:13:4:17 | error | char | wchar_t | wchar_t |
+| real_world.h:8:12:8:18 | fprintf | char | wchar_t | wchar_t |
+| real_world.h:33:6:33:12 | msg_out | char | wchar_t | wchar_t |
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/formattingFunction.ql b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/formattingFunction.ql
new file mode 100644
index 000000000000..45f12aca07f2
--- /dev/null
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/formattingFunction.ql	
@@ -0,0 +1,8 @@
+import cpp
+
+from FormattingFunction f
+select
+  f,
+  concat(f.getDefaultCharType().toString(), ", "),
+  concat(f.getNonDefaultCharType().toString(), ", "),
+  concat(f.getWideCharType().toString(), ", ")

From 580471ab1d327797554caa8c97b59cb9adde9879 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Tue, 11 Sep 2018 15:32:58 +0100
Subject: [PATCH 12/18] CPP: Replace stripTopLevelSpecifiers to emulate old
 behaviour.

---
 .../cpp/models/interfaces/FormattingFunction.qll    | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll b/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll
index c4d21933e2b7..6155dda32ef3 100644
--- a/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll
@@ -19,6 +19,15 @@ private Type getAFormatterWideTypeOrDefault() {
   )
 }
 
+private Type stripTopLevelSpecifiersOnly(Type t) {
+  (
+    result = stripTopLevelSpecifiersOnly(t.(SpecifiedType).getBaseType())
+  ) or (
+    result = t and
+    not t instanceof SpecifiedType
+  )
+}
+
 /**
  * A standard library function that uses a `printf`-like formatting string.
  */
@@ -37,8 +46,8 @@ abstract class FormattingFunction extends Function {
    * `char` or `wchar_t`.
    */
   Type getDefaultCharType() {
-    result = getParameter(getFormatParameterIndex()).getType().
-      getUnderlyingType().(PointerType).getBaseType().stripTopLevelSpecifiers()
+    result = stripTopLevelSpecifiersOnly(getParameter(getFormatParameterIndex()).getType().
+      getUnderlyingType().(PointerType).getBaseType())
   }
 
   /**

From 2841897e3a01308cf844d81e46e269eb9b095448 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Mon, 10 Sep 2018 18:41:12 +0100
Subject: [PATCH 13/18] CPP: Make getAFormatterWideType more general and move
 it into FormattingFunction.qll.

---
 cpp/ql/src/semmle/code/cpp/commons/Printf.qll | 19 ------------
 .../models/interfaces/FormattingFunction.qll  | 30 +++++++++++++------
 .../WrongTypeFormatArguments.expected         |  2 ++
 .../formattingFunction.expected               |  2 +-
 .../Linux_mixed_byte_wprintf/tests.cpp        |  2 +-
 5 files changed, 25 insertions(+), 30 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/commons/Printf.qll b/cpp/ql/src/semmle/code/cpp/commons/Printf.qll
index 78dcdfe75747..eb9c43f5b5bb 100644
--- a/cpp/ql/src/semmle/code/cpp/commons/Printf.qll
+++ b/cpp/ql/src/semmle/code/cpp/commons/Printf.qll
@@ -28,25 +28,6 @@ class AttributeFormattingFunction extends FormattingFunction {
   }
 }
 
-/**
- * A type that is used as a format string by a wide variadic formatter such as
- * `vwprintf` or by a user-defined formatting function with the GNU `format`
- * attribute.
- */
-Type getAFormatterWideType() {
-  exists(TopLevelFunction f, int formatParamIndex |
-    primitiveVariadicFormatter(f, formatParamIndex, true) and
-    result = f.getParameter(formatParamIndex).getType().getUnspecifiedType() and
-    result.(PointerType).getBaseType().getSize() != 1 and
-    f.hasDefinition()
-  )
-  or
-  exists(AttributeFormattingFunction f, int formatParamIndex |
-    result = f.getParameter(formatParamIndex).getType().getUnspecifiedType() and
-    result.(PointerType).getBaseType().getSize() != 1
-  )
-}
-
 /**
  * A standard function such as `vprintf` that has a format parameter
  * and a variable argument list of type `va_arg`.
diff --git a/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll b/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll
index 6155dda32ef3..b5893516af0d 100644
--- a/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll
@@ -7,6 +7,27 @@
  */
 
 import semmle.code.cpp.Function
+
+private Type stripTopLevelSpecifiersOnly(Type t) {
+  (
+    result = stripTopLevelSpecifiersOnly(t.(SpecifiedType).getBaseType())
+  ) or (
+    result = t and
+    not t instanceof SpecifiedType
+  )
+}
+
+/**
+ * A type that is used as a format string by any formatting function.
+ */
+Type getAFormatterWideType() {
+  exists(FormattingFunction ff, Type t |
+    t = stripTopLevelSpecifiersOnly(ff.getDefaultCharType()) and
+    t.getSize() != 1 and
+    result.(PointerType).getBaseType() = t
+  )
+}
+
 /**
  * A type that is used as a format string by any formatting function, or `wchar_t` if
  * there is none.
@@ -19,15 +40,6 @@ private Type getAFormatterWideTypeOrDefault() {
   )
 }
 
-private Type stripTopLevelSpecifiersOnly(Type t) {
-  (
-    result = stripTopLevelSpecifiersOnly(t.(SpecifiedType).getBaseType())
-  ) or (
-    result = t and
-    not t instanceof SpecifiedType
-  )
-}
-
 /**
  * A standard library function that uses a `printf`-like formatting string.
  */
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected
index 498ed9a84ddc..19c17ad221eb 100644
--- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected	
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected	
@@ -1,7 +1,9 @@
 | tests.cpp:18:15:18:22 | Hello | This argument should be of type 'char *' but is of type 'char16_t *' |
 | tests.cpp:19:15:19:22 | Hello | This argument should be of type 'char *' but is of type 'wchar_t *' |
+| tests.cpp:21:15:21:21 | Hello | This argument should be of type 'char16_t *' but is of type 'char *' |
 | tests.cpp:21:15:21:21 | Hello | This argument should be of type 'wchar_t *' but is of type 'char *' |
 | tests.cpp:22:15:22:22 | Hello | This argument should be of type 'wchar_t *' but is of type 'char16_t *' |
+| tests.cpp:23:15:23:22 | Hello | This argument should be of type 'char16_t *' but is of type 'wchar_t *' |
 | tests.cpp:25:17:25:23 | Hello | This argument should be of type 'wchar_t *' but is of type 'char *' |
 | tests.cpp:26:17:26:24 | Hello | This argument should be of type 'wchar_t *' but is of type 'char16_t *' |
 | tests.cpp:30:17:30:24 | Hello | This argument should be of type 'char *' but is of type 'char16_t *' |
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/formattingFunction.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/formattingFunction.expected
index 2096c6aa8caf..bb17832eff79 100644
--- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/formattingFunction.expected	
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/formattingFunction.expected	
@@ -1,3 +1,3 @@
-| tests.cpp:8:5:8:10 | printf | char | wchar_t | wchar_t |
+| tests.cpp:8:5:8:10 | printf | char | char16_t, wchar_t | char16_t, wchar_t |
 | tests.cpp:9:5:9:11 | wprintf | wchar_t | char | wchar_t |
 | tests.cpp:10:5:10:12 | swprintf | char16_t | char | char16_t |
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp
index f208faded648..8fc933f92f44 100644
--- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp	
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp	
@@ -20,7 +20,7 @@ void tests() {
 
 	printf("%S", "Hello"); // BAD: expecting wchar_t or char16_t
 	printf("%S", u"Hello"); // GOOD [FALSE POSITIVE]
-	printf("%S", L"Hello"); // GOOD
+	printf("%S", L"Hello"); // GOOD [FALSE POSITIVE]
 
 	wprintf(L"%s", "Hello"); // BAD: expecting wchar_t
 	wprintf(L"%s", u"Hello"); // BAD: expecting wchar_t

From 94ff2e569312804e5e6555124e271f7e0ab82ecf Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Mon, 10 Sep 2018 21:22:17 +0100
Subject: [PATCH 14/18] CPP: Lets just not report when we're not sure.

---
 cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql   | 3 ++-
 .../WrongTypeFormatArguments.expected                       | 4 ----
 .../Linux_mixed_byte_wprintf/tests.cpp                      | 6 +++---
 .../Linux_mixed_word_size/WrongTypeFormatArguments.expected | 2 --
 .../Linux_mixed_word_size/tests_32.cpp                      | 2 +-
 .../Linux_mixed_word_size/tests_64.cpp                      | 2 +-
 6 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql b/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql
index 984642d109b9..4f6c139b74be 100644
--- a/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql	
+++ b/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql	
@@ -25,7 +25,8 @@ private predicate formattingFunctionCallExpectedType(FormattingFunctionCall ffc,
       ffc.getTarget() = f and
       f.getFormatParameterIndex() = i and
       ffc.getArgument(i) = fl and
-      fl.getConversionType(pos) = expected
+      fl.getConversionType(pos) = expected and
+      count(fl.getConversionType(pos)) = 1
     )
 }
 
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected
index 19c17ad221eb..8e50e74c9e50 100644
--- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected	
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected	
@@ -1,9 +1,5 @@
 | tests.cpp:18:15:18:22 | Hello | This argument should be of type 'char *' but is of type 'char16_t *' |
 | tests.cpp:19:15:19:22 | Hello | This argument should be of type 'char *' but is of type 'wchar_t *' |
-| tests.cpp:21:15:21:21 | Hello | This argument should be of type 'char16_t *' but is of type 'char *' |
-| tests.cpp:21:15:21:21 | Hello | This argument should be of type 'wchar_t *' but is of type 'char *' |
-| tests.cpp:22:15:22:22 | Hello | This argument should be of type 'wchar_t *' but is of type 'char16_t *' |
-| tests.cpp:23:15:23:22 | Hello | This argument should be of type 'char16_t *' but is of type 'wchar_t *' |
 | tests.cpp:25:17:25:23 | Hello | This argument should be of type 'wchar_t *' but is of type 'char *' |
 | tests.cpp:26:17:26:24 | Hello | This argument should be of type 'wchar_t *' but is of type 'char16_t *' |
 | tests.cpp:30:17:30:24 | Hello | This argument should be of type 'char *' but is of type 'char16_t *' |
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp
index 8fc933f92f44..bd5353460bc3 100644
--- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp	
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp	
@@ -18,9 +18,9 @@ void tests() {
 	printf("%s", u"Hello"); // BAD: expecting char
 	printf("%s", L"Hello"); // BAD: expecting char
 
-	printf("%S", "Hello"); // BAD: expecting wchar_t or char16_t
-	printf("%S", u"Hello"); // GOOD [FALSE POSITIVE]
-	printf("%S", L"Hello"); // GOOD [FALSE POSITIVE]
+	printf("%S", "Hello"); // BAD: expecting wchar_t or char16_t [NOT DETECTED]
+	printf("%S", u"Hello"); // GOOD
+	printf("%S", L"Hello"); // GOOD
 
 	wprintf(L"%s", "Hello"); // BAD: expecting wchar_t
 	wprintf(L"%s", u"Hello"); // BAD: expecting wchar_t
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.expected
index 225d725b628b..f3cddba8012b 100644
--- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.expected	
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.expected	
@@ -1,4 +1,2 @@
 | tests_32.cpp:14:16:14:23 | void_ptr | This argument should be of type 'long' but is of type 'void *' |
-| tests_32.cpp:15:15:15:15 | l | This argument should be of type 'void *' but is of type 'long' |
 | tests_64.cpp:14:16:14:23 | void_ptr | This argument should be of type 'long' but is of type 'void *' |
-| tests_64.cpp:15:15:15:15 | l | This argument should be of type 'void *' but is of type 'long' |
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_32.cpp b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_32.cpp
index 3c9b802a7a74..94aa803c6402 100644
--- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_32.cpp	
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_32.cpp	
@@ -12,6 +12,6 @@ void test_32()
 
 	printf("%li", l); // GOOD
 	printf("%li", void_ptr); // BAD
-	printf("%p", l); // BAD
+	printf("%p", l); // BAD [NOT DETECTED]
 	printf("%p", void_ptr); // GOOD
 }
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_64.cpp b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_64.cpp
index 6b38c4e0245c..a9eda51c5c70 100644
--- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_64.cpp	
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_64.cpp	
@@ -12,6 +12,6 @@ void test_64()
 
 	printf("%li", l); // GOOD
 	printf("%li", void_ptr); // BAD
-	printf("%p", l); // BAD
+	printf("%p", l); // BAD [NOT DETECTED]
 	printf("%p", void_ptr); // GOOD
 }

From 605db444a68f7279cf0aac2662a0dd75d87f4eb6 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 5 Oct 2018 13:41:08 +0100
Subject: [PATCH 15/18] CPP: Fix for consistency.

---
 cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll
index bd8b6d87cca8..ef696a526f79 100644
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll
@@ -203,7 +203,7 @@ class Syslog extends FormattingFunction {
     this instanceof TopLevelFunction and (
       hasGlobalName("syslog")
     ) and
-    not hasDefinition()
+    not exists(getDefinition().getFile().getRelativePath())
   }
 
   override int getFormatParameterIndex() { result=1 }

From 67a7b75b8458a3c2c4769873efef89240f804098 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 5 Oct 2018 13:46:20 +0100
Subject: [PATCH 16/18] CPP: Simplify getAFormatterWideType.

---
 .../code/cpp/models/interfaces/FormattingFunction.qll | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll b/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll
index b5893516af0d..757012290e24 100644
--- a/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll
@@ -21,10 +21,9 @@ private Type stripTopLevelSpecifiersOnly(Type t) {
  * A type that is used as a format string by any formatting function.
  */
 Type getAFormatterWideType() {
-  exists(FormattingFunction ff, Type t |
-    t = stripTopLevelSpecifiersOnly(ff.getDefaultCharType()) and
-    t.getSize() != 1 and
-    result.(PointerType).getBaseType() = t
+  exists(FormattingFunction ff |
+    result = stripTopLevelSpecifiersOnly(ff.getDefaultCharType()) and
+    result.getSize() != 1
   )
 }
 
@@ -33,9 +32,9 @@ Type getAFormatterWideType() {
  * there is none.
  */
 private Type getAFormatterWideTypeOrDefault() {
-  result = getAFormatterWideType().(PointerType).getBaseType() or
+  result = getAFormatterWideType() or
   (
-    not exists(getAFormatterWideType().(PointerType).getBaseType()) and
+    not exists(getAFormatterWideType()) and
     result instanceof Wchar_t
   )
 }

From 998b28b3593fa30994e3f2f490ca599bff205228 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 5 Oct 2018 13:21:29 +0100
Subject: [PATCH 17/18] CPP: Change note.

---
 change-notes/1.19/analysis-cpp.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/change-notes/1.19/analysis-cpp.md b/change-notes/1.19/analysis-cpp.md
index 9bc73fda28c4..b1f84b60e51e 100644
--- a/change-notes/1.19/analysis-cpp.md
+++ b/change-notes/1.19/analysis-cpp.md
@@ -13,8 +13,8 @@
 | **Query**                  | **Expected impact**    | **Change**                                                       |
 |----------------------------|------------------------|------------------------------------------------------------------|
 | Resource not released in destructor | Fewer false positive results | Placement new is now excluded from the query. |
-| Wrong type of arguments to formatting function | Fewer false positive results | False positive results involving typedefs have been removed. |
 | Missing return statement (`cpp/missing-return`) | Visible by default | The precision of this query has been increased from 'medium' to 'high', which makes it visible by default in LGTM. It was 'medium' in release 1.17 and 1.18 because it had false positives due to an extractor bug that was fixed in 1.18. |
+| Wrong type of arguments to formatting function | Fewer false positive results | False positive results involving typedefs have been removed.  Expected argument types are determined more accurately, especially for wide string and pointer types.  Custom (non-standard) formatting functions are also identified more accurately. |
 
 ## Changes to QL libraries
 

From 99816d77e3e447ebbd7ea54b676c8a1356d8a8ba Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 5 Oct 2018 17:13:50 +0100
Subject: [PATCH 18/18] CPP: Additional test case fixed in combination with
 typedef work.

---
 .../Microsoft_no_wchar/WrongTypeFormatArguments.expected         | 1 -
 1 file changed, 1 deletion(-)

diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/WrongTypeFormatArguments.expected
index 01ad5d8f33a0..64c9bd0de7c8 100644
--- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/WrongTypeFormatArguments.expected	
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/WrongTypeFormatArguments.expected	
@@ -18,7 +18,6 @@
 | printf1.h:75:19:75:28 | sizeof(<expr>) | This argument should be of type 'ssize_t' but is of type 'unsigned long long' |
 | printf1.h:84:23:84:35 | ... - ... | This argument should be of type 'ssize_t' but is of type 'long long' |
 | printf1.h:130:18:130:18 | 0 | This argument should be of type 'void *' but is of type 'int' |
-| real_world.h:46:36:46:43 | filename | This argument should be of type 'wchar_t *' but is of type 'char16_t *' |
 | real_world.h:61:21:61:22 | & ... | This argument should be of type 'int *' but is of type 'short *' |
 | real_world.h:62:22:62:23 | & ... | This argument should be of type 'short *' but is of type 'int *' |
 | real_world.h:63:22:63:24 | & ... | This argument should be of type 'short *' but is of type 'unsigned int *' |