[clang] Reland "[Modules] Fix using `va_list` with modules and a precompiled header. (#101752)" (PR #101762)
Volodymyr Sapsai via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 2 15:13:40 PDT 2024
https://github.com/vsapsai created https://github.com/llvm/llvm-project/pull/101762
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
>From 54458ad7d84ea7e59f974b1a600dee46456117b2 Mon Sep 17 00:00:00 2001
From: Volodymyr Sapsai <vsapsai at apple.com>
Date: Fri, 2 Aug 2024 19:08:04 -0300
Subject: [PATCH] Reland "[Modules] Fix using `va_list` with modules and a
precompiled header. (#101752)"
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 2e989f0ba6fe4..19d8692ee6484 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<SemaConsumer>(&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 0000000000000..79e4ddfe0c510
--- /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 <stdarg.h>
+
+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 <declare-varargs.h>
+
+//--- test.c
+#include <declare-varargs.h>
+
+void test(const char *format, ...) {
+ va_list argParams;
+ va_start(argParams, format);
+ vprintf(format, argParams);
+ va_end(argParams);
+}
More information about the cfe-commits
mailing list