-
Notifications
You must be signed in to change notification settings - Fork 13.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
std::any
is still broken with visbility=hidden
and libc++
when used cross DSO boundaries
#133725
Comments
If you switch to Without it, you end up generating two copies of the RTTI for |
(feel free to comment or reopen if you don't think my explanation makes sense) |
But why is this needed for libc++ when it isn't required for libstdc++? Note that wrapped int is completely inline, and as such should not need to be exported at all - this goes against everything I learned about symbol visibility so far. |
Maybe to explain this a bit more on why I still consider this to be a defect: Some types are not under our control, think e.g. templates types from some thirdparty libraries, those are usually not exported and thus std::any would break with them like here: https://godbolt.org/z/oWqqjEbM1 With libstdc++ it works fine: https://godbolt.org/z/r8sMbavEc |
@milianw We should also mention, that we don't have this problem with Windows and Visual Studio Compiler and GCC with glibc |
@milianw @StefanoD With libstdc++ this most likely happens to work out because the function taking the typeinfo doesn't get inlined and libstdc++ gives all functions default visibility. Because of that it's always the same function that gets the typeinfo. As soon as there is more than one function taking the actual typeinfo you're out of luck. We could work around the issue by having a single function in the library with default visibility and |
The "super hacky" thing you describe sounds ideal to me, as this works on all other platforms and only libc++ suffers from this. Is there at least an arcane way for std::any or similar to detect that one tries to use it with a non-exported type, to then warn/error out? Because right now everything looks fine at compile time only to then fail fatally at runtime (and only on macOS for us, as that's the only platform we target with clang and libc++). I also would like to have confirmation: Are you really saying one should export fully inline types? That goes against what I learned, one does not want to do that normally. |
@philnik777 You break essentially |
It's not. This only happens to work out right now with libstdc++. Not annotating the types properly is a latent bug in your code. Also, other people who annotate their types properly would also complain because we'd essentially be adding an entirely unnecessary optimization barrier.
Not that I'm aware of. Also note that this is only a problem across dylib boundaries, so most uses don't suffer from this problem.
Yes, you have to export the typeinfo of all types. I don't know what you've learned, but that's exactly what both libc++ and libstdc++ do to make using typeinfo for stdlib types across dylib boundaries work.
You can always wrap it into your own type that has the correct annotations. I also pointed out above that you can hack your way around third library code not annotating their code via pragmas. Another option is to not use
Why? You're saying you don't need the typeinfo across dylib boundaries via |
I found the source where I learned not to apply export to fully inline types, see: https://wiki.qt.io/Things_To_Look_Out_For_In_Reviews see Value Classes #6 "Never export a non-polymorphic class wholesale"
How so? Can you please expand on this? Are you saying I should do something like (pseudo code)
|
Given that the rationale talks specifically about MSVC I don't think it applies here. Also note that we're not asking you to export everything. We're only asking you to export the RTTI.
Yes, exactly. It would be something like
You can change |
Well, when writing cross platform code, we must find a solution that works everywhere. As such, I cannot ignore MSVC. It sounds as-if we would need separate export macros then, to pleasen the different platforms which makes me quite sad. If only the platforms could work a bit more similar to simplify things dream
This is the first time I hear about the distinction here, thanks! For others who are equally unaware: https://releases.llvm.org/20.1.0/tools/clang/docs/AttributeReference.html#type-visibility Sadly, this seems to be a clang specific feature, I cannot find something similar for GCC. And there is also apparently no flag to configure it globally while compiling. If that would exist, one would ideally be able to do
And get a result that works in a similar way to other platforms, right? Can we maybe repurpose this bug report accordingly then, to kindly ask for the addition for this global The "custom pragma" approach you are giving doesn't sound scalable - we would have to do that everywhere we include the thirdparty code after all, and that is extremely errorprone, no? |
Well, sadly we don't live in that dream world. In libc++ we currently have nine different visibility macros for all the platform specifics.
You're welcome. We should probably have been more explicit about this. Louis and I have had long conversations about the visibility attributes in Clang and are therefore quite deep in the topic, so it didn't really occur to me at first that it may not be obvious that RTTI has its own visibility, separate from other symbols. FYI this is also the case for vtables, which should (almost?) always have the same visibility as RTTI.
I agree that it's sad. Feel free to comment on https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113958.
Yes, that sounds like a reasonable solution. That's probably what a lot of people actually want in the end.
I think it would be better to open a separate issue with a link to this one.
Actually, never mind. It looks like Clang doesn't support |
Depending on the platform you're seeing this on, this could also be due to how typeinfos are compared. I think libstdc++ performs a string comparison of the typeinfo name on some platforms where we instead use pointer equality (of the RTTI pointer). When using string comparison, types will compare equal even when the RTTIs have messed up visibility. On the other hand, this will also result in genuinely different types in different dylibs that happen to share the same name (which can easily happen at global scope) to compare RTTI-equal. That's equally, if not more, broken. So I fear there is no perfect answer. If you decide to play with visibility, you're out of the purview of ISO C++ and you can't expect perfect portability. You'll need some amount of platform specific stuff no matter what you do. |
We saw it working on windows with msvc and on linux with gcc+libstdc++. Only macOS/clang failed us.
Isn't that an ODR violation, and one reason why we use namespaces and file local linkage etc?
Yes, that is expected, and has been the case for the past 20 years or so already that I've been using export macros, in the form that eventually got quasi-standardized via https://cmake.org/cmake/help/latest/module/GenerateExportHeader.html - but note how that is completely unaware of your clang specific type visibility macros. So it's not just "me" who was blissfully unaware of these traps, but I bet a ton of C++/CMake developers and basically the whole KDE/Qt ecosystem at large. |
The ISO Standard doesn't have a notion of IMO the key takeaway here is that if you're going to try controlling your visibility, you have to be ready to understand how visibility works and part of that means understanding how RTTI comparison works on your platform and how it's affected by visibility. I'm not saying that's easy, it's a total mess, but it's the world we have right now. |
I was under the misconception of having understood how visibility works, but this issue here clearly shows that's not the case :) So the question now becomes, where can we learn more about your platform, i.e. clang/libc++ - I only found e.g. https://releases.llvm.org/20.1.0/tools/clang/docs/AttributeReference.html#type-visibility but that is very scarce on details and definitely doesn't talk about RTTI comparison. Do you know any resources that would us to better understand all the required nitty gritty details to properly handle this? Again, now how this seems to be broken for everyone that relies on standard CMake functionality to reduce the visibility of symbols, which is in many places advertised as a good thing, mostly to ensure linux builds behave more similar to Windows ones e.g. |
I think this comment is useful to understand a bit more about RTTI and visibility: https://github.com/llvm/llvm-project/blob/main/libcxx/include/typeinfo#L120 However, I am not aware of any official documentation that does a holistic coverage of the whole subject, sorry. For most of what I know about this topic, I don't even know why I know it anymore, I just kind of learned it as I went and with trial and error. Whenever we do something funky with visibility in libc++ we try to leave decent documentation behind, but that's far from "teaching material" level. |
Thank you Louis, this is interesting and a good start. For me as someone who wants to "understand how RTTI comparison works on [my] platform", more exhaustive material would be crucial - esp. targeted at library developers. After all, we seem to know what to do for libstdc++ and msvc to a degree that it works, but I fear we'll now have to sift through the code to understand what to do for libc++ specifically, and what best practices there are. I am well aware of the load of the people involved in these projects, and the extra burden of documentation, so please just take this as a friendly request for considering to convince someone to spent some time on documenting what to do for libc++ as a platform wrt. visibility. As a motivation: Everyone using standard CMake facilities and hidden visibility is doing it wrong apparently, so quite the chance to apply https://xkcd.com/386/ ;-) Cheers and keep up the good work |
This is related to #48743 which seems to have fixed this issue for
libstdc++
, but apparently there are remaining problems when usinglibc++
:libc++ with clang-20.1.0: https://godbolt.org/z/4TW4rMYoe note that the typeid name is the same, but the hash differs already:
this then leads to:
If we don't enforce
libc++
and thus default tolibstdc++
then everything works:https://godbolt.org/z/bsGvfzWsh
The text was updated successfully, but these errors were encountered: