From e15079ec68522ec896ee98f7bacf25e8a0e4d9e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9B=D0=B5=D0=BE=D0=BD=D0=B8=D0=B4=20=D0=AE=D1=80=D1=8C?= =?UTF-8?q?=D0=B5=D0=B2=20=28Leonid=20Yuriev=29?= Date: Tue, 17 Dec 2024 22:00:33 +0300 Subject: [PATCH] =?UTF-8?q?mdbx:=20=D0=B8=D0=B7=D0=BC=D0=B5=D0=BD=D0=B5?= =?UTF-8?q?=D0=BD=D0=B8=D0=B5=20`log=5Fif=5Ferror()`=20=D1=80=D0=B0=D0=B4?= =?UTF-8?q?=D0=B8=20=D1=83=D1=81=D1=82=D1=80=D0=B0=D0=BD=D0=B5=D0=BD=D0=B8?= =?UTF-8?q?=D1=8F=20=D0=BB=D0=BE=D0=B6=D0=BD=D1=8B=D1=85=20"may=20be=20use?= =?UTF-8?q?d=20uninitialized"=20=D0=BF=D1=80=D0=B5=D0=B4=D1=83=D0=BF=D1=80?= =?UTF-8?q?=D0=B5=D0=B6=D0=B4=D0=B5=D0=BD=D0=B8=D0=B9=20=D0=B2=20LTO-?= =?UTF-8?q?=D1=81=D0=B1=D0=BE=D1=80=D0=BA=D0=B0=D1=85.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit При включении LTO анализатор путей выполнения внутри GCC начинает укачивать из-за выражений вида `return LOG_IFERR(MDBX_EINVAL);` Проблема в том, что несмотря на __builtin_assume() и __builtin_unreachable(), комплятор не хочет видеть что функция log_if_error() всегда возвращает получаемое значение. А если допустить что значение будет изменено, то вместо ошибки может быть MDBX_SUCCESS, и тогда в вызывающем как-бы может произойти обращение к неинициализированным данным, что и беспокоит компилятор. Например, при сборке mdbx_load: ‘txn_info.txn_space_dirty’ may be used uninitialized [-Wmaybe-uninitialized] Проэтому проще пойти анализатору навстречу и упростить исходный код. Теперь код ошибки явно пробрасывается через тело inline-функции, но это требует 1-2 дополнительных процессорных инструкции на каждое применение макроса LOG_IFERROR. Также здесь откатывается коммит 81a8127084d9a6a7777bb375e029062330e51979. --- src/api-cold.c | 16 ++++++-------- src/api-dbi.c | 48 +++++++++++++++-------------------------- src/api-misc.c | 10 +++------ src/api-txn-data.c | 9 ++++---- src/logging_and_debug.c | 3 +-- src/logging_and_debug.h | 23 ++++---------------- 6 files changed, 37 insertions(+), 72 deletions(-) diff --git a/src/api-cold.c b/src/api-cold.c index dfa082db..7498d8ab 100644 --- a/src/api-cold.c +++ b/src/api-cold.c @@ -361,17 +361,15 @@ __cold int mdbx_env_set_flags(MDBX_env *env, MDBX_env_flags_t flags, bool onoff) return MDBX_SUCCESS; } -__cold int mdbx_env_get_flags(const MDBX_env *env, unsigned *arg) { - if (unlikely(!arg)) +__cold int mdbx_env_get_flags(const MDBX_env *env, unsigned *flags) { + int rc = check_env(env, false); + if (unlikely(rc != MDBX_SUCCESS)) + return LOG_IFERR(rc); + + if (unlikely(!flags)) return LOG_IFERR(MDBX_EINVAL); - int rc = check_env(env, false); - if (unlikely(rc != MDBX_SUCCESS)) { - *arg = 0; - return LOG_IFERR(rc); - } - - *arg = env->flags & ENV_USABLE_FLAGS; + *flags = env->flags & ENV_USABLE_FLAGS; return MDBX_SUCCESS; } diff --git a/src/api-dbi.c b/src/api-dbi.c index 5f52d770..a102f8b6 100644 --- a/src/api-dbi.c +++ b/src/api-dbi.c @@ -197,22 +197,16 @@ int mdbx_dbi_close(MDBX_env *env, MDBX_dbi dbi) { } int mdbx_dbi_flags_ex(const MDBX_txn *txn, MDBX_dbi dbi, unsigned *flags, unsigned *state) { - if (unlikely(!flags || !state)) - return LOG_IFERR(MDBX_EINVAL); - int rc = check_txn(txn, MDBX_TXN_BLOCKED - MDBX_TXN_ERROR - MDBX_TXN_PARKED); - if (unlikely(rc != MDBX_SUCCESS)) { - *flags = 0; - *state = 0; + if (unlikely(rc != MDBX_SUCCESS)) return LOG_IFERR(rc); - } rc = dbi_check(txn, dbi); - if (unlikely(rc != MDBX_SUCCESS)) { - *flags = 0; - *state = 0; + if (unlikely(rc != MDBX_SUCCESS)) return LOG_IFERR(rc); - } + + if (unlikely(!flags || !state)) + return LOG_IFERR(MDBX_EINVAL); *flags = txn->dbs[dbi].flags & DB_PERSISTENT_FLAGS; *state = txn->dbi_state[dbi] & (DBI_FRESH | DBI_CREAT | DBI_DIRTY | DBI_STALE); @@ -230,41 +224,33 @@ static void stat_get(const tree_t *db, MDBX_stat *st, size_t bytes) { } __cold int mdbx_dbi_stat(const MDBX_txn *txn, MDBX_dbi dbi, MDBX_stat *dest, size_t bytes) { - if (unlikely(!dest)) - return LOG_IFERR(MDBX_EINVAL); - int rc = check_txn(txn, MDBX_TXN_BLOCKED); if (unlikely(rc != MDBX_SUCCESS)) - goto bailout; + return LOG_IFERR(rc); rc = dbi_check(txn, dbi); if (unlikely(rc != MDBX_SUCCESS)) - goto bailout; + return LOG_IFERR(rc); - const size_t size_before_modtxnid = offsetof(MDBX_stat, ms_mod_txnid); - if (unlikely(bytes != sizeof(MDBX_stat)) && bytes != size_before_modtxnid) { - rc = MDBX_EINVAL; - goto bailout; - } - - if (unlikely(txn->flags & MDBX_TXN_BLOCKED)) { - rc = MDBX_BAD_TXN; - goto bailout; - } + if (unlikely(txn->flags & MDBX_TXN_BLOCKED)) + return LOG_IFERR(MDBX_BAD_TXN); if (unlikely(txn->dbi_state[dbi] & DBI_STALE)) { rc = tbl_fetch((MDBX_txn *)txn, dbi); if (unlikely(rc != MDBX_SUCCESS)) - goto bailout; + return LOG_IFERR(rc); } + if (unlikely(!dest)) + return LOG_IFERR(MDBX_EINVAL); + + const size_t size_before_modtxnid = offsetof(MDBX_stat, ms_mod_txnid); + if (unlikely(bytes != sizeof(MDBX_stat)) && bytes != size_before_modtxnid) + return LOG_IFERR(MDBX_EINVAL); + dest->ms_psize = txn->env->ps; stat_get(&txn->dbs[dbi], dest, bytes); return MDBX_SUCCESS; - -bailout: - memset(dest, 0, bytes); - return LOG_IFERR(rc); } __cold int mdbx_enumerate_tables(const MDBX_txn *txn, MDBX_table_enum_func *func, void *ctx) { diff --git a/src/api-misc.c b/src/api-misc.c index ef3a172f..c34c9df7 100644 --- a/src/api-misc.c +++ b/src/api-misc.c @@ -29,21 +29,17 @@ __cold int mdbx_is_readahead_reasonable(size_t volume, intptr_t redundancy) { int mdbx_dbi_sequence(MDBX_txn *txn, MDBX_dbi dbi, uint64_t *result, uint64_t increment) { int rc = check_txn(txn, MDBX_TXN_BLOCKED); - if (unlikely(rc != MDBX_SUCCESS)) { - bailout: - if (likely(result)) - *result = ~UINT64_C(0); + if (unlikely(rc != MDBX_SUCCESS)) return LOG_IFERR(rc); - } rc = dbi_check(txn, dbi); if (unlikely(rc != MDBX_SUCCESS)) - goto bailout; + return LOG_IFERR(rc); if (unlikely(txn->dbi_state[dbi] & DBI_STALE)) { rc = tbl_fetch(txn, dbi); if (unlikely(rc != MDBX_SUCCESS)) - goto bailout; + return LOG_IFERR(rc); } tree_t *dbs = &txn->dbs[dbi]; diff --git a/src/api-txn-data.c b/src/api-txn-data.c index f12b06d3..b3cee3c3 100644 --- a/src/api-txn-data.c +++ b/src/api-txn-data.c @@ -6,8 +6,8 @@ __cold int mdbx_dbi_dupsort_depthmask(const MDBX_txn *txn, MDBX_dbi dbi, uint32_t *mask) { if (unlikely(!mask)) return LOG_IFERR(MDBX_EINVAL); - *mask = 0; + int rc = check_txn(txn, MDBX_TXN_BLOCKED); if (unlikely(rc != MDBX_SUCCESS)) return LOG_IFERR(rc); @@ -16,6 +16,7 @@ __cold int mdbx_dbi_dupsort_depthmask(const MDBX_txn *txn, MDBX_dbi dbi, uint32_ rc = cursor_init(&cx.outer, txn, dbi); if (unlikely(rc != MDBX_SUCCESS)) return LOG_IFERR(rc); + if ((cx.outer.tree->flags & MDBX_DUPSORT) == 0) return MDBX_RESULT_TRUE; @@ -50,15 +51,15 @@ __cold int mdbx_dbi_dupsort_depthmask(const MDBX_txn *txn, MDBX_dbi dbi, uint32_ } int mdbx_canary_get(const MDBX_txn *txn, MDBX_canary *canary) { - if (unlikely(canary == nullptr)) - return LOG_IFERR(MDBX_EINVAL); - int rc = check_txn(txn, MDBX_TXN_BLOCKED); if (unlikely(rc != MDBX_SUCCESS)) { memset(canary, 0, sizeof(*canary)); return LOG_IFERR(rc); } + if (unlikely(canary == nullptr)) + return LOG_IFERR(MDBX_EINVAL); + *canary = txn->canary; return MDBX_SUCCESS; } diff --git a/src/logging_and_debug.c b/src/logging_and_debug.c index e09f4dd6..f796db35 100644 --- a/src/logging_and_debug.c +++ b/src/logging_and_debug.c @@ -56,14 +56,13 @@ __cold void debug_log(int level, const char *function, int line, const char *fmt va_end(args); } -__cold int log_error(const int err, const char *func, unsigned line) { +__cold void log_error(const int err, const char *func, unsigned line) { assert(err != MDBX_SUCCESS); if (unlikely(globals.loglevel >= MDBX_LOG_DEBUG) && (globals.loglevel >= MDBX_LOG_TRACE || !(err == MDBX_RESULT_TRUE || err == MDBX_NOTFOUND))) { char buf[256]; debug_log(MDBX_LOG_ERROR, func, line, "error %d (%s)\n", err, mdbx_strerror_r(err, buf, sizeof(buf))); } - return err; } /* Dump a val in ascii or hexadecimal. */ diff --git a/src/logging_and_debug.h b/src/logging_and_debug.h index 9382eafc..e99ccea5 100644 --- a/src/logging_and_debug.h +++ b/src/logging_and_debug.h @@ -148,27 +148,12 @@ MDBX_INTERNAL const char *pagetype_caption(const uint8_t type, char buf4unknown[ #define DVAL_DEBUG(x) ("-") #endif -MDBX_INTERNAL int log_error(const int err, const char *func, unsigned line); +MDBX_INTERNAL void log_error(const int err, const char *func, unsigned line); MDBX_MAYBE_UNUSED static inline int log_if_error(const int err, const char *func, unsigned line) { - if (likely(err == MDBX_SUCCESS)) - return err; - int rc = log_error(err, func, line); -#if __has_c_attribute(assume) - [[assume(rc == err && rc != MDBX_SUCCESS)]]; -#endif -#if defined(__clang__) || __has_builtin(assume) - __builtin_assume(rc == err && rc != MDBX_SUCCESS); -#endif - if (rc != err || rc == MDBX_SUCCESS) { -#if defined(__GNUC__) - __builtin_unreachable(); -#elif defined(_MSC_VER) && !defined(__clang__) - __assume(0); -#endif - rc = err; - } - return rc; + if (unlikely(err != MDBX_SUCCESS)) + log_error(err, func, line); + return err; } #define LOG_IFERR(err) log_if_error((err), __func__, __LINE__)