[c-interop] Ban non-ASCII identifiers in @_extern(c) conservatively

Author: kateinoigakukun

include/swift/AST/Attr.h

@@ -2368,6 +2368,10 @@ class ExternAttr : public DeclAttribute {
     return static_cast<ExternKind>(Bits.ExternAttr.kind);
   }
 
+  /// Returns the C name of the given declaration.
+  /// \p forDecl is the func decl that the attribute belongs to.
+  StringRef getCName(const FuncDecl *forDecl) const;
+
   /// Find an ExternAttr with the given kind in the given DeclAttributes.
   static ExternAttr *find(DeclAttributes &attrs, ExternKind kind);
 

include/swift/AST/DiagnosticsSema.def

@@ -1878,6 +1878,8 @@ ERROR(extern_empty_c_name,none,
       "expected non-empty C name in @_extern attribute", ())
 ERROR(extern_only_non_other_attr,none,
       "@_extern attribute cannot be applied to an '@%0' declaration", (StringRef))
+ERROR(extern_c_invalid_name, none,
+      "@_extern attribute has invalid C name '%0'", (StringRef))
 ERROR(c_func_variadic, none,
       "cannot declare variadic argument %0 in %kind1",
       (DeclName, const ValueDecl *))

lib/AST/Attr.cpp

@@ -2651,6 +2651,14 @@ bool MacroRoleAttr::hasNameKind(MacroIntroducedDeclNameKind kind) const {
   }) != getNames().end();
 }
 
+StringRef ExternAttr::getCName(const FuncDecl *D) const {
+  if (auto cName = this->Name)
+    return cName.value();
+  // If no name was specified, fall back on the Swift base name without mangling.
+  // Base name is always available and non-empty for FuncDecl.
+  return D->getBaseIdentifier().str();
+}
+
 ExternAttr *ExternAttr::find(DeclAttributes &attrs, ExternKind kind) {
   for (DeclAttribute *attr : attrs) {
     if (auto *externAttr = dyn_cast<ExternAttr>(attr)) {

lib/SIL/IR/SILDeclRef.cpp

@@ -1149,12 +1149,8 @@ std::string SILDeclRef::mangle(ManglingKind MKind) const {
       }
 
     if (auto *ExternA = ExternAttr::find(getDecl()->getAttrs(), ExternKind::C)) {
-      if (auto cName = ExternA->Name)
-        return cName->str();
-      // If no name was specified, fall back on the Swift base name without mangling.
       assert(isa<FuncDecl>(getDecl()) && "non-FuncDecl with @_extern should be rejected by typechecker");
-      // Base name is always available and non-empty for FuncDecl.
-      return getDecl()->getBaseIdentifier().str().str();
+      return ExternA->getCName(cast<FuncDecl>(getDecl())).str();
     }
 
     // Use a given cdecl name for native-to-foreign thunks.

lib/Sema/TypeCheckAttr.cpp

@@ -2131,9 +2131,14 @@ void AttributeChecker::visitExternAttr(ExternAttr *attr) {
 
   // C name must not be empty.
   if (attr->getExternKind() == ExternKind::C) {
-    if (auto cName = attr->Name) {
-      if (cName->empty())
-        diagnose(attr->getLocation(), diag::extern_empty_c_name);
+    StringRef cName = attr->getCName(FD);
+    if (cName.empty()) {
+      diagnose(attr->getLocation(), diag::extern_empty_c_name);
+    } else if (!clang::isValidAsciiIdentifier(cName)) {
+      // Conservatively ban non ASCII identifiers. The C standard allows
+      // Universal Character Names in identifiers, but clang doesn't provide
+      // an easy way to validate them, so we don't allow them for now.
+      diagnose(attr->getLocation(), diag::extern_c_invalid_name, cName);
     }
 
     // Ensure the decl has C compatible interface. Otherwise it produces diagnostics.

test/IRGen/extern_c.swift

@@ -1,28 +1,10 @@
-// RUN: %empty-directory(%t)
-// RUN: split-file %s %t
-// RUN: %target-swift-frontend -emit-ir %t/extern_c.swift -I%t | %FileCheck %s
-
-//--- c_types.h
-struct c_struct {
-  int field;
-};
-
-//--- module.modulemap
-module c_types {
-  header "c_types.h"
-}
-
-//--- extern_c.swift
-import c_types
+// RUN: %target-swift-frontend -emit-ir %s | %FileCheck %s
 
 func test() {
   // CHECK: call void @explicit_extern_c()
   explicit_extern_c()
   // CHECK: call void @implicit_extern_c()
   implicit_extern_c()
-
-  // CHECK: call i32 @"+"({{.*}})
-  _ = c_struct(field: 1) + c_struct(field: 2)
 }
 
 test()
@@ -32,6 +14,3 @@ test()
 
 // CHECK: declare void @implicit_extern_c()
 @_extern(c) func implicit_extern_c()
-
-// CHECK: declare i32 @"+"({{.*}})
-@_extern(c) func +(a: c_struct, b: c_struct) -> c_struct

test/attr/attr_extern.swift

@@ -43,9 +43,18 @@ func f12InvalidLang() // expected-error {{expected '{' in body of function decla
 @_extern(c, "valid")
 func externCValid()
 
+@_extern(c, "_start_with_underscore")
+func underscoredValid()
+
 @_extern(c, "") // expected-error {{expected non-empty C name in @_extern attribute}}
 func emptyCName()
 
+@_extern(c, "0start_with_digit") // expected-error {{@_extern attribute has invalid C name '0start_with_digit'}}
+func digitPrefixed()
+
+@_extern(c) // expected-error {{@_extern attribute has invalid C name '+'}}
+func +(a: Int, b: Bool) -> Bool
+
 @_extern(c)
 func omitCName()