<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>Issue</th>
<td>
<a href=https://github.com/llvm/llvm-project/issues/60112>60112</a>
</td>
</tr>
<tr>
<th>Summary</th>
<td>
Wrongful cleanup for `trivial_abi` parameter after passing it to callee
</td>
</tr>
<tr>
<th>Labels</th>
<td>
clang:codegen
</td>
</tr>
<tr>
<th>Assignees</th>
<td>
</td>
</tr>
<tr>
<th>Reporter</th>
<td>
aaronpuchert
</td>
</tr>
</table>
<pre>
Invoking `caller` in the following code leads to double destruction:
```c++
struct __attribute__((trivial_abi)) trivial {
trivial() = default;
trivial(const trivial &) noexcept;
~trivial();
int* p = nullptr;
};
struct other {
other() = default;
other(const other&) noexcept;
~other();
};
void callee(trivial f, other) { throw 0; }
bool b = true;
void caller() {
trivial f;
other n;
b ? callee(f, n) : void();
}
```
Since `trivial` has a `trivial_abi` attribute, it must generally be cleaned up by the callee (except if initialization of another parameter fails, which cannot happen here due to the `noexcept` on the `other` copy constructor). But the code around the call looks like this:
```llvm
define dso_local void @_Z6callerv() #0 personality ptr @__gxx_personality_v0 {
entry:
; ... setting up locals
store i1 false, ptr %cleanup.cond, align 1
store i1 false, ptr %cleanup.cond2, align 1
br i1 %tobool, label %cond.true, label %cond.false
cond.true: ; preds = %entry
call void @_ZN7trivialC1ERKS_(ptr noundef nonnull align 8 dereferenceable(8) %agg.tmp, ptr noundef nonnull align 8 dereferenceable(8) %f) #3
store i1 true, ptr %cleanup.cond, align 1
call void @_ZN5otherC1ERKS_(ptr noundef nonnull align 1 dereferenceable(1) %agg.tmp1, ptr noundef nonnull align 1 dereferenceable(1) %n) #3
store i1 true, ptr %cleanup.cond2, align 1
%coerce.dive = getelementptr inbounds %struct.trivial, ptr %agg.tmp, i32 0, i32 0
%1 = load ptr, ptr %coerce.dive, align 8
invoke void @_Z6callee7trivial5other(ptr %1, ptr noundef %agg.tmp1)
to label %invoke.cont unwind label %lpad
lpad: ; preds = %cond.true
%2 = landingpad { ptr, i32 }
cleanup
; ... exception logistics
%cleanup.is_active3 = load i1, ptr %cleanup.cond2, align 1 ; = true
br i1 %cleanup.is_active3, label %cleanup.action4, label %cleanup.done5
cleanup.action4: ; preds = %lpad
call void @_ZN5otherD1Ev(ptr noundef nonnull align 1 dereferenceable(1) %agg.tmp1) #3
br label %cleanup.done5
cleanup.done5: ; preds = %cleanup.action4, %lpad
%cleanup.is_active6 = load i1, ptr %cleanup.cond, align 1 ; = true
br i1 %cleanup.is_active6, label %cleanup.action7, label %cleanup.done8
cleanup.action7: ; preds = %cleanup.done5
call void @_ZN7trivialD1Ev(ptr noundef nonnull align 8 dereferenceable(8) %agg.tmp) #3
br label %cleanup.done8
cleanup.done8: ; preds = %cleanup.action7, %cleanup.done5
call void @_ZN5otherD1Ev(ptr noundef nonnull align 1 dereferenceable(1) %n) #3
call void @_ZN7trivialD1Ev(ptr noundef nonnull align 8 dereferenceable(8) %f) #3
br label %eh.resume
eh.resume: ; preds = %cleanup.done8
; ... exception logistics
resume { ptr, i32 } %lpad.val9
}
```
So if `caller` throws, we destruct both parameters (and both locals). But we don't own the first parameter anymore after the call. Inspecting `callee` shows that it calls `_ZN7trivialD1Ev` on unwinding the exception that it has thrown. So we destruct the first parameter twice.
If we drop the ternary and unconditionally call `callee`, all is fine. We get:
```llvm
define dso_local void @_Z6callerv() #0 personality ptr @__gxx_personality_v0 {
entry:
; ... setting up locals
call void @_ZN7trivialC1ERKS_(ptr noundef nonnull align 8 dereferenceable(8) %agg.tmp, ptr noundef nonnull align 8 dereferenceable(8) %f) #3
call void @_ZN5otherC1ERKS_(ptr noundef nonnull align 1 dereferenceable(1) %agg.tmp1, ptr noundef nonnull align 1 dereferenceable(1) %n) #3
%coerce.dive = getelementptr inbounds %struct.trivial, ptr %agg.tmp, i32 0, i32 0
%0 = load ptr, ptr %coerce.dive, align 8
invoke void @_Z6callee7trivial5other(ptr %0, ptr noundef %agg.tmp1)
to label %invoke.cont unwind label %lpad
lpad: ; preds = %entry
%1 = landingpad { ptr, i32 }
cleanup
; ... exception logistics
call void @_ZN5otherD1Ev(ptr noundef nonnull align 1 dereferenceable(1) %agg.tmp1) #3
call void @_ZN5otherD1Ev(ptr noundef nonnull align 1 dereferenceable(1) %n) #3
call void @_ZN7trivialD1Ev(ptr noundef nonnull align 8 dereferenceable(8) %f) #3
br label %eh.resume
eh.resume: ; preds = %lpad
; ... exception logistics
resume { ptr, i32 } %lpad.val2
}
```
It gets more interesting if we drop `noexcept` from `other`'s copy constructor:
```llvm
define dso_local void @_Z6callerv() #0 personality ptr @__gxx_personality_v0 {
entry:
; ... setting up locals
store i1 false, ptr %cleanup.cond, align 1
store i1 false, ptr %cleanup.cond2, align 1
br i1 %tobool, label %cond.true, label %cond.false
cond.true: ; preds = %entry
call void @_ZN7trivialC1ERKS_(ptr noundef nonnull align 8 dereferenceable(8) %agg.tmp, ptr noundef nonnull align 8 dereferenceable(8) %f) #4
store i1 true, ptr %cleanup.cond, align 1
invoke void @_ZN5otherC1ERKS_(ptr noundef nonnull align 1 dereferenceable(1) %agg.tmp1, ptr noundef nonnull align 1 dereferenceable(1) %n)
to label %invoke.cont unwind label %lpad
invoke.cont: ; preds = %cond.true
store i1 true, ptr %cleanup.cond2, align 1
%coerce.dive = getelementptr inbounds %struct.trivial, ptr %agg.tmp, i32 0, i32 0
%1 = load ptr, ptr %coerce.dive, align 8
store i1 false, ptr %cleanup.cond, align 1 ; !!!
invoke void @_Z6callee7trivial5other(ptr %1, ptr noundef %agg.tmp1)
to label %invoke.cont4 unwind label %lpad3
lpad: ; preds = %cond.true
%2 = landingpad { ptr, i32 }
cleanup
; ... exception logistics
br label %ehcleanup
lpad3: ; preds = %invoke.cont
%5 = landingpad { ptr, i32 }
cleanup
; ... exception logistics
%cleanup.is_active5 = load i1, ptr %cleanup.cond2, align 1 ; = true
br i1 %cleanup.is_active5, label %cleanup.action6, label %cleanup.done7
cleanup.action6: ; preds = %lpad3
call void @_ZN5otherD1Ev(ptr noundef nonnull align 1 dereferenceable(1) %agg.tmp1) #4
br label %cleanup.done7
cleanup.done7: ; preds = %cleanup.action6, %lpad3
br label %ehcleanup
ehcleanup: ; preds = %cleanup.done7, %lpad
%cleanup.is_active8 = load i1, ptr %cleanup.cond, align 1 ; = phi i1 [ false, %cleanup.done7 ], [ true, %lpad ]
br i1 %cleanup.is_active8, label %cleanup.action9, label %cleanup.done10
cleanup.action9: ; preds = %ehcleanup
call void @_ZN7trivialD1Ev(ptr noundef nonnull align 8 dereferenceable(8) %agg.tmp) #4
br label %cleanup.done10
cleanup.done10: ; preds = %cleanup.action9, %ehcleanup
call void @_ZN5otherD1Ev(ptr noundef nonnull align 1 dereferenceable(1) %n) #4
call void @_ZN7trivialD1Ev(ptr noundef nonnull align 8 dereferenceable(8) %f) #4
br label %eh.resume
eh.resume: ; preds = %cleanup.done10
; ... exception logistics
resume { ptr, i32 } %lpad.val13
}
```
So after having initialized the second parameter, and before handing ownership of the `trivial_abi` object into the callee, we write `false` into `cleanup.cond`. So we only clean up if initializing the second parameter failed and we couldn't pass on the object.
</pre>
<img width="1px" height="1px" alt="" src="http://email.email.llvm.org/o/eJzsWk1v4zjS_jX0pTCCRFmyffAhmUyAxgu8h-3DAHsxKKlkcZsmBZKyO3uY374gJVmyLTtOTzozWWxgdMf8KFbVU8X6CJkxfCsR1yR5JMnTjDW2UnrNmFaybvIKtZ1lqnhZf5F79Y3LLZA0zJkQqEkaApdgK4RSCaEObjZXBYJAVhiwCgrVZAKhQGN1k1uuJIkfSPhEwv7fNGw_OaGP7uNH29Ww2TBrNc8ai5sNoUtCl1bzPWdiwzJO6IrQFXQjQBbdZuiH_I4VkPgJCixZIyyJJ9bkSho7kKGp2yQVfs-xHu3444TqiBKXltAHqP1BshGitvo4TRZPw-9j2ZStUI-Z9gM3We5XtAx3365wC3-M6N3gZq94AR5OHLQLJaG_9gesHJNgK60OEJL4ERyREYVMKQGZZ9nqBq_SP8p2gROU50KCHI042s8Dj5432WrpARz9CSFPbWvM0Fcuc3Q23MOZhlAxA2w05q0rDeFofe5IbmHXGAtblKiZEC-QIeQCmcQCmhqyF-8JLZ9A6LKFBHgJXHLLmeD_Zs4DQJXAZCtnzTTboUUNJePCuHMOFc8ryJmUykLF6holVKgRigadS7lDSBoeIU9DULIfbTFLQ8hV_QLeUJy1KYdjAI-NbXl0Psq0amRx5BmEUt8MCP4NwVbcXPNTIfa7dqjAkkuEwqiNUDkTHgsg83Dzz7RFfN9DTuMQatRGSSa4fYHaar9ws_3-fTOa2OzDwT5QWv1yZAPA2V4QBGDQWnfTNDX4c02_wFilEXgEJRPGY-bPoYlHqamDXMnCDTPBtxKiN-2jExsz7XYRmljlvMCtECxD4fcqWQTeIS5G22NGyh3Wxg9w54_TRq2xMN7zCE1adXWceUQHPP5_0Vn2r9Fv__i_r-4udSJKZwFYglTS3VudeEsoUGOJGmWOLBPO6ToYE7bdBnZX9zp6O4Gys4f4Qvm9ru7D7FzAxFv-PeJFE9xFp-JFt-W7RUH-iHxTtuWNBXWOQcH36EHeokWBO5TW0eAyc9wZt7L18uAYoY6njPDiMYVw-GU4JvLEhWKF2zVmcTh_YHA5hL29-oYXTo-9qSV9AOqoXSj1RN-rnmz_Y9XgNe1RTlUWGnngshjmRM2KsS_57_e70bQvDQ45qIm2amKy4HJbs8KHxU5hTqXHsDPQ7TA-v8Dai9tFAqG23FiemzHqnWFws2G55XuMB3x4dIcFjYQ6BuXzC-vykNNrqptmPmWbT84VSmJycoudbYofLhU7oHXNhZ-i3_bv4L-nTpjpN_DfTtxnQpeWc6m5M7GntJ_eAfElwm-EOL0B8eIqxMvrEC8mIZ7Q7_V49CradwWj-8CelKSduCXHSEH3SvcOlnweR36KAi-C8Vh1WAUaTbM7yVSGwdegX77h1mtJTlyove8EeyZWt7P7r8pl2ie1qa9Z2px6KEEhU7Ya8m4XPpdMFu1wl0_2ubLbpiShCwvq0BW6XBs7StuZfNm5AM9K961PpgP4Ik2NuR3Xy-h4MpU6GLAVs66gcOPGLThHtE3q21jnaDjCgwL77a5u8ULKAL6qEymneLUHnmMwhvNL6TdpVfsNFrVk-gWcPhrpLh3uzvPFjjfAsSjtfSSAG3B1QAC_o8tQhrrhE1QMnydN_jT57sflreHH5K3hJ8pbT2rAIbX_uTnrB-ZxHxBo_5aR9kft4STvfKc4TO_psn2xzusN-NjIpUWNxl_BfIg4Z02sUqvduIVF6MJctrE-U3T5Xz_q8_Sj5n-yH3URVf5WEfq9gtRo7XQBcN43-a_tfr3Vt2_6DaFR9_lLe2vzSfzjqSzlnTtmb086TuPnyf6B0_iNCdWFVGN7H-RK_qpOYPJDncC3dYmSG12i6Q6Sq_YX17tE6dVG4M9M684yyPlrzaFJAdqJd-kEpqNO4JUkcMKIh8HX2i2Ly07jFLrLH2o09hZUV9ybTfI4XHvnfABJnvx48ni89Du-_NSrBri8YYCrqwYYhdctcDWpvTOFf0iP8lUznBajm3m9S7nqtP2qbO9YOM0_okM5qbg_2aGMwvcujaL41R5l2yes2N4XQ_27AGz_Dm_Q-d_QsfOeKAvIsHTZRtVGHFAHidpUvAZV9n_4P3u5oLJ_YW5d5aVGrxK6VuhBc-s3tU7sX_FY5Zt742sgDfuuopLipQ1hrsgZP2joe5PnnPvnDFh47g8IuWpE0TZSa2ZM_2Ch5TKYFeu4WMUrNsN1lC7i1SpO6GJWreMwwmWW0bwsl1mKcZqFMV2V0Xy-oCWdz2d8TUMah1G0oDSi4SKIYxbOMU2TIkzyLEQyD3HHuAhcmRgovZ1xYxpcp2EU0Zm3JONfPlGaCya3JH7IVYFblIRSkjzN9Nrt_CVrtobMQ8GNNQMty63A9e9ayW3ZiD7IQ6n0BCSjnrG3AacIbwTW5WMtQLNGi3Vlbe2fX9BnQp-33FZNFuRqR-izL3bb_36ptXLaI_TZi2QIffZS_ScAAP__Ds511A">