Bug 106547 - std::variant::emplace goes into an infinite recursion with certain weird trivially copyable types
Summary: std::variant::emplace goes into an infinite recursion with certain weird triv...
Status: NEW
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: wrong-code
Depends on:
Blocks:
 
Reported: 2022-08-06 18:27 UTC by Valentine Anderson
Modified: 2023-09-11 13:53 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2022-08-06 00:00:00

Attachments
Preprocessed source file (39.44 KB, text/plain)
2022-08-06 18:27 UTC, Valentine Anderson 		Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Valentine Anderson 2022-08-06 18:27:35 UTC 
Created attachment 53421 [details]
Preprocessed source file

Consider the following source code (preprocessed version attached):

#include <concepts>
#include <variant>

struct weird {
  weird() {}
  weird(std::same_as<weird> auto&&) {}
  weird(const weird&) = default;
  weird& operator=(const weird&) = default;
};

int main() {
  std::variant<std::monostate, weird> data;
  data.emplace<weird>();
}

When executing, the emplace call goes into an infinite recursion.

The emplace function has a number of different implementations, selected using `if constexpr`. The first `if constexpr` branch has the condition `is_nothrow_constructible_v<type, _Args...>`; since the constructor that we're using is not noexcept, that branch is not taken. The second branch has the condition `is_scalar_v<type>`, which does not apply, either.

The third branch has the condition `__detail::__variant::_Never_valueless_alt<type>() && _Traits::_S_move_assign`. The second part simply requires that all alternatives be move-assignable, which is true here. The first part checks two things: that the type's size not exceed 256 bytes, which is true, and that the type be trivially copyable. The `weird` type has a trivial destructor, a trivial copy constructor, a trivial copy assignment operator, no move constructor, and no move assignment operator; therefore, it is trivially copyable and the third branch is chosen.

The implementation in the third branch constructs a temporary variant on the stack and then move-assigns it to `*this`. The move assignment ultimately calls emplace again, this time constructing the alternative from `weird&&`. The text in the comment appears to expect that the first `if constexpr` branch -- corresponding to the noexcept constructor -- will be chosen.

However, construction from `weird&&` actually calls `weird`'s second constructor, which is not noexcept. (It does not prevent `weird` from being trivially copyable, since it is a template and therefore is not a move constructor.) Therefore, this actually chooses the third branch once again, which, once again, constructs a temporary variant on the stack and move-assigns it to `*this`. This causes an infinite recursion.

To sum up, the key problem here is that the `weird` type is trivially copyable, but its move construction is not noexcept, because it invokes a constructor template that is not a move constructor. (A real-world example of such a weird type is gsl::not_null from Microsoft's GSL implementation <https://github.com/Microsoft/GSL>).

Full invocation of GCC is below. (I also reproduced it on godbolt.org using the "gcc (trunk)" compiler and checked the source code in the git repository to make sure the offending lines are still there.)

$ g++ -v -save-temps -std=c++20 test.cpp -o test
Using built-in specs.
COLLECT_GCC=g++
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/11/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 11.2.0-19ubuntu1' --with-bugurl=file:///usr/share/doc/gcc-11/README.Bugs --enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++,m2 --prefix=/usr --with-gcc-major-version-only --program-suffix=-11 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-plugin --enable-default-pie --with-system-zlib --enable-libphobos-checking=release --with-target-system-zlib=auto --enable-objc-gc=auto --enable-multiarch --disable-werror --enable-cet --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none=/build/gcc-11-gBFGDP/gcc-11-11.2.0/debian/tmp-nvptx/usr,amdgcn-amdhsa=/build/gcc-11-gBFGDP/gcc-11-11.2.0/debian/tmp-gcn/usr --without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu --with-build-config=bootstrap-lto-lean --enable-link-serialization=2
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 11.2.0 (Ubuntu 11.2.0-19ubuntu1)
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-std=c++20' '-o' 'test' '-shared-libgcc' '-mtune=generic' '-march=x86-64'
 /usr/lib/gcc/x86_64-linux-gnu/11/cc1plus -E -quiet -v -imultiarch x86_64-linux-gnu -D_GNU_SOURCE test.cpp -mtune=generic -march=x86-64 -std=c++20 -fpch-preprocess -fasynchronous-unwind-tables -fstack-protector-strong -Wformat -Wformat-security -fstack-clash-protection -fcf-protection -o test.ii
ignoring duplicate directory "/usr/include/x86_64-linux-gnu/c++/11"
ignoring nonexistent directory "/usr/local/include/x86_64-linux-gnu"
ignoring nonexistent directory "/usr/lib/gcc/x86_64-linux-gnu/11/include-fixed"
ignoring nonexistent directory "/usr/lib/gcc/x86_64-linux-gnu/11/../../../../x86_64-linux-gnu/include"
#include "..." search starts here:
#include <...> search starts here:
 /usr/include/c++/11
 /usr/include/x86_64-linux-gnu/c++/11
 /usr/include/c++/11/backward
 /usr/lib/gcc/x86_64-linux-gnu/11/include
 /usr/local/include
 /usr/include/x86_64-linux-gnu
 /usr/include
End of search list.
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-std=c++20' '-o' 'test' '-shared-libgcc' '-mtune=generic' '-march=x86-64'
 /usr/lib/gcc/x86_64-linux-gnu/11/cc1plus -fpreprocessed test.ii -quiet -dumpbase test.cpp -dumpbase-ext .cpp -mtune=generic -march=x86-64 -std=c++20 -version -fasynchronous-unwind-tables -fstack-protector-strong -Wformat -Wformat-security -fstack-clash-protection -fcf-protection -o test.s
GNU C++20 (Ubuntu 11.2.0-19ubuntu1) version 11.2.0 (x86_64-linux-gnu)
        compiled by GNU C version 11.2.0, GMP version 6.2.1, MPFR version 4.1.0, MPC version 1.2.1, isl version isl-0.24-GMP

GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
GNU C++20 (Ubuntu 11.2.0-19ubuntu1) version 11.2.0 (x86_64-linux-gnu)
        compiled by GNU C version 11.2.0, GMP version 6.2.1, MPFR version 4.1.0, MPC version 1.2.1, isl version isl-0.24-GMP

GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
Compiler executable checksum: 619370d92bf6efc3023abfa7394da0c1
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-std=c++20' '-o' 'test' '-shared-libgcc' '-mtune=generic' '-march=x86-64'
 as -v --64 -o test.o test.s
GNU assembler version 2.38 (x86_64-linux-gnu) using BFD version (GNU Binutils for Ubuntu) 2.38
COMPILER_PATH=/usr/lib/gcc/x86_64-linux-gnu/11/:/usr/lib/gcc/x86_64-linux-gnu/11/:/usr/lib/gcc/x86_64-linux-gnu/:/usr/lib/gcc/x86_64-linux-gnu/11/:/usr/lib/gcc/x86_64-linux-gnu/
LIBRARY_PATH=/usr/lib/gcc/x86_64-linux-gnu/11/:/usr/lib/gcc/x86_64-linux-gnu/11/../../../x86_64-linux-gnu/:/usr/lib/gcc/x86_64-linux-gnu/11/../../../../lib/:/lib/x86_64-linux-gnu/:/lib/../lib/:/usr/lib/x86_64-linux-gnu/:/usr/lib/../lib/:/usr/lib/gcc/x86_64-linux-gnu/11/../../../:/lib/:/usr/lib/
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-std=c++20' '-o' 'test' '-shared-libgcc' '-mtune=generic' '-march=x86-64' '-dumpdir' 'test.'
 /usr/lib/gcc/x86_64-linux-gnu/11/collect2 -plugin /usr/lib/gcc/x86_64-linux-gnu/11/liblto_plugin.so -plugin-opt=/usr/lib/gcc/x86_64-linux-gnu/11/lto-wrapper -plugin-opt=-fresolution=test.res -plugin-opt=-pass-through=-lgcc_s -plugin-opt=-pass-through=-lgcc -plugin-opt=-pass-through=-lc -plugin-opt=-pass-through=-lgcc_s -plugin-opt=-pass-through=-lgcc --build-id --eh-frame-hdr -m elf_x86_64 --hash-style=gnu --as-needed -dynamic-linker /lib64/ld-linux-x86-64.so.2 -pie -z now -z relro -o test /usr/lib/gcc/x86_64-linux-gnu/11/../../../x86_64-linux-gnu/Scrt1.o /usr/lib/gcc/x86_64-linux-gnu/11/../../../x86_64-linux-gnu/crti.o /usr/lib/gcc/x86_64-linux-gnu/11/crtbeginS.o -L/usr/lib/gcc/x86_64-linux-gnu/11 -L/usr/lib/gcc/x86_64-linux-gnu/11/../../../x86_64-linux-gnu -L/usr/lib/gcc/x86_64-linux-gnu/11/../../../../lib -L/lib/x86_64-linux-gnu -L/lib/../lib -L/usr/lib/x86_64-linux-gnu -L/usr/lib/../lib -L/usr/lib/gcc/x86_64-linux-gnu/11/../../.. test.o -lstdc++ -lm -lgcc_s -lgcc -lc -lgcc_s -lgcc /usr/lib/gcc/x86_64-linux-gnu/11/crtendS.o /usr/lib/gcc/x86_64-linux-gnu/11/../../../x86_64-linux-gnu/crtn.o
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-std=c++20' '-o' 'test' '-shared-libgcc' '-mtune=generic' '-march=x86-64' '-dumpdir' 'test.'
Comment 1 Jiang An 2023-09-11 12:38:18 UTC 
Well, is_trivially_copyable looks like a red herring - it doesn't really mean that we can trivially copy such an object in some way.
Comment 2 Valentine Anderson 2023-09-11 13:27:08 UTC 
From what I understand, the key feature of trivially copyable types is that memcpy‘ing an object of such a type onto another object is equivalent to a copy assignment. So it is possible to trivially copy such an object, using memcpy.
Comment 3 Jiang An 2023-09-11 13:53:27 UTC 
(In reply to Valentine Anderson from comment #2)
> From what I understand, the key feature of trivially copyable types is that
> memcpy‘ing an object of such a type onto another object is equivalent to a
> copy assignment. So it is possible to trivially copy such an object, using
> memcpy.

The current standard wording only guarantees that such copy is OK when the destination object is already created.

A trivially copyable class is usually also an implicit-lifetime class, so memcpy is usually sufficient to create that object. But there're also weird trivially copyable class that is not an implicit-lifetime class (e.g. the class may have deleted copy/move ctors and trivial assignment operators).

Moreover, it doesn't seem suitable to use memcpy in the cases involved in this issue...