AST: Move __va_list tag back to std conditionally on AArch64.

In post-commit feedback on D104830 Jessica Clarke pointed out that
unconditionally adding __va_list to the std namespace caused namespace
debug info to be emitted in C, which is not only inappropriate but
turned out to confuse the dtrace tool. Therefore, move __va_list back
to std only in C++ so that the correct debug info is generated. We
also considered moving __va_list to the top level unconditionally
but this would contradict the specification and be visible to AST
matchers and such, so make it conditional on the language mode.

To avoid breaking name mangling for __va_list, teach the Itanium
name mangler to always mangle it as if it were in the std namespace
when targeting ARM architectures. This logic is not needed for the
Microsoft name mangler because Microsoft platforms define va_list as
a typedef of char *.

Depends on D116773

Differential Revision: https://reviews.llvm.org/D116774

Author: pcc

clang/lib/AST/ASTContext.cpp

@@ -8547,21 +8547,18 @@ static TypedefDecl *CreateVoidPtrBuiltinVaListDecl(const ASTContext *Context) {
 
 static TypedefDecl *
 CreateAArch64ABIBuiltinVaListDecl(const ASTContext *Context) {
+  // struct __va_list
   RecordDecl *VaListTagDecl = Context->buildImplicitRecord("__va_list");
-  // namespace std { struct __va_list {
-  // Note that we create the namespace even in C. This is intentional so that
-  // the type is consistent between C and C++, which is important in cases where
-  // the types need to match between translation units (e.g. with
-  // -fsanitize=cfi-icall). Ideally we wouldn't have created this namespace at
-  // all, but it's now part of the ABI (e.g. in mangled names), so we can't
-  // change it.
-  auto *NS = NamespaceDecl::Create(
-      const_cast<ASTContext &>(*Context), Context->getTranslationUnitDecl(),
-      /*Inline*/ false, SourceLocation(), SourceLocation(),
-      &Context->Idents.get("std"),
-      /*PrevDecl*/ nullptr);
-  NS->setImplicit();
-  VaListTagDecl->setDeclContext(NS);
+  if (Context->getLangOpts().CPlusPlus) {
+    // namespace std { struct __va_list {
+    auto *NS = NamespaceDecl::Create(
+        const_cast<ASTContext &>(*Context), Context->getTranslationUnitDecl(),
+        /*Inline*/ false, SourceLocation(), SourceLocation(),
+        &Context->Idents.get("std"),
+        /*PrevDecl*/ nullptr);
+    NS->setImplicit();
+    VaListTagDecl->setDeclContext(NS);
+  }
 
   VaListTagDecl->startDefinition();
 

clang/lib/AST/ItaniumMangle.cpp

@@ -71,6 +71,7 @@ class ItaniumMangleContextImpl : public ItaniumMangleContext {
   llvm::DenseMap<DiscriminatorKeyTy, unsigned> Discriminator;
   llvm::DenseMap<const NamedDecl*, unsigned> Uniquifier;
   const DiscriminatorOverrideTy DiscriminatorOverride = nullptr;
+  NamespaceDecl *StdNamespace = nullptr;
 
   bool NeedsUniqueInternalLinkageNames = false;
 
@@ -194,6 +195,8 @@ class ItaniumMangleContextImpl : public ItaniumMangleContext {
     return DiscriminatorOverride;
   }
 
+  NamespaceDecl *getStdNamespace();
+
   const DeclContext *getEffectiveDeclContext(const Decl *D);
   const DeclContext *getEffectiveParentContext(const DeclContext *DC) {
     return getEffectiveDeclContext(cast<Decl>(DC));
@@ -590,6 +593,18 @@ class CXXNameMangler {
 
 }
 
+NamespaceDecl *ItaniumMangleContextImpl::getStdNamespace() {
+  if (!StdNamespace) {
+    StdNamespace = NamespaceDecl::Create(
+        getASTContext(), getASTContext().getTranslationUnitDecl(),
+        /*Inline*/ false, SourceLocation(), SourceLocation(),
+        &getASTContext().Idents.get("std"),
+        /*PrevDecl*/ nullptr);
+    StdNamespace->setImplicit();
+  }
+  return StdNamespace;
+}
+
 /// Retrieve the declaration context that should be used when mangling the given
 /// declaration.
 const DeclContext *
@@ -614,6 +629,17 @@ ItaniumMangleContextImpl::getEffectiveDeclContext(const Decl *D) {
       return ContextParam->getDeclContext();
   }
 
+  // On ARM and AArch64, the va_list tag is always mangled as if in the std
+  // namespace. We do not represent va_list as actually being in the std
+  // namespace in C because this would result in incorrect debug info in C,
+  // among other things. It is important for both languages to have the same
+  // mangling in order for -fsanitize=cfi-icall to work.
+  if (D == getASTContext().getVaListTagDecl()) {
+    const llvm::Triple &T = getASTContext().getTargetInfo().getTriple();
+    if (T.isARM() || T.isThumb() || T.isAArch64())
+      return getStdNamespace();
+  }
+
   const DeclContext *DC = D->getDeclContext();
   if (isa<CapturedDecl>(DC) || isa<OMPDeclareReductionDecl>(DC) ||
       isa<OMPDeclareMapperDecl>(DC)) {