Hi everyone, I am getting a weird padding at the beginning of a std::pair. See the following example: #include <cstdio> #include <utility> int main() { std::pair< std::pair< char, char >, char > p; printf("%lu %p %p\n", sizeof(p), &p, &p.first); return 0; } Output: 4 0x7fffffffe738 0x7fffffffe739 There is an additional byte at the beginning of the pair, for no reason. This is not really a bug, but is worth improving. It causes crashes in boost::container::flat_map, see https://github.com/boostorg/container/issues/97 Please note that this only happens on FreeBSD. System: FreeBSD 12.0 Compiler: Clang 6.0.1 (default compiler on FreeBSD) libc++: master branch on Github I'm pretty sure this is related to https://reviews.llvm.org/D25389 The base class __non_trivially_copyable_base introduces the padding. I don't understand why Empty Base Optimization doesn't work in that case. To reproduce this, I used the following virtual machine: https://download.freebsd.org/ftp/releases/VM-IMAGES/12.0-RELEASE/amd64/Latest/ Upstream issue in IKOS: https://github.com/NASA-SW-VnV/ikos/issues/22
Confirmed, this is not just weird, it's an (unintended) ABI break. The problem is that p.first can't be laid out at offset 0, because that would put two objects of __non_trivially_copyable_base at that offset, which C++'s layout rules don't permit. The easiest solution would be to template __non_trivially_copyable_base on the same template arguments as the pair.
Thanks Richard for the suggested fix. This ABI break has been shipping in FreeBSD for a while now. While I'm happy to re-commit a fix, we should ask the FreeBSD folks when they're going to be able to take a real ABI break on pair so we can make it trivial.
@emaste and @dim Could you weigh in on behalf of FreeBSD? Adding `_LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR` accidentally broke the ABI as described in this bug. Fixing in "breaks" the ABI again. Wh
Fix up for review as https://reviews.llvm.org/D56357
This is has always been a bit of a (In reply to Eric Fiselier from comment #3) > @emaste and @dim Could you weigh in on behalf of FreeBSD? > > Adding `_LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR` accidentally > broke the ABI as described in this bug. Fixing in "breaks" the ABI again. Wh The "Wh" was maybe part of a sentence that you were either going to finish, or decided not to add? :) In any case, this std::pair trivial copy constructor has always been a little bit of an ugly wart for FreeBSD, since we were the first to actually ship with a libc++.so using this bad ABI. I don't really know what our plans are breaking ABI, we'll probably have to go for libc++.so.2 at some point (and with /usr/include/c++/v2 too?), but for backwards compat reasons, we will likely always have to ship a libc++.so.1 too. (Or it will go into a compat-for-freebsd-12-and-earlier package.) Ideally you would solve such an issue with a linker script and symbol versioning, but this is probably hard with libc++. That said, I don't think anybody will actually want to *depend* on having the extra padding that this bug report is about, so I think it is likely best to apply the D56357 fix at some point, and maybe even backport it to older FreeBSD branches. Ed, what's your opinion on this?
(In reply to Dimitry Andric from comment #5) > > The "Wh" was maybe part of a sentence that you were either going to finish, > or decided not to add? :) Yeah, it was just a mistake. Ignore it. > > In any case, this std::pair trivial copy constructor has always been a > little bit of an ugly wart for FreeBSD, since we were the first to actually > ship with a libc++.so using this bad ABI. I don't think std::pair ever crosses the shared library boundary. If it doesn't then we can omit any discussion about shipping and versioning libc++.so. Then the only concern becomes programs and libraries that mix libc++ versions and pass std::pair between them. I'm assuming mixing libc++ header versions is a case FreeBSD supports? > > That said, I don't think anybody will actually want to *depend* on having > the extra padding that this bug report is about, so I think it is likely > best to apply the D56357 fix at some point, and maybe even backport it to > older FreeBSD branches. OK. Note that by "depending" on the extra padding really just means "I mixed libc++ headers of different versions", because accessing `p.first` depends on the if there is padding. So we will be breaking all programs that pass std::pair<std::pair<T, U>, V> between differently compiled TU's. Roughly, my point is that if we're going to break some ABI's using std::pair, We should consider changing triviality at the same time. However, note that D56357 only affects cases of nested pairs or a pair<T, U> where T has a pair at offset zero. But changing the triviality will break all usages of std::pair. Again, these cases only matter if FreeBSD supports programs and libraries which are compiled or linked against more than one version of the libc++ headers (not including libc++.so). > > Ed, what's your opinion on this?
I'm going to go ahead and commit the fix since the release branch is tomorrow. @Ed or @Dimitry, if you object to this fix, please speak up ASAP.
Committed in r351290.
Looks like this has been fixed, so I'm going to close this to clean up the bug list.