[clang-cl] Handle some pragma alloc_text corner cases handled by MSVC

[steplong]

Summary

MSVC's pragma alloc_text accepts a function that was redeclared in
a non extern-C context if the previous declaration was in an extern-C
context. i.e.

extern "C" { static void f(); }
static void f();

MSVC's pragma alloc_text also rejects non-functions.

[steplong]

This does make us accept, but MSVC doesn't:

static void f();
extern "C" {
  static void f();
}
#pragma alloc_text(c, f);
static void f() {}

MSVC and GCC fail with an error for conflicting declaration.
GCC: https://godbolt.org/z/Tavvx88E4
MSVC: https://godbolt.org/z/vj41TWdYh

[hans]

clang/lib/Sema/SemaAttr.cpp

811

Since the pragma only applies to functions, maybe we should error here if the decl is not a FunctionDecl?

816

Instead of walking the decls, would checking isExternC() on the getCanonicalDecl() be enough?

[steplong]

• Use getCanonicalDecl(). This does make us reject (MSVC rejects it already because of conflicting declarations):

static void f();
extern "C" { static void f(); }
static void f() {}

• Error if not function decl

I think we have to use isInExternCContext() to accept the following (MSVC accepts this):

extern "C" { static void f(); }
static void f() {}

[hans]

In D128649#3612626, @steplong wrote:

I think we have to use isInExternCContext() to accept the following (MSVC accepts this):

extern "C" { static void f(); }
static void f() {}

I'm probably missing something. Is the "static" important in this example? Since f has internal linkage, I don't see why a user would care if it's "extern C" or not, and also not in which section it would go (in this case it will not be emitted at all, because it's not used).

Isn't the question whether f is considered "extern C" in the end or not? I thought isExternC() checks that? Are you saying it would return false for f in your example?

clang/lib/Sema/SemaAttr.cpp

822

Since it's inside if (getLangOpts().CPlusPlus) { it will only error in C++ mode, but it should probably also error in C mode.

[steplong]

Isn't the question whether f is considered "extern C" in the end or not? I thought isExternC() checks that? Are you saying it would return false for f in your example?

Yup, isExternC() is returning false for that case because there's "static".

I'm probably missing something. Is the "static" important in this example? Since f has internal linkage, I don't see why a user would care if it's "extern C" or not, and also not in which section it would go (in this case it will not be emitted at all, because it's not used).

Hmm, good point. This is the disassembly I see for:

extern "C" { static void f(); }
static void f();
#pragma alloc_text(s, f)
static void f() {}

void bar() {
  f();
}

Disassembly of section .text$mn:
?bar@@YAXXZ:
       0:       fd 7b bf a9     stp     x29, x30, [sp, #-16]!
       4:       fd 03 00 91     mov     x29, sp
       8:       00 00 00 94     bl      #0 <?bar@@YAXXZ+0x8>
       c:       fd 7b c1 a8     ldp     x29, x30, [sp], #16
      10:       c0 03 5f d6     ret
Disassembly of section s:
s:
       0:       c0 03 5f d6     ret

[steplong]

• Error for c also

[hans]

In D128649#3612875, @steplong wrote:

Isn't the question whether f is considered "extern C" in the end or not? I thought isExternC() checks that? Are you saying it would return false for f in your example?

Yup, isExternC() is returning false for that case because there's "static".

I see. It seems there's already some code that gives special treatment to "static extern c" functions in CodeGenModule::MaybeHandleStaticInExternC(): https://github.com/llvm/llvm-project/blob/llvmorg-14.0.6/clang/lib/CodeGen/CodeGenModule.cpp#L4420

Okay, I think this is almost ready. I just had one more comment about the if-statements above.

clang/lib/Sema/SemaAttr.cpp

824

Instead of checking this in two places, how about doing something like:

FunctionDecl *FD = dyn_cast<FunctionDecl>(ND->getCanonicalDecl());
if (!FD) {
  // error
}

above right after the if (!ND) check? Then the C++ specific code just becomes

if (!FD->isInExternCContext()) {
  // error
}

[steplong]

• Rework logic for rejecting non-functions and non-extern C functions when C++

[hans]

lgtm

clang/lib/Sema/SemaAttr.cpp

for (auto &Function : Functions) {
     SourceLocation Loc;
     std::tie(II, Loc) = Function;
 
     DeclarationName DN(II);
     NamedDecl *ND = LookupSingleName(TUScope, DN, Loc, LookupOrdinaryName);
     if (!ND) {
       Diag(Loc, diag::err_undeclared_use) << II->getName();
       return;
     }
 
-    DeclContext *DC = ND->getDeclContext();
-    if (getLangOpts().CPlusPlus && !DC->isExternCContext()) {
+    if (getLangOpts().CPlusPlus) {
+      bool IsInExternCContext = false;
+      Decl *D = ND;
+      while (D != nullptr) {
+        if (auto *FD = dyn_cast<FunctionDecl>(D)) {
+          if (FD->isInExternCContext()) {
+            IsInExternCContext = true;
+            break;
+          }
+        }
+        D = D->getPreviousDecl();
+      }
+
+      if (!IsInExternCContext) {
         Diag(Loc, diag::err_pragma_alloc_text_c_linkage);
         return;
       }
+    }
 
     FunctionToSectionMap[II->getName()] = std::make_tuple(Section, Loc);
   }
 }
 
 void Sema::ActOnPragmaUnused(const Token &IdTok, Scope *curScope,
                              SourceLocation PragmaLoc) {
 

clang/test/Sema/pragma-ms-alloc-text.cpp

 #pragma alloc_text(c, foo5) // expected-error {{'#pragma alloc_text' is applicable only to functions with C linkage}}
 extern "C" void foo5() {}   // expected-error {{declaration of 'foo5' has a different language linkage}}
 
 extern "C" {
 static void foo6();
 #pragma alloc_text(c, foo6) // no-warning
 void foo6() {}
 }
+
+extern "C" {
+static void foo7();
+}
+static void foo7();
+#pragma alloc_text(c, foo7) // no-warning
+void foo7() {}