[libcxx-commits] [libcxxabi] [libcxxabi] Don't enable -fvisibility=hidden on Windows (PR #207943)

[Martin Storsjö via libcxx-commits]

mstorsjo wrote:

> LGTM, but I'm a bit confused how this works with libc++. I don't see any similar guards there.

That's a very good point.

First I thought that `_LibcxxExplicitABIAnnotations.push` which touches visibility and similar things could be involved, but it turns out to be even much more subtle than that.

A trivial example like this doesn't trigger the issue:
```c++
void __declspec(dllexport) func(void) {
}
```
```console
$ clang -target x86_64-windows-gnu -c export.cpp -fvisibility=hidden
$ clang -target x86_64-windows-gnu -S -emit-llvm export.cpp -fvisibility=hidden -o -
[...]

; Function Attrs: mustprogress noinline nounwind optnone
define dso_local dllexport void @_Z4funcv() #0 {
entry:
  ret void
}

[...]
```

So for such a case, Clang does know and set the right nonconflicting attributes - dllexport only, nothing else.

I took the source file that triggered the issue and fed it to cvise to reduce it, and came up with this:
```c++
namespace __cxxabiv1 {
struct __attribute__((dllexport)) __fundamental_type_info {
  virtual ~__fundamental_type_info();
};
__fundamental_type_info::~__fundamental_type_info() {}
} // namespace __cxxabiv1
```
```console
$ clang -target x86_64-windows-gnu -c repro.cpp -fvisibility=hidden
dllexport GlobalValue must have default or protected visibility
ptr @_ZTIv
dllexport GlobalValue must have default or protected visibility
ptr @_ZTSv
dllexport GlobalValue must have default or protected visibility
ptr @_ZTIPv
[...]
fatal error: error in backend: Broken module found, compilation
      aborted!
```

So apparently there's some very custom code generation triggered around the class `__cxxabiv1::__fundamental_type_info`, in `clang/lib/CodeGen/ItaniumCXXABI.cpp`, which ends up with inconsistent attributes on the generated symbols here.

So this is even clearer for a bug report to Clang itself for now. The best workaround probably also is roughly the same, but we probably should tweak the comments a bit...

https://github.com/llvm/llvm-project/pull/207943