MySQL Bugs: #115712: Assignment from a local empty SQL_I

Bug #115712	Assignment from a local empty SQL_I_list got an illegal object
Submitted:	29 Jul 2024 8:58	Modified:	5 Aug 2024 12:23
Reporter:	Xingyu Yang (OCA)	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: Compiling	Severity:	S3 (Non-critical)
Version:	8.0.39, 8.4, 9.0	OS:	Any
Assigned to:		CPU Architecture:	Any
Tags:	Contribution

Description:
SQL_I_list uses member field 'next' to track the pointer field to
the next object. It is initialized as the address of member field
'first', which is only valid with the list object constructed with
regular constructors.

However, assignment operators and move constructor of SQL_I_list
used trivial implementations, as of 8.0.14, by Bug#28509897 GCC 9
WARNINGS. As a result, the lhs got an illegal 'next' field.

How to repeat:
SQL_I_List<TABLE_LIST> *x = new (thd->mem_root) SQL_I_List<TABLE_LIST>;
SQL_I_List<TABLE_LIST> *y = new (thd->mem_root) SQL_I_List<TABLE_LIST>;
*y = *x;
destroy(x);

gdb> p *x
elements =
0
first =
0x0
next =
0x7ffb3aafba10
gdb> p &x->first
0x7ffb3aafba10
gdb> p *y
elements =
0
first =
0x0
next =
0x7ffb3aafba10
gdb> p &y->first
0x7ffb3aafba28

y->next is illegal here

Suggested fix:
Supply customized move constructor and operator= functions.

Supply customized move constructor and operator= functions (*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: 0001-bugfix-issue-1432-Assignment-from-a-local-empty-SQL_.patch (application/octet-stream, text), 2.02 KiB.

Hello Xingyu Yang,

Thank you for the report and contribution.

regards,
Umesh

Posted by developer:
 
Note that the bug was not introduced by the patch for  
  Bug#28509897 GCC 9 WARNINGS.
This bug has always been there, since default assignment operator is memberwise assignment.

The bug has apparently never caused any problems, probably because we never have done any assignment of empty lists.

Documented fix as follows in the MySQL 9.1.0 changelog:

    SQL_I_list uses a member field -next- to track the pointer field
    to the next object, and is initialized as the address of the
    member field -first-, which is valid only with the list object
    constructed with regular constructors. The default assignment
    operator used memberwise assignment, which is not correct for an
    empty list.

    This is fixed by supplying a customized move constructor and
    operator = function. In addition, we also change the
    implementation of save_and_clear() to use std::move() rather
    than assignment.

    Our thanks to Xingyu Yang and the Tencent team for the
    contribution.

Closed.