<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>Issue</th>
<td>
<a href=https://github.com/llvm/llvm-project/issues/87662>87662</a>
</td>
</tr>
<tr>
<th>Summary</th>
<td>
[clang] Double free generated in move assignment when accessing a pointer via mixin
</td>
</tr>
<tr>
<th>Labels</th>
<td>
clang
</td>
</tr>
<tr>
<th>Assignees</th>
<td>
</td>
</tr>
<tr>
<th>Reporter</th>
<td>
kaimfrai
</td>
</tr>
</table>
<pre>
The following code erroneously results in a double free/delete being generated for the move assignment operator. It's a simplified version of a customizable dynamic array template: One version would simply borrow it (ommited for brevity) the other owns it. The difference is handled in 2 distinct mixin classes, selected at compile time.
```
#include <utility>
#include <new>
#include <cassert>
struct Ptr
{
int* p = nullptr;
};
template<typename T>
struct ArrayMixin
{
template<typename... U>
constexpr ArrayMixin(U... u) noexcept
{
static_cast<T&>(*this).p = new int[sizeof...(U)]{static_cast<int>(u)...};
}
constexpr ArrayMixin(ArrayMixin&& o) noexcept
{
int*& p = static_cast<T&>(*this).p;
int*& q = static_cast<T&>(o).p;
p = std::exchange(q, nullptr);
}
friend constexpr void swap(ArrayMixin& l, ArrayMixin& r) noexcept
{
int*& p = static_cast<T&>(l).p;
int*& q = static_cast<T&>(r).p;
assert(p != q);
std::swap(p, q);
}
constexpr ArrayMixin& operator=(ArrayMixin&& o) noexcept
{
int*& p = static_cast<T&>(*this).p;
int*& q = static_cast<T&>(o).p;
assert(p != q);
if (p)
{
delete[] p;
p = nullptr;
}
p = std::exchange(q, nullptr);
return *this;
}
constexpr auto& operator[](int i) noexcept
{
return static_cast<T&>(*this).p[i];
}
constexpr ~ArrayMixin() noexcept
{
int*& p = static_cast<T&>(*this).p;
if (p)
{
delete[] p;
p = nullptr;
}
}
};
struct Combined : Ptr, ArrayMixin<Combined>
{
using ArrayMixin<Combined>::ArrayMixin;
};
constexpr inline auto Swap() noexcept
{
Combined c{9, 7, 6, 5, 8};
c[0] = 5;
Combined d{};
swap(c, d);
return d[0];
}
constexpr inline auto Construct() noexcept
{
Combined c{9, 7, 6, 5, 8};
c[0] = 5;
Combined d{std::move(c)};
return d[0];
}
constexpr inline auto Assign() noexcept
{
Combined c{9, 7, 6, 5, 8};
c[0] = 5;
Combined d{};
d = std::move(c);
return d[0];
}
int main()
{
return 5 - Swap() + Construct() - Assign();
}
```
https://godbolt.org/z/Ef193W48G
With -O3 and -DNDEBUG this generates 2 calls to delete[] for only 1 call for new[]. Without either flag an assertion is triggered in the assignment operator. Suprisingly the pointers pointing to different allocations are the same, resulting in that allocation being freed twice. This does not happen when using the move constructor or swap.
Moving the int* p member directly into the mixin or letting the mixin inherit from Ptr directly resolves the issue. It is not an option however if you want multiple mixins to operate on the same member and only one mixin can hold the member to access it.
It is my understanding that acessing the variable p from ArrayMixin would also be correct, as Ptr is within its lifetime by the time the constructor for ArrayMixin is called and especially by the time the assignment operator is called. Even when adding another cast from T& (not fully constructed in the Array Mixin constructor, but it's just referenced anyways) to Ptr&, the behaviour is the same.
</pre>
<img width="1px" height="1px" alt="" src="http://email.email.llvm.org/o/eJzUWEuP47gR_jX0pdCGTfl58KHdnl7ksNkAO4M9BhRZsrihSA1J2e055LcHRcoPtXtmOtlBkjUaalmq1_dVuciiCEHvLeKGzbdsvhuJLtbOb_4hdFN5oUelU6fNxxqhcsa4o7Z7kE4hoPfOouuCOYHH0JkYQFsQoFxXGoTKIzL-rNBgRCiRFPdo0YuICirnIdYIjTsg5BAatBFcSwLOj-EvkfFlAAFBN63RlUYFB_RBOwuuAgGyC9E1-osgb-pkRaMlCO_FCSI2rRERWfEIv1i86B1dZ1Q2eILSee-OoCMwvnJNo89xlR4POp4YX6cQXazRgzvaADqOgahQuqrQo5UIOkAtrDKoCD0HpUPUVkZo9Iu2II0IAQPjTxDQoCQfIoJ0TasNQtQNjtlkxyaP_XUx6f_yV15oK02nEFjx1EVtKLDiw1tvLR6_8kZSDD5e36ZriL6TEf4Wff90uc03oG1k_BFaYMUObGdMGz0rtmex3fU-Xa9sP8VTi1Y0CB8vzno3j5SYn4mT194A4A0L4_EYPl2MkJB0NkR8af2tLb76RJIdJcs6fJHYxqvKwAl9QhRRy79LESIrnj4yviAXfMX4Y6x1YHw97lHjMdEw3wb9BV01Ho_JF-NrNt-x5XZoiCSTHYqDRG84ynHsbgn7BprbLwvGF-DeiSznjDQygPcgHcQ4NPL5m0bcUL03cnasWPHIikd8kbWwe2R89Znq_1xIfP09biqv0aobig5OKwhH0b5mCAxZHj7yP5gx88e48q_U-98iX7XA-JQ0P7-mJFdqT2MPuyWgd5LvLqzFpbOyYvdnrbP3cqcryIyth8_vwNAnL1B58YP7UOnzdiO8Wr3LwVXp3_k1DPU9xs5bOPP4raxfUy666AbJTrAYX2kbQb8zxb3nd2V2vtXk4N01-c9Bt_uv1Nz_qBqGVNwtmv2y-OSaUltUQFsVWoqH7ax4OgtcF-_bsLtA26qvKqTKu3371TX8miFtjbaYKgl-zb3nPk2DIC4YJFtu14RgSZcFXeZ0Wb1eDyWbbyfEMDE5v766WFLk4JVS3wglWVRv_276ylW9-QHe74J9oqeUlO8g_nFwb7lTaVPRNwvaFWeo6_tU_WGgj2m7_X-TVzVslLfY_2Pg1O4ace4xb2Dr7czh4bbIGd_elcHDgK83HA537HWMbSAk_Jnx571TpTNx7Pye8ecvjD9_qKbr4rfZ6qfbcH_TsYaHXwoQVsHD7q-7D9tPPwH1ssvIFICDFMYEiG7YpWhkcdacYJoE0neaBdLbMZBt10VAneaYyog9CNsvpTQU6QDR6_0efZ5haOZ5cyL7tWu9poZjTkmoddpG9CHfUCOi0PrRKIIwxklBLgIIj0kliAapcvLASCrJobiV7mdFmh8VxKOWSEOXDqAcBrAuQi3aFi0ca7R9B7yMkvKcPSLFp54xmK9-doez_GXKabAp0YPSHmU0J3rhssU0wjkPBmO8uEkPta3R6wiVdw317au2x-DMAUP2EUKHNMsSyxS6sODahLJ2Rzygp-Xp5Do4CipZIqU1vZOU6kw_grMXAs8BU7GkxDt7DksKMmxUjjSLRQdCSgxpfM0s5HiaE3RWoQ9RWJXhUSJI9Az2ILxO83WbkV7Xkn6WFiY4KIl3T_gptyIkQnSAo441cRUDGF0hDbtQ5tpJ93RzmzCq3BsPOqSCponZKsDQotTC0OT-ysYb1XpVHsOHw7lYhEo4hc0zPW0gMi7aQ9AugVJUdeTjEtf1R5FCgxzbTdiEuewi6Hxg8XsXInjszwco9tNRnEI6TXB5iU8dlCyWWIuDdl0K95zeQb2O1KZQ62ItRriZLqecz_l0Nh_Vm9l6sVgX05mcr1ez9WQ1n6zEcjItBUe5KKvVSG_4hM8ms8lsuizmxXrMF2o5WZVSLtR6UUwEm02wEdqMjTk01KFGqVo3q-ViwUdGlGhCOhTiXBph94xzNt-N_IbkH8puH9hsYnSI4Woh6mjSSVLWmO9gdz0Nujn-0fbu5CcnSJ6rT5zbCxy0yNU96rzZvOqvOtZdOZauYfyZouj_PbTe_Z7q8TmBCow_J1z_CgAA__8ly5mE">