-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[C++20] [Modules] Undefined reference to std::allocator<char>::deallocate in Clang 19.0.0git when a function exported from a module returns a type template instantiation which uses std::allocator<char> #86893
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
Comments
@llvm/issue-subscribers-clang-modules Author: None (ashley-hawkins)
Sorry if the title is not very clear. Basically if in my main.cc I'm importing a module, and that module exports a function which returns std::string, and I call that function from main.cc, it will fail to compile on latest clang with a linker error that
```
/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/14.0.1/../../../../x86_64-linux-gnu/bin/ld: CMakeFiles/repro.dir/main.cc.o: in function `std::allocator_traits<std::allocator<char> >::deallocate(std::allocator<char>&, char*, unsigned long)':
/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/14.0.1/../../../../include/c++/14.0.1/bits/alloc_traits.h:513: undefined reference to `std::allocator<char>::deallocate(char*, unsigned long)'
clang++: error: linker command failed with exit code 1 (use -v to see invocation)
```
I tried to make a minimal reproducer but I'm not sure whether or not this is actually minimal, because I don't know if it could be reproduced without std::allocator or if it's specifically a problem with std::allocator. In module.cc you can change the return type of repro::repro to Compiler Explorer reproducer for 19.0.0git: https://godbolt.org/z/5GbczMEa7 As of writing this: Problematic version of Clang tested on Compiler Explorer: Problematic version of Clang that I originally found the problem on, from apt.llvm.org: Working version of Clang tested on Compiler Explorer: |
I suspect this is not a compiler's issue but a standard library issue. See my workaround and the comments in https://github.com/alibaba/async_simple/blob/d6f201192bb332fc9dabcfdf3fa29e85c83c7f45/third_party_module/stdmodules/string.cppm#L28-L42. And it works for https://godbolt.org/z/7Y3r7sM66. Also the problem disappear too if we're using libc++: https://godbolt.org/z/oPrY3Y8Es So I think this is a libstdc++ issue. I'll close this as not planned. Feel free to comment or reopen this. |
I did a git bisect, and observed that the issue only appears in 1ecbab5 and later. Is this change just revealing a problem with libstdc++? If it's the case, I will see if it needs to be reported to libstdc++'s bug tracker. |
Historically, clang has worked around issues with libstdc++ so that clang users are not forced to use libc++ (that might not be possible for ABI reasons). That being said, I'm not sure exactly what the issue is here. I see If I'm understanding the linked commit (1ecbab5) correctly, then I'm also surprised by that behavior. It seems like that commit turns something that literally says "always inline this" into "unless this only under these conditions". I would expect always_inline to be stronger than inline, not stronger within a module but weaker between modules. There is a reference to ABI requirements in that commit, but I don't see the issue here -- someone who marks a function always_inline isn't going for ABI stability as they change that function. Does this mean that people who want the 'old' behavior would have to say both inline and always_inline? |
This should not be relevant to clang. Since I can't see the symbols in libstdc++.so, which should be compiled from GCC. So I believe this may be a GCC defect. But,
Yeah, I've received multiple complaints about this too. So probably I need to do some workaround in clang even if I think it is a GCC issue.
My thought was, the |
Please could you file a Bugzilla for GCC/libstc++ (https://gcc.gnu.org/bugzilla/) in that case; there is an intent that the compilers and libraries can interoperate. |
The functions that generate those symbols only exist for It seems that modules changes this picture, and extern symbols are now needed for these always_inline functions. Compiling the explicit instantiation definitions as C++20 instead of C++98 causes the explicit instantiation definitions to include those functions, so they're in the .so That change might be OK for GCC 14 still, but backporting to GCC 13 would be complicated. |
David wrote:
N.B. GCC requires that anyway; So if the linked LLVM commit is supposed to "not export non-inline function bodies even if marked always_inline" then why does it change anything here? The affected functions are inline. Is there a bug where functions that are implicitly inline due to being defined in the body are somehow being treated as not inline because they don't have a redundant Edit: Oh, and those functions are also marked |
The commit was to fix #80949 which has a non-inline function marked But I don't see why that's caused this regression with libstdc++'s |
Here is a minimal reproducer without libstdc++ involved: https://godbolt.org/z/7eMbxoxY3 |
Yeah, this is the case. The linkage In another point of view, without modules, for |
I'm not sure what you mean by "inline the explicit instantiations". An inline function which has an explicit instantiation declaration is still eligible to be inlined. Especially if it's marked The compiler is not forced to use an explicit instantiation definition in some other TU, it's allowed to do an implicit instantiation and inline it. And I think for an |
And when modules aren't involved, Clang seems to agree with GCC there. The If they were not being inlined, and an extern symbol was being expected, you would never be able to link any code using libstdc++'s |
This is what I mean: https://godbolt.org/z/TK5Ye9z1P, both GCC and Clang don't generate the body for Then let's forget Then for |
We use it in libstdc++ specifically to deal with the case where a symbol is not part of the explicit instantiation definition in the .so e.g. for symbols that are new C++26 features, which are not exported from the .so yet. |
The GCC docs for Now if modules change the equation, we need to consider what changes. But I don't think we can just say "forget always_inline for a while" when that attribute is being used specifically to avoid the need for an extern symbol. The whole issue hinges on the use of that attribute. |
If Clang wants to make this change and thereby require ABI changes to libstdc++.so then I think either the change should be delayed until libstdc++ can be updated to deal with it, or Clang should be very clear that you can't use Clang 19 with libstdc++ and modules, because clang chose to change something that was previously guaranteed and relied on by libstdc++. |
Patch to provide those symbols in libstdc++: https://gcc.gnu.org/pipermail/gcc-patches/2024-April/649312.html |
(IMHO) we should make sure that we understand the behaviour we want in this case, before amending ABIs.. it seems that there could have been a misunderstanding about what is expected of |
e.g If we
does that work? |
I don't know, I don't know what effect emitting it in the BMI would have because I don't understand modules. But I wonder why the always_inline function isn't just inlined in the first place. Why is there an undefined reference to it? Here's the minimal reproducer again: https://godbolt.org/z/7eMbxoxY3 This gets an undefined reference to the always_inline function, because it's called from a function that's exported from the module. Why isn't it inlined? Why doesn't ChuanqiXu9 said:
But that's not the same. Yes, in that case of course there's an undefined ref to But if the same example is compiled with optimization enabled, even at -O1, then there's no undefined reference. Why does an always_inline function used by a function that is exported from a module not get inlined? I'm not expecting the always_inline function to be exported, but I do expect it to be inlined. Because that's what the attribute says to do. In fact, the code should not even compile if it's not inlined. Why is that code giving a linker error? Either the function should be inlined (and there's no undefined reference) or it should be a compiler error. That's what the attribute means, by definition. Inline it or die trying. The bug that the behaviour change was intended to fix (#80949) doesn't seem to be the same as this problem, that has an always_inline function (which is not an inline function) that is also supposed to be exported. I have no idea what that is meant to do. But that's not the problem here. The |
It seems to me that a more reasonable response to #80949 would have been "you can't export always_inline functions, that doesn't work". Instead the documented always_inline guarantee (which working code in the real world relies on) has been broken to support a novel use of the attribute with modules, something that has never been officially described as supported in any documentation. |
It seems that the underlying issue is not especially modules-related; instead it is that clang is treating the So first we need to decide what to do about this in general - we can then decide on whether always_inline bodies need to be in the BMI (probably only if they are exported, I suppose). |
There are some separated topics in the thread now. Let's try to separate them clear. always_inline changes ABII think that it is not strictly correct to make the behavior of the program (or the ABI) relies on On the practical side, given I've received many complaints, and even if libstdc++ fixes the issue in their side, the end users still need to update the libstdc++ to get the correct behavior, which is not very user friendly. So I've decided to use a new mechanism to make sure users won't meet this problem again. How should we do?Previously I thought I need to do something fancy about static local variables. But the ideas of forbidding non-inline always_inline function in module interfaces looks promising. I don't understand why export matter here. It should be good to export an always_inline inline function. (The first inline is a requirement to the optimizer and the second 'inline' means its linkage. So it should be well defined even if it looks redundant.) |
I disagree.
Adding Please leave it to library authors to worry about how they define their ABI instead of deciding that clang knows better.
You've broken a valid use case to support a crazy use case with local statics in functions with multiple definitions.
Because you should be able to inline the
Why? |
What I mean is, the ABI now depends on the attribute
Maybe we're not on the same page. I'm not saying
should be valid while
may not be valid. |
We could separate this into two issues:
The vendor extension
|
About the clang's behavior about
For what libstdc++ need to do:
For what clang will do:
|
Close by reverting 1ecbab5 |
…n if it is marked as always_inline" This reverts commit 1ecbab5. See the discussion in llvm#86893. The original commit receives too many complaints. Let's try to workaround the issue to give better user experiences.
…n if it is marked as always_inline" This reverts commit 1ecbab5. See the discussion in llvm#86893. The original commit receives too many complaints. Let's try to workaround the issue to give better user experiences.
Sorry if the title is not very clear. Basically if in my main.cc I'm importing a module, and that module exports a function which returns std::string, and I call that function from main.cc, it will fail to compile on latest clang with a linker error that
I tried to make a minimal reproducer but I'm not sure whether or not this is actually minimal, because I don't know if it could be reproduced without std::allocator or if it's specifically a problem with std::allocator. In module.cc you can change the return type of repro::repro to
std::string
to see that it also happens withstd::string
. It also happens withstd::vector<char>
, andstd::deque<char>
, and I guess any other standard container would have the same problem with an element type of char. I wasn't able to reproduce without using modules, which is why I labelled it as a modules problem.Compiler Explorer reproducer for 19.0.0git: https://godbolt.org/z/5GbczMEa7
Compiler Explorer example of inability to reproduce for 18.1.0: https://godbolt.org/z/EEajTa1nj (same code as in above I just changed the compiler to x86-64 Clang 18.1.0)
As of writing this:
Problematic version of Clang tested on Compiler Explorer:
Problematic version of Clang that I originally found the problem on, from apt.llvm.org:
Working version of Clang tested on Compiler Explorer:
The text was updated successfully, but these errors were encountered: