This ensures that the mangled type names match between C and C++,
which is significant when using -fsanitize=cfi-icall. Ideally we
wouldn't have created this namespace at all, but it's now part of
the ABI (e.g. in mangled names), so we can't change it.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
This results in the DWARF type info containing DW_TAG_namespace for C, which breaks DTrace's CTF (as the name implies, it is for C, not C++), as used during the build of FreeBSD. This does not seem correct to me. If your mangling for va_list is broken then you should special case that in the CFI mangler as an ABI quirk, IMO, not change the DWARF for C to contain C++ things.
Or you can still do it in the frontend, just make sure that the DWARF emitted by the frontend doesn't have the DINamespace wrapper when not compiling C++.
I also note that 32-bit Arm uses the same namespacing, and has CFI-icall support (at least, it's enabled, I don't know if it's actually expected to work) but did not have this change made to it (though probably best not to do so until the DWARF issue is resolved)
This also leaks out to users via __builtin_dump_struct (https://godbolt.org/z/vx3rjdPdq), and of course the DWARF having the namespace in it will result in users debugging plain C seeing C++ namespaces.
ASTDiagnostic's Desugar happens to special-case va_list so it doesn't get desugared from the result of the typedef, and thus the struct std::__va_list doesn't leak out that way.
I also wouldn't be surprised if some consumers of Clang's AST get confused by a namespace appearing for C output; e.g. I could imagine IDEs using clangd displaying very confusing information to users writing C code.
Agreed that this should probably be done in the mangler, I'll try to take a look next week.