Skip to content

Commit

Permalink
Reland "[Modules] Fix using va_list with modules and a precompiled …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
vsapsai authored Aug 2, 2024
1 parent 8cf8565 commit d9f786f
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 0 deletions.
7 changes: 7 additions & 0 deletions clang/lib/Sema/Sema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
84 changes: 84 additions & 0 deletions clang/test/Modules/builtin-vararg.c
Original file line number Diff line number Diff line change
@@ -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);
}

0 comments on commit d9f786f

Please sign in to comment.