MDEV-39241: THD::backup_commit_lock warning dangling-pointer

[grooverdan]

The mechanisms of XA backup locks use a stack allocated mdl_request object. This is passed and stored in the THD object. Compilers complain about this behaviour because the lifetime of the THD->backup_commit_lock may exceed the lifetime of the stack variable mdl_request.

As the complier is also free to reuse the memory of mdl_request after its last reference in the code, the lifetime of the variable isn't defined after the function call to trans_xa_get_backup_lock.

As the THD has an allocation mechanism defined, and can release used memory, lets change this implementation and be explicit, and not rely on the compiler doing the right thing in the realm of undefined behaviour.

caused by MDEV-35110 / 066f920

[svoj]

It is very wrong that the code stores MDL_request in general, it should store MDL_ticket instead. Could you check if it is doable to make this change?

[andrelkin]

Dear gents,

MDEV-36025 patch and analysis visited this piece of code that the Jira ticket compiler complained.
And it did it wrong!

If we carefully check thd->backup_commit_lock value change in trans_xa_commit (the caller for next)
we find the following strict sequence of events:

1. Allocation for MDL_request by the caller
2. Use of its address as RHS in the caller's stack
3. Reset/Cleanup with the same allocation's stack.

That is thd->backup_commit_lock == nullptr when execution leaves that caller's branch.

I did not not overlook anything?

[svoj]

@andrelkin you're right, it must be compiler bug.

[grooverdan]

Closing in favour o f #5160

[grooverdan]

@andrelkin you're right, it must be compiler bug.

No, current code is just relying on the complier not to reuse its stack after the last reference to the local variable in the branch//frame/function, which is is free to do so.