Bug 114817 - Wrong codegen for std::copy of "trivially copyable but not trivially assignable" type
Summary: Wrong codegen for std::copy of "trivially copyable but not trivially assignab...
Status: UNCONFIRMED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: unknown
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-04-23 04:02 UTC by Arthur O'Dwyer
Modified: 2024-04-26 18:43 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Arthur O'Dwyer 2024-04-23 04:02:26 UTC
Jiang An raises an interesting issue over at
https://github.com/llvm/llvm-project/pull/89652#discussion_r1575540420
namely:

// https://godbolt.org/z/64KGP1avE
template<class T, class U>
struct EvilPair {
    T& first;
    U& second;
    EvilPair(T& t, U& u) : first(t), second(u) {}
    EvilPair(const EvilPair&) = default;
    void operator=(const volatile EvilPair&) = delete;
    template<class = void> EvilPair& operator=(const EvilPair& rhs) {
        first = rhs.first;
        second = rhs.second;
        return *this;
    }
};

static_assert(std::is_trivially_copyable_v<EvilPair<int&, int&>>);

int main() {
    int a[] = {1,2,3};
    int b[] = {4,5,6};
    int c[] = {0,0,0};
    int d[] = {0,0,0};

    EvilPair<int&, int&> ps[] = {
        {a[0], b[0]},
        {a[1], b[1]},
        {a[2], b[2]},
    };
    EvilPair<int&, int&> qs[] = {
        {c[0], d[0]},
        {c[1], d[1]},
        {c[2], d[2]},
    };
    std::copy(ps, ps+3, qs);
    printf("%d %d %d\n", c[0], c[1], c[2]);
}

Here, EvilPair is trivially copyable, and also copy-assignable, but it is not trivially copy-assignable. libstdc++'s std::copy assumes that any trivially copyable type can be... well, trivially copied. So it copies the object representation of EvilPair, instead of doing overload resolution to discover that in fact the templated `operator=` should be called instead.

Looks like the std::copy optimization was introduced in the GCC 9 release.

Allegedly Microsoft STL's `std::pair<int&, int&>` is isomorphic to `EvilPair` these days.

libc++'s `std::copy` avoids this issue (AFAICT) by gating their optimization on *both* is_trivially_assignable and is_trivially_copyable.

I suspect there will be similar issues with `uninitialized_foo` functions and/or vector reallocation, for types that are trivially copyable (or trivially relocatable!) and yet not trivially destructive-movable.
Comment 1 Arthur O'Dwyer 2024-04-23 18:29:53 UTC
Yes, vector reallocation has analogous trouble with types that are "trivial, but not trivially copy constructible."
https://godbolt.org/z/Psboqf3MP

(libc++ happens to sidestep this pitfall *on Clang 15+,* because libc++ gates their vector reallocation optimization differently depending on whether `__has_builtin(__is_trivially_relocatable)`; but libc++ would have the same bug as libstdc++ when compiled on GCC.)
Comment 2 Jiang An 2024-04-24 03:31:04 UTC
Bug 106547 seems somehow related.
Comment 3 Arthur O'Dwyer 2024-04-26 18:43:37 UTC
https://github.com/boostorg/container/issues/153 , from 2020, is another similar issue. There, boost::container::vector had assumed that if __has_trivial_copy(T) and is_copy_constructible_v<T>, then T could be relocated via memcpy; but in fact __has_trivial_copy(T) is also true for move-only types. So it was possible to create a type such that (__has_trivial_copy(T) && __is_constructible(T, T&)) and yet not __is_trivially_constructible(T, T&).

// https://godbolt.org/z/x5Wda1M6E
struct T {
    template<class U> T(U&&);
    T(T&&);
    ~T();
};
static_assert(__has_trivial_copy(T) && __is_constructible(T, T&));
  // ...and yet...
static_assert(not __is_trivially_constructible(T, T&));