Bug #74842 Incorrect attribute((nonnull)) for btr_cur_ins_lock_and_undo callees

Description:
lock_rec_insert_check_and_lock is attribute((nonnull)). But its thr arg might be NULL legitimately, inspect btr_insert_on_non_leaf_level_func calling btr_cur_pessimistic_insert.

This might be a potential cause for a too-aggressive compiler optimization. We might have seen this on Percona Server 5.6 built with GCC 4.9 on Ubuntu Utopic, 32 bits, still working to confirm this, but this is a bug in any case.

This also might be related to a change in 5.7.6 that documented to remove nonnull attributes altogether.

How to repeat:
Code inspection as described above. Or, add ut_ad(thr) to lock_rec_insert_check_and_lock, remove the nonnull attribute from its prototype so that the ut_ad is not compiled away, run MTR innodb.innodb test, observe the stacktrace where this function is called with a legal NULL arg.

Suggested fix:
Make the nonnull attribute not apply to the thr arg.

Bug 74842 patch for 5.6

(*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: bug74842.patch (application/octet-stream, text), 1.03 KiB.

The same applies to trx_undo_report_row_operation.

Bug 74842 patch for 5.6, including trx_undo_report_row_operation

(*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: bug74842.patch (application/octet-stream, text), 1.60 KiB.

Confirmed that fixing this bug fixed the too-aggressive optimisation we encountered.

I wonder why this bug is not verified yet? The thing that needs verifying is not a miscompilation (which is a potential consequence of this bug), but simply an incorrect attribute((nonnull)) in the source code.

Hello Laurynas,

Thank you for the report and contribution.

Thanks,
Umesh