From d9f786fd13fe03256ef0f2983ecd379d0e7e8c93 Mon Sep 17 00:00:00 2001 From: Volodymyr Sapsai Date: Fri, 2 Aug 2024 15:15:47 -0700 Subject: [PATCH] Reland "[Modules] Fix using `va_list` with modules and a precompiled header. (#100837)" Fix the false warning > incompatible pointer types passing 'va_list' (aka '__builtin_va_list') to parameter of type 'struct __va_list_tag *' [-Wincompatible-pointer-types] The warning is wrong because both in the function declaration and at the call site we are using `va_list`. When we call `ASTContext::getBuiltinVaListDecl` at a specific moment, we end up re-entering this function which causes creating 2 instances of `BuiltinVaListDecl` and 2 instances of `VaListTagDecl` but the stored instances are unrelated to each other because of the call sequence like getBuiltinVaListDecl CreateX86_64ABIBuiltinVaListDecl VaListTagDecl = TagA indirectly call getBuiltinVaListDecl CreateX86_64ABIBuiltinVaListDecl VaListTagDecl = TagB BuiltinVaListDecl = ListB BuiltinVaListDecl = ListA Now we have `BuiltinVaListDecl == ListA` and `VaListTagDecl == TagB`. For x86_64 '__builtin_va_list' and 'struct __va_list_tag *' are compatible because '__builtin_va_list' == '__va_list_tag[1]'. But because we have unrelated decls for VaListDecl and VaListTagDecl the types are considered incompatible as we are comparing type pointers. Fix the error by creating `BuiltinVaListDecl` before `ASTReader::InitializeSema`, so that during `ASTContext::getBuiltinVaListDecl` ASTReader doesn't try to de-serialize '__builtin_va_list' and to call `ASTContext::getBuiltinVaListDecl` again. Reland with the requirement to have x86 target to avoid errors like > error: unable to create target: 'No available targets are compatible with triple "x86_64-apple-darwin"' rdar://130947515 --- clang/lib/Sema/Sema.cpp | 7 +++ clang/test/Modules/builtin-vararg.c | 84 +++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 clang/test/Modules/builtin-vararg.c diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index 2e989f0ba6fe45..19d8692ee64849 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -310,6 +310,13 @@ void Sema::addImplicitTypedef(StringRef Name, QualType T) { } void Sema::Initialize() { + // Create BuiltinVaListDecl *before* ExternalSemaSource::InitializeSema(this) + // because during initialization ASTReader can emit globals that require + // name mangling. And the name mangling uses BuiltinVaListDecl. + if (Context.getTargetInfo().hasBuiltinMSVaList()) + (void)Context.getBuiltinMSVaListDecl(); + (void)Context.getBuiltinVaListDecl(); + if (SemaConsumer *SC = dyn_cast(&Consumer)) SC->InitializeSema(*this); diff --git a/clang/test/Modules/builtin-vararg.c b/clang/test/Modules/builtin-vararg.c new file mode 100644 index 00000000000000..79e4ddfe0c510d --- /dev/null +++ b/clang/test/Modules/builtin-vararg.c @@ -0,0 +1,84 @@ +// Check how builtins using varargs behave with the modules. + +// REQUIRES: x86-registered-target +// RUN: rm -rf %t +// RUN: split-file %s %t + +// RUN: %clang_cc1 -triple x86_64-apple-darwin \ +// RUN: -fmodules -fno-implicit-modules -fbuiltin-headers-in-system-modules \ +// RUN: -emit-module -fmodule-name=DeclareVarargs \ +// RUN: -x c %t/include/module.modulemap -o %t/DeclareVarargs.pcm \ +// RUN: -fmodule-map-file=%t/resource_dir/module.modulemap -isystem %t/resource_dir +// RUN: %clang_cc1 -triple x86_64-apple-darwin \ +// RUN: -fmodules -fno-implicit-modules -fbuiltin-headers-in-system-modules \ +// RUN: -emit-pch -fmodule-name=Prefix \ +// RUN: -x c-header %t/prefix.pch -o %t/prefix.pch.gch \ +// RUN: -fmodule-map-file=%t/include/module.modulemap -fmodule-file=DeclareVarargs=%t/DeclareVarargs.pcm \ +// RUN: -I %t/include +// RUN: %clang_cc1 -triple x86_64-apple-darwin \ +// RUN: -fmodules -fno-implicit-modules -fbuiltin-headers-in-system-modules \ +// RUN: -emit-obj -fmodule-name=test \ +// RUN: -x c %t/test.c -o %t/test.o \ +// RUN: -Werror=incompatible-pointer-types \ +// RUN: -fmodule-file=%t/DeclareVarargs.pcm -include-pch %t/prefix.pch.gch \ +// RUN: -I %t/include + +//--- include/declare-varargs.h +#ifndef DECLARE_VARARGS_H +#define DECLARE_VARARGS_H + +#include + +int vprintf(const char *format, va_list args); + +// 1. initializeBuiltins 'acos' causes its deserialization and deserialization +// of 'implementation_of_builtin'. Because this is done before Sema initialization, +// 'implementation_of_builtin' DeclID is added to PreloadedDeclIDs. +#undef acos +#define acos(__x) implementation_of_builtin(__x) + +// 2. Because of 'static' the function isn't added to EagerlyDeserializedDecls +// and not deserialized in `ASTReader::StartTranslationUnit` before `ASTReader::InitializeSema`. +// 3. Because of '__overloadable__' attribute the function requires name mangling during deserialization. +// And the name mangling requires '__builtin_va_list' decl. +// Because the function is added to PreloadedDeclIDs, the deserialization happens in `ASTReader::InitializeSema`. +static int __attribute__((__overloadable__)) implementation_of_builtin(int x) { + return x; +} + +#endif // DECLARE_VARARGS_H + +//--- include/module.modulemap +module DeclareVarargs { + header "declare-varargs.h" + export * +} + +//--- resource_dir/stdarg.h +#ifndef STDARG_H +#define STDARG_H + +typedef __builtin_va_list va_list; +#define va_start(ap, param) __builtin_va_start(ap, param) +#define va_end(ap) __builtin_va_end(ap) + +#endif // STDARG_H + +//--- resource_dir/module.modulemap +module _Builtin_stdarg { + header "stdarg.h" + export * +} + +//--- prefix.pch +#include + +//--- test.c +#include + +void test(const char *format, ...) { + va_list argParams; + va_start(argParams, format); + vprintf(format, argParams); + va_end(argParams); +}