[Clang][ARM] __va_list declaration is not saved in ASTContext causing compilation error or crash

[iid_iunknown]

Summary

When the code is compiled for arm32 and the builtin __va_list declaration is created by CreateAAPCSABIBuiltinVaListDecl, the declaration is not saved in the ASTContext which may lead to a compilation error or crash.

Minimal reproducer I was able to find:
header.h

#include <stdarg.h>
typedef va_list va_list_1;

test.cpp

typedef __builtin_va_list va_list_2;
void foo(const char* format, ...) { va_list args; va_start( args, format ); }

Steps to reproduce:

clang -x c++-header --target=armv7l-linux-eabihf header.h
clang -c -include header.h --target=armv7l-linux-eabihf test.cpp

Compilation error:

error: non-const lvalue reference to type '__builtin_va_list'
      cannot bind to a value of unrelated type 'va_list' (aka '__builtin_va_list')

Compiling the same code as a C source leads to a crash:

clang --target=armv7l-linux-eabihf header.h
clang -c -x c -include header.h --target=armv7l-linux-eabihf test.cpp

[rsmith]

Thanks, the change itself LGTM, but please add your testcase, perhaps to test/PCH/__va_list_tag.c

[iid_iunknown]

Test case added.

[iid_iunknown]

Thank you for your remark, Richard.
I added the test. Could you check if the patch is good now, please?

[rsmith]

test/PCH/arm-__va_list_tag.c

1–7

Can we remove the triple from this test, and instead test it on the default target for each bot? That should give us broader coverage, and let us find out if the same bug exists for other targets too.

[iid_iunknown]

The test enabled for all targets.

[iid_iunknown]

test/PCH/arm-__va_list_tag.c

1–7

Yes, that's a nice idea.
I've updated the test.
Thanks!

[rsmith]

LGTM, do you need someone to commit this for you?

[iid_iunknown]

I have commit access.
Thank you for your help with the patch!

lib/AST/ASTContext.cpp

FieldDecl *Field = FieldDecl::Create(const_cast<ASTContext &>(*Context),
                                        /*BitWidth=*/nullptr,
                                        /*Mutable=*/false,
                                        ICIS_NoInit);
   Field->setAccess(AS_public);
   VaListDecl->addDecl(Field);
 
   // };
   VaListDecl->completeDefinition();
+  Context->VaListTagDecl = VaListDecl;
 
   // typedef struct __va_list __builtin_va_list;
   QualType T = Context->getRecordType(VaListDecl);
   return Context->buildImplicitTypedef(T, "__builtin_va_list");
 }
 
 static TypedefDecl *
 CreateSystemZBuiltinVaListDecl(const ASTContext *Context) {

test/PCH/Inputs/arm-__va_list_tag.h

+// Header for PCH test arm-__va_list_tag.c
+
+#include <stdarg.h>
+typedef va_list va_list_1;

test/PCH/arm-__va_list_tag.c

+// REQUIRES: arm-registered-target
+
+// This test checks the patch for the compilation error / crash described in the D18557.
+
+// Test as a C source
+// RUN: %clang_cc1 -triple=armv7-linux-gnueabihf -emit-pch -x c-header -o %t %S/Inputs/arm-__va_list_tag.h
+// RUN: %clang_cc1 -triple=armv7-linux-gnueabihf -fsyntax-only -include-pch %t %s
+
+// Test as a C++ source
+// RUN: %clang_cc1 -triple=armv7-linux-gnueabihf -emit-pch -x c++-header -o %t %S/Inputs/arm-__va_list_tag.h
+// RUN: %clang_cc1 -triple=armv7-linux-gnueabihf -x c++ -fsyntax-only -include-pch %t %s
+
+// expected-no-diagnostics
+
+typedef __builtin_va_list va_list_2;
+void test(const char* format, ...) { va_list args; va_start( args, format ); }