Skip to content
Permalink
Browse files

MDEV-12358 Work around what looks like a bug in GCC 7.1.0

The parameter thr of the function btr_cur_optimistic_insert()
is not declared as nonnull, but GCC 7.1.0 with -O3 is wrongly
optimizing away the first part of the condition
UNIV_UNLIKELY(thr && thr_get_trx(thr)->fake_changes)
when the function is being called by row_merge_insert_index_tuples()
with thr==NULL.

The fake_changes is an XtraDB addition. This GCC bug only appears
to have an impact on XtraDB, not InnoDB.

We work around the problem by not attempting to dereference thr
when both BTR_NO_LOCKING_FLAG and BTR_NO_UNDO_LOG_FLAG are set
in the flags. Probably BTR_NO_LOCKING_FLAG alone should suffice.

btr_cur_optimistic_insert(), btr_cur_pessimistic_insert(),
btr_cur_pessimistic_update(): Correct comments that disagree with
usage and with nonnull attributes. No other parameter than thr can
actually be NULL.

row_ins_duplicate_error_in_clust(): Remove an unused parameter.

innobase_is_fake_change(): Unused function; remove.

ibuf_insert_low(), row_log_table_apply(), row_log_apply(),
row_undo_mod_clust_low():
Because we will be passing BTR_NO_LOCKING_FLAG | BTR_NO_UNDO_LOG_FLAG
in the flags, the trx->fake_changes flag will be treated as false,
which is the right thing to do at these low-level operations
(change buffer merge, ALTER TABLE…LOCK=NONE, or ROLLBACK).
This might be fixing actual XtraDB bugs.

Other callers that pass these two flags are also passing thr=NULL,
implying fake_changes=false. (Some callers in ROLLBACK are passing
BTR_NO_LOCKING_FLAG and a nonnull thr. In these callers, fake_changes
better be false, to avoid corruption.)
  • Loading branch information...
dr-m committed May 17, 2017
1 parent e22d86a commit 9f89b94ba6d69f896b2af3e84a9b420131c530d8
@@ -3,6 +3,7 @@
Copyright (c) 1994, 2016, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2008, Google Inc.
Copyright (c) 2012, Facebook Inc.
Copyright (c) 2017, MariaDB Corporation.
Portions of this file contain modifications contributed and copyrighted by
Google, Inc. Those modifications are gratefully acknowledged and are described
@@ -1223,15 +1224,17 @@ btr_cur_optimistic_insert(
btr_cur_t* cursor, /*!< in: cursor on page after which to insert;
cursor stays valid */
ulint** offsets,/*!< out: offsets on *rec */
mem_heap_t** heap, /*!< in/out: pointer to memory heap, or NULL */
mem_heap_t** heap, /*!< in/out: pointer to memory heap */
dtuple_t* entry, /*!< in/out: entry to insert */
rec_t** rec, /*!< out: pointer to inserted record if
succeed */
big_rec_t** big_rec,/*!< out: big rec vector whose fields have to
be stored externally by the caller, or
NULL */
be stored externally by the caller */
ulint n_ext, /*!< in: number of externally stored columns */
que_thr_t* thr, /*!< in: query thread or NULL */
que_thr_t* thr, /*!< in/out: query thread; can be NULL if
!(~flags
& (BTR_NO_LOCKING_FLAG
| BTR_NO_UNDO_LOG_FLAG)) */
mtr_t* mtr) /*!< in/out: mini-transaction;
if this function returns DB_SUCCESS on
a leaf page of a secondary index in a
@@ -1252,6 +1255,7 @@ btr_cur_optimistic_insert(
ulint rec_size;
dberr_t err;

ut_ad(thr || !(~flags & (BTR_NO_LOCKING_FLAG | BTR_NO_UNDO_LOG_FLAG)));
*big_rec = NULL;

block = btr_cur_get_block(cursor);
@@ -1510,15 +1514,17 @@ btr_cur_pessimistic_insert(
cursor stays valid */
ulint** offsets,/*!< out: offsets on *rec */
mem_heap_t** heap, /*!< in/out: pointer to memory heap
that can be emptied, or NULL */
that can be emptied */
dtuple_t* entry, /*!< in/out: entry to insert */
rec_t** rec, /*!< out: pointer to inserted record if
succeed */
big_rec_t** big_rec,/*!< out: big rec vector whose fields have to
be stored externally by the caller, or
NULL */
be stored externally by the caller */
ulint n_ext, /*!< in: number of externally stored columns */
que_thr_t* thr, /*!< in: query thread or NULL */
que_thr_t* thr, /*!< in/out: query thread; can be NULL if
!(~flags
& (BTR_NO_LOCKING_FLAG
| BTR_NO_UNDO_LOG_FLAG)) */
mtr_t* mtr) /*!< in/out: mini-transaction */
{
dict_index_t* index = cursor->index;
@@ -1530,6 +1536,7 @@ btr_cur_pessimistic_insert(
ulint n_reserved = 0;

ut_ad(dtuple_check_typed(entry));
ut_ad(thr || !(~flags & (BTR_NO_LOCKING_FLAG | BTR_NO_UNDO_LOG_FLAG)));

*big_rec = NULL;

@@ -2426,12 +2433,12 @@ btr_cur_pessimistic_update(
ulint** offsets,/*!< out: offsets on cursor->page_cur.rec */
mem_heap_t** offsets_heap,
/*!< in/out: pointer to memory heap
that can be emptied, or NULL */
that can be emptied */
mem_heap_t* entry_heap,
/*!< in/out: memory heap for allocating
big_rec and the index tuple */
big_rec_t** big_rec,/*!< out: big rec vector whose fields have to
be stored externally by the caller, or NULL */
be stored externally by the caller */
const upd_t* update, /*!< in: update vector; this is allowed also
contain trx id and roll ptr fields, but
the values in update vector have no effect */
@@ -1,6 +1,7 @@
/*****************************************************************************
Copyright (c) 1997, 2016, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2017, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
@@ -3662,7 +3663,7 @@ ibuf_insert_low(

if (mode == BTR_MODIFY_PREV) {
err = btr_cur_optimistic_insert(
BTR_NO_LOCKING_FLAG,
BTR_NO_LOCKING_FLAG | BTR_NO_UNDO_LOG_FLAG,
cursor, &offsets, &offsets_heap,
ibuf_entry, &ins_rec,
&dummy_big_rec, 0, thr, &mtr);
@@ -1,6 +1,7 @@
/*****************************************************************************
Copyright (c) 1994, 2016, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2017, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
@@ -220,15 +221,17 @@ btr_cur_optimistic_insert(
btr_cur_t* cursor, /*!< in: cursor on page after which to insert;
cursor stays valid */
ulint** offsets,/*!< out: offsets on *rec */
mem_heap_t** heap, /*!< in/out: pointer to memory heap, or NULL */
mem_heap_t** heap, /*!< in/out: pointer to memory heap */
dtuple_t* entry, /*!< in/out: entry to insert */
rec_t** rec, /*!< out: pointer to inserted record if
succeed */
big_rec_t** big_rec,/*!< out: big rec vector whose fields have to
be stored externally by the caller, or
NULL */
be stored externally by the caller */
ulint n_ext, /*!< in: number of externally stored columns */
que_thr_t* thr, /*!< in: query thread or NULL */
que_thr_t* thr, /*!< in/out: query thread; can be NULL if
!(~flags
& (BTR_NO_LOCKING_FLAG
| BTR_NO_UNDO_LOG_FLAG)) */
mtr_t* mtr) /*!< in/out: mini-transaction;
if this function returns DB_SUCCESS on
a leaf page of a secondary index in a
@@ -256,15 +259,17 @@ btr_cur_pessimistic_insert(
cursor stays valid */
ulint** offsets,/*!< out: offsets on *rec */
mem_heap_t** heap, /*!< in/out: pointer to memory heap
that can be emptied, or NULL */
that can be emptied */
dtuple_t* entry, /*!< in/out: entry to insert */
rec_t** rec, /*!< out: pointer to inserted record if
succeed */
big_rec_t** big_rec,/*!< out: big rec vector whose fields have to
be stored externally by the caller, or
NULL */
be stored externally by the caller */
ulint n_ext, /*!< in: number of externally stored columns */
que_thr_t* thr, /*!< in: query thread or NULL */
que_thr_t* thr, /*!< in/out: query thread; can be NULL if
!(~flags
& (BTR_NO_LOCKING_FLAG
| BTR_NO_UNDO_LOG_FLAG)) */
mtr_t* mtr) /*!< in/out: mini-transaction */
MY_ATTRIBUTE((nonnull(2,3,4,5,6,7,10), warn_unused_result));
/*************************************************************//**
@@ -390,12 +395,12 @@ btr_cur_pessimistic_update(
ulint** offsets,/*!< out: offsets on cursor->page_cur.rec */
mem_heap_t** offsets_heap,
/*!< in/out: pointer to memory heap
that can be emptied, or NULL */
that can be emptied */
mem_heap_t* entry_heap,
/*!< in/out: memory heap for allocating
big_rec and the index tuple */
big_rec_t** big_rec,/*!< out: big rec vector whose fields have to
be stored externally by the caller, or NULL */
be stored externally by the caller */
const upd_t* update, /*!< in: update vector; this is allowed also
contain trx id and roll ptr fields, but
the values in update vector have no effect */
@@ -1,6 +1,7 @@
/*****************************************************************************
Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2017, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
@@ -2121,14 +2122,10 @@ for a clustered index!
@retval DB_SUCCESS if no error
@retval DB_DUPLICATE_KEY if error,
@retval DB_LOCK_WAIT if we have to wait for a lock on a possible duplicate
record
@retval DB_SUCCESS_LOCKED_REC if an exact match of the record was found
in online table rebuild (flags & (BTR_KEEP_SYS_FLAG | BTR_NO_LOCKING_FLAG)) */
record */
static MY_ATTRIBUTE((nonnull, warn_unused_result))
dberr_t
row_ins_duplicate_error_in_clust(
/*=============================*/
ulint flags, /*!< in: undo logging and locking flags */
btr_cur_t* cursor, /*!< in: B-tree cursor */
const dtuple_t* entry, /*!< in: entry to insert */
que_thr_t* thr, /*!< in: query thread */
@@ -2386,7 +2383,7 @@ row_ins_clust_index_entry_low(
DB_LOCK_WAIT */

err = row_ins_duplicate_error_in_clust(
flags, &cursor, entry, thr, &mtr);
&cursor, entry, thr, &mtr);
}

if (err != DB_SUCCESS) {
@@ -3,6 +3,7 @@
Copyright (c) 1994, 2016, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2008, Google Inc.
Copyright (c) 2012, Facebook Inc.
Copyright (c) 2017, MariaDB Corporation.
Portions of this file contain modifications contributed and copyrighted by
Google, Inc. Those modifications are gratefully acknowledged and are described
@@ -1328,15 +1329,17 @@ btr_cur_optimistic_insert(
btr_cur_t* cursor, /*!< in: cursor on page after which to insert;
cursor stays valid */
ulint** offsets,/*!< out: offsets on *rec */
mem_heap_t** heap, /*!< in/out: pointer to memory heap, or NULL */
mem_heap_t** heap, /*!< in/out: pointer to memory heap */
dtuple_t* entry, /*!< in/out: entry to insert */
rec_t** rec, /*!< out: pointer to inserted record if
succeed */
big_rec_t** big_rec,/*!< out: big rec vector whose fields have to
be stored externally by the caller, or
NULL */
be stored externally by the caller */
ulint n_ext, /*!< in: number of externally stored columns */
que_thr_t* thr, /*!< in: query thread or NULL */
que_thr_t* thr, /*!< in/out: query thread; can be NULL if
!(~flags
& (BTR_NO_LOCKING_FLAG
| BTR_NO_UNDO_LOG_FLAG)) */
mtr_t* mtr) /*!< in/out: mini-transaction;
if this function returns DB_SUCCESS on
a leaf page of a secondary index in a
@@ -1357,6 +1360,7 @@ btr_cur_optimistic_insert(
ulint rec_size;
dberr_t err;

ut_ad(thr || !(~flags & (BTR_NO_LOCKING_FLAG | BTR_NO_UNDO_LOG_FLAG)));
*big_rec = NULL;

block = btr_cur_get_block(cursor);
@@ -1366,7 +1370,10 @@ btr_cur_optimistic_insert(
page = buf_block_get_frame(block);
index = cursor->index;

ut_ad((thr && thr_get_trx(thr)->fake_changes)
const bool fake_changes = (~flags & (BTR_NO_LOCKING_FLAG
| BTR_NO_UNDO_LOG_FLAG))
&& thr_get_trx(thr)->fake_changes;
ut_ad(fake_changes
|| mtr_memo_contains(mtr, block, MTR_MEMO_PAGE_X_FIX));
ut_ad(!dict_index_is_online_ddl(index)
|| dict_index_is_clust(index)
@@ -1507,7 +1514,7 @@ btr_cur_optimistic_insert(
goto fail_err;
}

if (UNIV_UNLIKELY(thr && thr_get_trx(thr)->fake_changes)) {
if (UNIV_UNLIKELY(fake_changes)) {
/* skip CHANGE, LOG */
*big_rec = big_rec_vec;
return(err); /* == DB_SUCCESS */
@@ -1625,15 +1632,17 @@ btr_cur_pessimistic_insert(
cursor stays valid */
ulint** offsets,/*!< out: offsets on *rec */
mem_heap_t** heap, /*!< in/out: pointer to memory heap
that can be emptied, or NULL */
that can be emptied */
dtuple_t* entry, /*!< in/out: entry to insert */
rec_t** rec, /*!< out: pointer to inserted record if
succeed */
big_rec_t** big_rec,/*!< out: big rec vector whose fields have to
be stored externally by the caller, or
NULL */
be stored externally by the caller */
ulint n_ext, /*!< in: number of externally stored columns */
que_thr_t* thr, /*!< in: query thread or NULL */
que_thr_t* thr, /*!< in/out: query thread; can be NULL if
!(~flags
& (BTR_NO_LOCKING_FLAG
| BTR_NO_UNDO_LOG_FLAG)) */
mtr_t* mtr) /*!< in/out: mini-transaction */
{
dict_index_t* index = cursor->index;
@@ -1645,13 +1654,17 @@ btr_cur_pessimistic_insert(
ulint n_reserved = 0;

ut_ad(dtuple_check_typed(entry));
ut_ad(thr || !(~flags & (BTR_NO_LOCKING_FLAG | BTR_NO_UNDO_LOG_FLAG)));

*big_rec = NULL;

ut_ad((thr && thr_get_trx(thr)->fake_changes) || mtr_memo_contains(mtr,
const bool fake_changes = (~flags & (BTR_NO_LOCKING_FLAG
| BTR_NO_UNDO_LOG_FLAG))
&& thr_get_trx(thr)->fake_changes;
ut_ad(fake_changes || mtr_memo_contains(mtr,
dict_index_get_lock(btr_cur_get_index(cursor)),
MTR_MEMO_X_LOCK));
ut_ad((thr && thr_get_trx(thr)->fake_changes) || mtr_memo_contains(mtr, btr_cur_get_block(cursor),
ut_ad(fake_changes || mtr_memo_contains(mtr, btr_cur_get_block(cursor),
MTR_MEMO_PAGE_X_FIX));
ut_ad(!dict_index_is_online_ddl(index)
|| dict_index_is_clust(index)
@@ -1712,7 +1725,7 @@ btr_cur_pessimistic_insert(
}
}

if (UNIV_UNLIKELY(thr && thr_get_trx(thr)->fake_changes)) {
if (UNIV_UNLIKELY(fake_changes)) {
/* skip CHANGE, LOG */
if (n_reserved > 0) {
fil_space_release_free_extents(index->space,
@@ -1803,7 +1816,7 @@ btr_cur_upd_lock_and_undo(

ut_ad((thr != NULL) || (flags & BTR_NO_LOCKING_FLAG));

if (UNIV_UNLIKELY(thr && thr_get_trx(thr)->fake_changes)) {
if (!(flags & BTR_NO_LOCKING_FLAG) && thr_get_trx(thr)->fake_changes) {
/* skip LOCK, UNDO */
return(DB_SUCCESS);
}
@@ -2582,12 +2595,12 @@ btr_cur_pessimistic_update(
ulint** offsets,/*!< out: offsets on cursor->page_cur.rec */
mem_heap_t** offsets_heap,
/*!< in/out: pointer to memory heap
that can be emptied, or NULL */
that can be emptied */
mem_heap_t* entry_heap,
/*!< in/out: memory heap for allocating
big_rec and the index tuple */
big_rec_t** big_rec,/*!< out: big rec vector whose fields have to
be stored externally by the caller, or NULL */
be stored externally by the caller */
const upd_t* update, /*!< in: update vector; this is allowed also
contain trx id and roll ptr fields, but
the values in update vector have no effect */
@@ -769,17 +769,6 @@ innobase_purge_changed_page_bitmaps(
ulonglong lsn) __attribute__((unused)); /*!< in: LSN to purge files up to */


/*****************************************************************//**
Check whether this is a fake change transaction.
@return TRUE if a fake change transaction */
static
my_bool
innobase_is_fake_change(
/*====================*/
handlerton *hton, /*!< in: InnoDB handlerton */
THD* thd) __attribute__((unused)); /*!< in: MySQL thread handle of the user for
whom the transaction is being committed */

/** Get the list of foreign keys referencing a specified table
table.
@param thd The thread handle
@@ -4242,22 +4231,6 @@ innobase_purge_changed_page_bitmaps(
return (my_bool)log_online_purge_changed_page_bitmaps(lsn);
}

/*****************************************************************//**
Check whether this is a fake change transaction.
@return TRUE if a fake change transaction */
static
my_bool
innobase_is_fake_change(
/*====================*/
handlerton *hton MY_ATTRIBUTE((unused)),
/*!< in: InnoDB handlerton */
THD* thd) /*!< in: MySQL thread handle of the user for
whom the transaction is being committed */
{
trx_t* trx = check_trx_exists(thd);
return UNIV_UNLIKELY(trx->fake_changes);
}

/*****************************************************************//**
Commits a transaction in an InnoDB database. */
static
@@ -1,6 +1,7 @@
/*****************************************************************************
Copyright (c) 1997, 2016, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2017, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
@@ -3704,7 +3705,7 @@ ibuf_insert_low(

if (mode == BTR_MODIFY_PREV) {
err = btr_cur_optimistic_insert(
BTR_NO_LOCKING_FLAG,
BTR_NO_LOCKING_FLAG | BTR_NO_UNDO_LOG_FLAG,
cursor, &offsets, &offsets_heap,
ibuf_entry, &ins_rec,
&dummy_big_rec, 0, thr, &mtr);

0 comments on commit 9f89b94

Please sign in to comment.
You can’t perform that action at this time.