Merge pull request #74494 from hyp/eng/android-memcpy-nonnull-me-timbers

[cxx-interop] Builtin functions should ignore return type nullability…

Author: hyp

lib/ClangImporter/ImportType.cpp

@@ -2223,6 +2223,32 @@ ImportedType ClangImporter::Implementation::importFunctionReturnType(
   if (auto elaborated =
           dyn_cast<clang::ElaboratedType>(returnType))
     returnType = elaborated->desugar();
+  // In C interop mode, the return type of library builtin functions
+  // like 'memcpy' from headers like 'string.h' drops
+  // any nullability specifiers from their return type, and preserves it on the
+  // declared return type. Thus, in C mode the imported return type of such
+  // functions is always optional. However, in C++ interop mode, the return type
+  // of builtin functions can preseve the nullability specifiers on their return
+  // type, and thus the imported return type of such functions can be
+  // non-optional, if the type is annotated with
+  // `_Nonnull`. The difference between these two modes can break cross-module
+  // Swift serialization, as Swift will no longer be able to resolve an x-ref
+  // such as 'memcpy' from a Swift module that uses C interop, within a Swift
+  // context that uses C++ interop. In order to avoid the x-ref resolution
+  // failure, normalize the return type's nullability for builtin functions in
+  // C++ interop mode, to match the imported type in C interop mode.
+  auto builtinContext = clang::Builtin::Context();
+  if (SwiftContext.LangOpts.EnableCXXInterop && clangDecl->getBuiltinID() &&
+      !builtinContext.isTSBuiltin(clangDecl->getBuiltinID()) &&
+      builtinContext.isPredefinedLibFunction(
+          clangDecl->getBuiltinID()) &&
+      builtinContext.getHeaderName(clangDecl->getBuiltinID()) ==
+          StringRef("string.h")) {
+    if (const auto ART = dyn_cast<clang::AttributedType>(returnType)) {
+      if (ART->getImmediateNullability())
+        clang::AttributedType::stripOuterNullability(returnType);
+    }
+  }
 
   // Specialized templates need to match the args/result exactly (i.e.,
   // ptr -> ptr not ptr -> Optional<ptr>).

test/Interop/Cxx/function/Inputs/custom-string-builtins.h

@@ -0,0 +1,35 @@
+#pragma once
+
+#include <stddef.h>
+
+#define __attribute_pure__ __attribute__((__pure__))
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#ifdef __cplusplus
+    #define TEST_CONST_RETURN  const
+#else
+    #define TEST_CONST_RETURN
+#endif
+
+
+void* _Nonnull memcpy(void* _Nonnull, const void* _Nonnull, size_t);
+
+void* _Nonnull memcpy42(void* _Nonnull, const void* _Nonnull, size_t);
+
+void TEST_CONST_RETURN* _Nullable memchr(const void* _Nonnull __s, int __ch, size_t __n) __attribute_pure__;
+
+void* _Nonnull memmove(void* _Nonnull __dst, const void* _Nonnull __src, size_t __n);
+
+void* _Nonnull memset(void* _Nonnull __dst, int __ch, size_t __n);
+
+char TEST_CONST_RETURN* strrchr(const char* __s, int __ch) __attribute_pure__;
+
+char* _Nonnull strcpy(char* _Nonnull __dst, const char* _Nonnull __src);
+char* _Nonnull strcat(char* _Nonnull __dst, const char* _Nonnull __src);
+
+#ifdef __cplusplus
+}
+#endif

test/Interop/Cxx/function/Inputs/module.modulemap

@@ -2,3 +2,8 @@ module DefaultArguments {
   header "default-arguments.h"
   export *
 }
+
+module CustomStringBuiltins {
+  header "custom-string-builtins.h"
+  export *
+}

test/Interop/Cxx/function/builtin-nullability-return-type-typechecker.swift

@@ -0,0 +1,20 @@
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-nosource) -typecheck -verify -verify-ignore-unknown -I %S/Inputs -cxx-interoperability-mode=default -Xcc -D_CRT_SECURE_NO_WARNINGS %s
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-nosource) -typecheck -verify -verify-ignore-unknown -I %S/Inputs -Xcc -D_CRT_SECURE_NO_WARNINGS %s
+
+import CustomStringBuiltins
+
+public func testMemcpyOptionalReturn(p: UnsafeMutableRawPointer, e: UnsafeRawPointer, c: UnsafePointer<CChar>, pc: UnsafeMutablePointer<CChar>) {
+  // This 'memcpy' is a builtin and is always an optional, regardless of _Nonnull.
+  let x = CustomStringBuiltins.memcpy(p, e, 1)!
+
+  // Not a builtin, _Nonnull makes it a non-optional.
+  let y = CustomStringBuiltins.memcpy42(p, e, 1)! // expected-error {{cannot force unwrap value of non-optional type 'UnsafeMutableRawPointer'}}
+
+  // other builtins from 'string.h'
+  let _ = CustomStringBuiltins.memchr(e, 42, 1)!
+  let _ = CustomStringBuiltins.memmove(p, e, 42)!
+  let _ = CustomStringBuiltins.memset(p, 1, 42)!
+  let _ = CustomStringBuiltins.strrchr(c, 0)!
+  let _ = CustomStringBuiltins.strcpy(pc, c)!
+  let _ = CustomStringBuiltins.strcat(pc, c)!
+}