[cxx-interop] C library builtin functions from 'string.h' should ignore return type nullability to be compatible with C interop

In C interop mode, the return type of 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.

This fixed an issue when using 'memcpy' from the Android NDK in a x-module context, like
between Foundation (that inlines it) and another user module.

Author: hyp

lib/ClangImporter/ImportType.cpp

@@ -2223,6 +2223,31 @@ 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.
+  if (SwiftContext.LangOpts.EnableCXXInterop && clangDecl->getBuiltinID() &&
+      !clang::Builtin::Context().isTSBuiltin(clangDecl->getBuiltinID()) &&
+      clang::Builtin::Context().isPredefinedLibFunction(
+          clangDecl->getBuiltinID()) &&
+      clang::Builtin::Context().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,27 @@
+#pragma once
+
+#include <stddef.h>
+
+#define __attribute_pure__ __attribute__((__pure__))
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+void* _Nonnull memcpy(void* _Nonnull, const void* _Nonnull, size_t);
+
+void* _Nonnull memcpy42(void* _Nonnull, const void* _Nonnull, size_t);
+
+void* _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* _Nullable strrchr(const char* _Nonnull __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 %s
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-nosource) -typecheck -verify -verify-ignore-unknown -I %S/Inputs %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)!
+}