Merge pull request swiftlang#35525 from mikeash/swift_asprintf-static-checking

[Runtime] Mark swift_asprintf with __attribute__((__format__))

Author: mikeash

include/swift/Demangling/TypeDecoder.h

@@ -331,10 +331,11 @@ getObjCClassOrProtocolName(NodePointer node) {
 #endif
 
 #define MAKE_NODE_TYPE_ERROR(Node, Fmt, ...)                                   \
-  TypeLookupError("TypeDecoder.h:%d: Node kind %u \"%.*s\" - " Fmt, __LINE__,  \
-                  Node->getKind(),                                             \
-                  Node->hasText() ? (int)Node->getText().size() : 0,           \
-                  Node->hasText() ? Node->getText().data() : "", __VA_ARGS__)
+  TYPE_LOOKUP_ERROR_FMT("TypeDecoder.h:%u: Node kind %u \"%.*s\" - " Fmt,      \
+                        __LINE__, (unsigned)Node->getKind(),                   \
+                        Node->hasText() ? (int)Node->getText().size() : 0,     \
+                        Node->hasText() ? Node->getText().data() : "",         \
+                        __VA_ARGS__)
 
 #define MAKE_NODE_TYPE_ERROR0(Node, Str) MAKE_NODE_TYPE_ERROR(Node, "%s", Str)
 
@@ -406,7 +407,7 @@ class TypeDecoder {
     case NodeKind::BoundGenericOtherNominalType: {
       if (Node->getNumChildren() < 2)
         return MAKE_NODE_TYPE_ERROR(Node,
-                                    "fewer children (%u) than required (2)",
+                                    "fewer children (%zu) than required (2)",
                                     Node->getNumChildren());
 
       llvm::SmallVector<BuiltType, 8> args;
@@ -469,13 +470,13 @@ class TypeDecoder {
       // so that the parent type becomes 'S' and not 'P'.
       if (Node->getNumChildren() < 2)
         return MAKE_NODE_TYPE_ERROR(Node,
-                                    "fewer children (%u) than required (2)",
+                                    "fewer children (%zu) than required (2)",
                                     Node->getNumChildren());
 
       const auto &genericArgs = Node->getChild(1);
       if (genericArgs->getNumChildren() != 1)
         return MAKE_NODE_TYPE_ERROR(genericArgs,
-                                    "expected 1 generic argument, saw %u",
+                                    "expected 1 generic argument, saw %zu",
                                     genericArgs->getNumChildren());
 
       return decodeMangledType(genericArgs->getChild(0));
@@ -550,7 +551,7 @@ class TypeDecoder {
       if (Node->getKind() == NodeKind::ProtocolListWithClass) {
         if (Node->getNumChildren() < 2)
           return MAKE_NODE_TYPE_ERROR(Node,
-                                      "fewer children (%u) than required (2)",
+                                      "fewer children (%zu) than required (2)",
                                       Node->getNumChildren());
 
         auto superclassNode = Node->getChild(1);
@@ -579,7 +580,7 @@ class TypeDecoder {
     }
     case NodeKind::DynamicSelf: {
       if (Node->getNumChildren() != 1)
-        return MAKE_NODE_TYPE_ERROR(Node, "expected 1 child, saw %u",
+        return MAKE_NODE_TYPE_ERROR(Node, "expected 1 child, saw %zu",
                                     Node->getNumChildren());
 
       auto selfType = decodeMangledType(Node->getChild(0));
@@ -607,7 +608,7 @@ class TypeDecoder {
     case NodeKind::FunctionType: {
       if (Node->getNumChildren() < 2)
         return MAKE_NODE_TYPE_ERROR(Node,
-                                    "fewer children (%u) than required (2)",
+                                    "fewer children (%zu) than required (2)",
                                     Node->getNumChildren());
 
       FunctionTypeFlags flags;
@@ -655,7 +656,7 @@ class TypeDecoder {
 
       if (Node->getNumChildren() < firstChildIdx + 2)
         return MAKE_NODE_TYPE_ERROR(Node,
-                                    "fewer children (%u) than required (%u)",
+                                    "fewer children (%zu) than required (%u)",
                                     Node->getNumChildren(), firstChildIdx + 2);
 
       bool hasParamFlags = false;
@@ -828,7 +829,7 @@ class TypeDecoder {
       if (Node->getChild(0)->getKind() == NodeKind::TupleElementName) {
         if (Node->getNumChildren() < 2)
           return MAKE_NODE_TYPE_ERROR(Node,
-                                      "fewer children (%u) than required (2)",
+                                      "fewer children (%zu) than required (2)",
                                       Node->getNumChildren());
 
         return decodeMangledType(Node->getChild(1));
@@ -838,15 +839,15 @@ class TypeDecoder {
     case NodeKind::DependentGenericType: {
       if (Node->getNumChildren() < 2)
         return MAKE_NODE_TYPE_ERROR(Node,
-                                    "fewer children (%u) than required (2)",
+                                    "fewer children (%zu) than required (2)",
                                     Node->getNumChildren());
 
       return decodeMangledType(Node->getChild(1));
     }
     case NodeKind::DependentMemberType: {
       if (Node->getNumChildren() < 2)
         return MAKE_NODE_TYPE_ERROR(Node,
-                                    "fewer children (%u) than required (2)",
+                                    "fewer children (%zu) than required (2)",
                                     Node->getNumChildren());
 
       auto base = decodeMangledType(Node->getChild(0));
@@ -866,7 +867,7 @@ class TypeDecoder {
     case NodeKind::DependentAssociatedTypeRef: {
       if (Node->getNumChildren() < 2)
         return MAKE_NODE_TYPE_ERROR(Node,
-                                    "fewer children (%u) than required (2)",
+                                    "fewer children (%zu) than required (2)",
                                     Node->getNumChildren());
 
       return decodeMangledType(Node->getChild(1));
@@ -935,7 +936,7 @@ class TypeDecoder {
     case NodeKind::SugaredDictionary: {
       if (Node->getNumChildren() < 2)
         return MAKE_NODE_TYPE_ERROR(Node,
-                                    "fewer children (%u) than required (2)",
+                                    "fewer children (%zu) than required (2)",
                                     Node->getNumChildren());
 
       auto key = decodeMangledType(Node->getChild(0));
@@ -961,7 +962,7 @@ class TypeDecoder {
     case NodeKind::OpaqueType: {
       if (Node->getNumChildren() < 3)
         return MAKE_NODE_TYPE_ERROR(Node,
-                                    "fewer children (%u) than required (3)",
+                                    "fewer children (%zu) than required (3)",
                                     Node->getNumChildren());
       auto descriptor = Node->getChild(0);
       auto ordinalNode = Node->getChild(1);
@@ -1079,7 +1080,7 @@ class TypeDecoder {
     } else {
       if (node->getNumChildren() < 2)
         return MAKE_NODE_TYPE_ERROR(
-            node, "Number of node children (%u) less than required (2)",
+            node, "Number of node children (%zu) less than required (2)",
             node->getNumChildren());
 
       auto parentContext = node->getChild(0);
@@ -1097,7 +1098,7 @@ class TypeDecoder {
         // Decode the type being extended.
         if (parentContext->getNumChildren() < 2)
           return MAKE_NODE_TYPE_ERROR(parentContext,
-                                      "Number of parentContext children (%u) "
+                                      "Number of parentContext children (%zu) "
                                       "less than required (2)",
                                       node->getNumChildren());
         parentContext = parentContext->getChild(1);

include/swift/Demangling/TypeLookupError.h

@@ -125,18 +125,6 @@ class TypeLookupError {
     };
   }
 
-  /// Construct a TypeLookupError that creates a string using asprintf. The passed-in
-  /// format string and arguments are passed directly to swift_asprintf when
-  /// the string is requested. The arguments are captured and the string is only
-  /// formatted when needed.
-  template <typename... Args>
-  TypeLookupError(const char *fmt, Args... args)
-      : TypeLookupError([=] {
-          char *str;
-          swift_asprintf(&str, fmt, args...);
-          return str;
-        }) {}
-
   /// Construct a TypeLookupError that wraps a function returning a string. The
   /// passed-in function can return either a `std::string` or `char *`. If it
   /// returns `char *` then the string will be destroyed with `free()`.
@@ -201,6 +189,27 @@ template <typename T> class TypeLookupErrorOr {
   bool isError() { return getError() != nullptr; }
 };
 
+/// Construct a TypeLookupError that creates a string using asprintf. The
+/// passed-in format string and arguments are passed directly to swift_asprintf
+/// when the string is requested. The arguments are captured and the string is
+/// only formatted when needed.
+///
+/// The crazy sizeof(swift_asprintf(... construct gives us compile-time type
+/// checking of the format string and arguments, while still letting us use the
+/// variadic template to safely capture the arguments in the lambda.
+#define TYPE_LOOKUP_ERROR_FMT(...)                                             \
+  (sizeof(swift_asprintf(NULL, __VA_ARGS__)), TypeLookupErrorImpl(__VA_ARGS__))
+
+// Implementation for TYPE_LOOKUP_ERROR_FMT. Don't call directly.
+template <typename... Args>
+static TypeLookupError TypeLookupErrorImpl(const char *fmt, Args... args) {
+  return TypeLookupError([=] {
+    char *str;
+    swift_asprintf(&str, fmt, args...);
+    return str;
+  });
+}
+
 } // namespace swift
 
 #endif // SWIFT_DEMANGLING_TypeLookupError_H

include/swift/Runtime/Portability.h

@@ -28,7 +28,8 @@ size_t _swift_strlcpy(char *dst, const char *src, size_t maxlen);
 #ifdef SWIFT_RUNTIME_ATTRIBUTE_ALWAYS_INLINE
 SWIFT_RUNTIME_ATTRIBUTE_ALWAYS_INLINE
 #endif
-inline static int swift_asprintf(char **strp, const char *fmt, ...) {
+__attribute((__format__(__printf__, 2, 3))) inline static int
+swift_asprintf(char **strp, const char *fmt, ...) {
   va_list args;
   va_start(args, fmt);
 #if defined(_WIN32)

stdlib/public/runtime/ProtocolConformance.cpp

@@ -687,9 +687,9 @@ llvm::Optional<TypeLookupError> swift::_checkGenericRequirements(
                                &witnessTable)) {
         const char *protoName =
             req.getProtocol() ? req.getProtocol().getName() : "<null>";
-        return TypeLookupError(
-            "subject type %s does not conform to protocol %s", req.getParam(),
-            protoName);
+        return TYPE_LOOKUP_ERROR_FMT(
+            "subject type %.*s does not conform to protocol %s",
+            (int)req.getParam().size(), req.getParam().data(), protoName);
       }
 
       // If we need a witness table, add it.
@@ -714,8 +714,10 @@ llvm::Optional<TypeLookupError> swift::_checkGenericRequirements(
 
       // Check that the types are equivalent.
       if (subjectType != otherType)
-        return TypeLookupError("subject type %s does not match %s",
-                               req.getParam(), req.getMangledTypeName());
+        return TYPE_LOOKUP_ERROR_FMT(
+            "subject type %.*s does not match %.*s", (int)req.getParam().size(),
+            req.getParam().data(), (int)req.getMangledTypeName().size(),
+            req.getMangledTypeName().data());
 
       continue;
     }
@@ -724,14 +726,14 @@ llvm::Optional<TypeLookupError> swift::_checkGenericRequirements(
       switch (req.getLayout()) {
       case GenericRequirementLayoutKind::Class:
         if (!subjectType->satisfiesClassConstraint())
-          return TypeLookupError(
-              "subject type %s does not satisfy class constraint",
-              req.getParam());
+          return TYPE_LOOKUP_ERROR_FMT(
+              "subject type %.*s does not satisfy class constraint",
+              (int)req.getParam().size(), req.getParam().data());
         continue;
       }
 
       // Unknown layout.
-      return TypeLookupError("unknown layout kind %u", req.getLayout());
+      return TYPE_LOOKUP_ERROR_FMT("unknown layout kind %u", req.getLayout());
     }
 
     case GenericRequirementKind::BaseClass: {
@@ -753,8 +755,10 @@ llvm::Optional<TypeLookupError> swift::_checkGenericRequirements(
       }
 
       if (!isSubclass(subjectType, baseType))
-        return TypeLookupError("%s is not subclass of %s", req.getParam(),
-                               req.getMangledTypeName());
+        return TYPE_LOOKUP_ERROR_FMT(
+            "%.*s is not subclass of %.*s", (int)req.getParam().size(),
+            req.getParam().data(), (int)req.getMangledTypeName().size(),
+            req.getMangledTypeName().data());
 
       continue;
     }
@@ -766,8 +770,8 @@ llvm::Optional<TypeLookupError> swift::_checkGenericRequirements(
     }
 
     // Unknown generic requirement kind.
-    return TypeLookupError("unknown generic requirement kind %u",
-                           req.getKind());
+    return TYPE_LOOKUP_ERROR_FMT("unknown generic requirement kind %u",
+                                 (unsigned)req.getKind());
   }
 
   // Success!

stdlib/public/runtime/SwiftObject.mm

@@ -1411,7 +1411,7 @@ static bool usesNativeSwiftReferenceCounting_nonNull(
     char *log;
     swift_asprintf(
         &log, "%.*s: file %.*s, line %" PRIu32 ", column %" PRIu32 " \n",
-        messageLength, message, filenameLength, filename, line, column);
+        (int)messageLength, message, filenameLength, filename, line, column);
 
     printCurrentBacktrace(2/*framesToSkip*/);
 

unittests/Basic/TypeLookupError.cpp

@@ -24,8 +24,8 @@ TEST(TypeLookupError, ConstantString) {
 }
 
 TEST(TypeLookupError, FormatString) {
-  auto error = TypeLookupError("%d %d %d %d %d %d %d %d %d %d", 0, 1, 2, 3, 4,
-                               5, 6, 7, 8, 9, 10);
+  auto error = TYPE_LOOKUP_ERROR_FMT("%d %d %d %d %d %d %d %d %d %d", 0, 1, 2,
+                                     3, 4, 5, 6, 7, 8, 9);
   char *str = error.copyErrorString();
   ASSERT_STREQ(str, "0 1 2 3 4 5 6 7 8 9");
   error.freeErrorString(str);
@@ -35,8 +35,8 @@ TEST(TypeLookupError, Copying) {
   std::vector<TypeLookupError> vec;
 
   {
-    auto originalError = TypeLookupError("%d %d %d %d %d %d %d %d %d %d", 0, 1,
-                                         2, 3, 4, 5, 6, 7, 8, 9, 10);
+    auto originalError = TYPE_LOOKUP_ERROR_FMT("%d %d %d %d %d %d %d %d %d %d",
+                                               0, 1, 2, 3, 4, 5, 6, 7, 8, 9);
     for (int i = 0; i < 5; i++)
       vec.push_back(originalError);
   }