Lvalue reference types appear in the signature of __builtin_va_start when compiled as C for some targets

[tahonermann]

Bugzilla Link	37122
Version	6.0
OS	Windows NT
CC	@efriedma-quic,@synopsys-sig-compiler-frontends

Extended Description

It seems that the signature of __builtin_va_start contains a C++ lvalue reference type when targeting Windows, even when compiling as C:

$ cat t.c void f(int n, ...) { builtin_va_list v; builtin_va_start(v, n); }

$ clang --version clang version 6.0.0 (tags/RELEASE_600/final) Target: x86_64-unknown-linux-gnu ...

$ clang -c -Xclang -ast-dump -target x86_64-pc-windows-gnu t.c ... -FunctionDecl 0x4c89578 <col:3> col:3 implicit used __builtin_va_start 'void (__builtin_va_list &, ...)' extern |-ParmVarDecl 0x4c89618 <<invalid sloc>> <invalid sloc> '__builtin_va_list &' -NoThrowAttr 0x4c89680 Implicit

Note the presence of '__builtin_va_list &' as a parameter type.

The lvalue reference is a surprise for AST consumers that do not expect to encounter C++ reference types in code compiled as C.

[tahonermann]

We can't change the signature of __builtin_va_start; a bunch of existing code depends on the current signature.

We clearly can't require source code changes. I was thinking more from the code gen perspective (which I would expect to be unaffected). But yes, this would mean adding an implicit operator& in the AST. That's ugly.

[efriedma-quic]

We can't change the signature of __builtin_va_start; a bunch of existing code depends on the current signature.

[tahonermann]

The relevant code is DecodeTypeFromStr in ASTContext.cpp; there's a nice comment there explaining why this is target-dependent.

Thank you. The comment is a little imprecise since it mentions behavior for x86 vs x86_64, but doesn't indicate corresponding target OS dependencies. I did some more testing and I now see that the reference type appears for x86 on Linux as well. It looks like the comment is consistent with Linux.

Would it not make sense for the current 'char*&' case to be 'char**'?

[efriedma-quic]

The relevant code is DecodeTypeFromStr in ASTContext.cpp; there's a nice comment there explaining why this is target-dependent.

[tahonermann]

I don't see how it would make much of a difference in practice, though: the reference type is an accurate representation of the signature.

The difference for us is that we hit an assert in our own code that is present to ensure we aren't leaking C++ concepts into our C analysis.

[tahonermann]

The signature already differs based on target platform:

$ clang -c -Xclang -ast-dump -target x86_64-pc-linux-gnu t.c ... -FunctionDecl 0x5c13908 <col:3> col:3 implicit used __builtin_va_start 'void (struct __va_list_tag *, ...)' extern |-ParmVarDecl 0x5c139a8 <<invalid sloc>> <invalid sloc> 'struct __va_list_tag *' -NoThrowAttr 0x5c13a10 Implicit

I'm not familiar enough with the calling conventions to appreciate why a pointer is used in this case, but a "reference" is used on Windows.

The defined signature is:

include/clang/Basic/Builtins.def: .. 38 // A -> "reference" to builtin_va_list .. 426 BUILTIN(builtin_va_start, "vA.", "nt")

I presume that the signature translation for "A" is target dependent, but I haven't managed to identify the code that handles that yet.

[efriedma-quic]

We could change __builtin_va_start to use custom type-checking, and hide the signature, I guess? I don't see how it would make much of a difference in practice, though: the reference type is an accurate representation of the signature.