Bug #110254 Equality operator needs to be updated after LWG-3865
Submitted: 1 Mar 2023 15:43 Modified: 7 Aug 2023 22:17
Reporter: Stephan Lavavej Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Security: Privileges Severity:S3 (Non-critical)
Version:8.0 OS:Any
Assigned to: CPU Architecture:Any

[1 Mar 2023 15:43] Stephan Lavavej
Description:
I work on Microsoft's C++ Standard Library implementation, and we regularly build open-source projects (including mysql/mysql-server) with development builds of our compiler and libraries in order to find and fix regressions before they would affect projects like yours. This also allows us to deliver advance notice of breaking changes, which is the case here.

The C++ Standardization Committee's Library Working Group recently resolved an issue, LWG-3865 "Sorting a range of pairs" (see https://cplusplus.github.io/LWG/issue3865 ), which changes how std::pair's comparison operators are defined. Such issues are considered "defect reports" and apply retroactively to all published Standards, so we're implementing this unconditionally in all of our Standard modes (C++14, C++17, C++20, C++23). Our C++ Standard Library is open source so you can see the PR for this here: https://github.com/microsoft/STL/pull/3476 . This code will ship in Visual Studio 2022 17.7 Preview 1 at some time in the future.

This updated specification and implementation of std::pair's comparison operators will break certain code in mysql-server. Specifically, there's an overloaded equality operator that's declared and defined on these lines:

https://github.com/mysql/mysql-server/blob/8.0/sql/auth/auth_internal.h#L258-L259
https://github.com/mysql/mysql-server/blob/8.0/sql/auth/sql_authorization.cc#L7448-L7449

The signature is:

bool operator==(const std::pair<const Role_id, const Role_id> &a,
                const Auth_id_ref &b)

where Auth_id_ref is a typedef for std::pair<LEX_CSTRING, LEX_CSTRING>.

Elsewhere in mysql-server, an equality comparison is being performed between a std::pair<const Role_id, Role_id> (NOTE the lack of const on the second element) and a std::pair<LEX_CSTRING, LEX_CSTRING>.

Before LWG-3865's resolution, std::pair's own equality operator was templated on (const pair<T1, T2>&, const pair<T1, T2>&), i.e. it took the same types on both sides. This wasn't viable during overload resolution (no template arguments T1 and T2 could be deduced, because they can't simultaneously be Role_id and LEX_CSTRING). This left mysql-server's overloaded equality operator, which was viable with a conversion. A temporary pair<const Role_id, const Role_id> would be constructed from the pair<const Role_id, Role_id> and the comparison would be performed. That's why this code previously compiled.

After LWG-3865's resolution, std::pair's equality operator now accepts different types: (const pair<T1, T2>&, const pair<U1, U2>&). Now, this is viable during overload resolution, and it is an exact match (better than performing an implicit conversion), so the compiler selects it. Then, the types Role_id and LEX_CSTRING aren't actually equality-comparable themselves, so a compiler error is emitted.

There's a simple way to fix this. The declaration and the definition of mysql-server's operator here should be changed to omit the const for the second Role_id:

OLD CODE:
bool operator==(const std::pair<const Role_id, const Role_id> &a,
                const Auth_id_ref &b)

NEW CODE:
bool operator==(const std::pair<const Role_id, Role_id> &a,
                const Auth_id_ref &b)

With this change, the operator's signature will exactly match the element type that mysql-server uses elsewhere (I suspect that this pair type was ultimately emitted by a map, which uses pair<const K, V> as its value type). The C++ Standard has a tiebreaker rule that prefers non-templates to templates when both are exact matches, so mysql-server's updated operator will be selected instead of the Standard's updated operator, and the code will compile again. (Actually better than before, since no implicit conversion will happen.)

We have verified that this edit fixes the code in our test harness. Also, note that this change is backwards-compatible - it will work with all other compilers and versions, regardless of whether they've implemented this LWG issue resolution.

We wanted to let you know now, far in advance of Visual Studio 2022 17.7 shipping. Hope this helps!

How to repeat:
Compile mysql-server with Visual Studio 2022 17.7 Preview 1 (not yet released) or the microsoft/STL open source library. A compiler error will be emitted (I have snipped a long list of operator candidates for brevity; the remaining error context actually tells the story of what happened in a precise way, although it looks extremely complicated):

F:\Agent\_work\3\s\binaries\diff\amd64chk\inc\utility(485): error C2678: binary '==': no operator found which takes a left-hand operand of type '_Ty1' (or there is no acceptable conversion)
        with
        [
            _Ty1=const Role_id
        ]
[...]
F:\Agent\_work\3\s\binaries\diff\amd64chk\inc\utility(485): note: while trying to match the argument list '(_Ty1, const _Ty1)'
        with
        [
            _Ty1=const Role_id
        ]
        and
        [
            _Ty1=LEX_CSTRING
        ]
F:\Agent\_work\3\s\binaries\diff\amd64chk\inc\xutility(5767): note: see reference to function template instantiation 'bool std::operator ==<const Role_id,Role_id,LEX_CSTRING,LEX_CSTRING>(const std::pair<const Role_id,Role_id> &,const std::pair<LEX_CSTRING,LEX_CSTRING> &)' being compiled
F:\Agent\_work\3\s\binaries\diff\amd64chk\inc\xutility(5781): note: see reference to function template instantiation '_InIt std::_Find_unchecked<std::_List_unchecked_iterator<std::_List_val<std::_List_simple_types<_Ty>>>,std::pair<LEX_CSTRING,LEX_CSTRING>>(_InIt,const _InIt,const std::pair<LEX_CSTRING,LEX_CSTRING> &)' being compiled
        with
        [
            _InIt=std::_List_unchecked_iterator<std::_List_val<std::_List_simple_types<std::pair<const Role_id,Role_id>>>>,
            _Ty=std::pair<const Role_id,Role_id>
        ]
F:\gitP\mysql\mysql-server\sql\auth\sql_authorization.cc(6587): note: see reference to function template instantiation '_InIt std::find<std::_List_iterator<std::_List_val<std::_List_simple_types<_Ty>>>,std::pair<LEX_CSTRING,LEX_CSTRING>>(_InIt,const _InIt,const std::pair<LEX_CSTRING,LEX_CSTRING> &)' being compiled
        with
        [
            _InIt=std::_List_iterator<std::_List_val<std::_List_simple_types<std::pair<const Role_id,Role_id>>>>,
            _Ty=std::pair<const Role_id,Role_id>
        ]

Suggested fix:
Change the signatures in these two files:

https://github.com/mysql/mysql-server/blob/8.0/sql/auth/auth_internal.h#L258-L259

https://github.com/mysql/mysql-server/blob/8.0/sql/auth/sql_authorization.cc#L7448-L7449

from:

bool operator==(const std::pair<const Role_id, const Role_id> &a,
                const Auth_id_ref &b)

to:

bool operator==(const std::pair<const Role_id, Role_id> &a,
                const Auth_id_ref &b)
[2 Mar 2023 4:40] MySQL Verification Team
Hello Stephan,

Thank you very much for your report and feedback, we appreciate it!

Sincerely,
Umesh
[7 Aug 2023 22:17] Jon Stephens
Documented fix as follows in the MySQL 8.2.0 changelog:

    The C++ Standardization Committee's Library Working Group
    recently resolved an issue, LWG-3865 "Sorting a range of pairs"
    (see https://cplusplus.github.io/LWG/issue3865), which changes
    how the comparison operators are defined are defined for
    std::pair. This fix updates the equality operator used in two
    files in sql/auth to align with this change.

    Based on a suggestion by the Microsoft Visual Studio team.

Closed.
[24 Aug 2023 20:08] Jon Stephens
Also fixed in MySQL 8.0.35.

Closed.