diff --git a/CHANGES b/CHANGES index 86ff535f29..6faa12370c 100644 --- a/CHANGES +++ b/CHANGES @@ -22,6 +22,10 @@ message that was emitted even when the feature was explicitly disabled. [GL #4497] +6300. [bug] Fix statistics export to use full 64 bit signed numbers + instead of truncating values to unsigned 32 bits. + [GL #4467] + 6299. [port] NetBSD has added 'hmac' to libc which collides with our use of 'hmac'. [GL #4478] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index a47401186f..be1bc21a15 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -35,7 +35,9 @@ Feature Changes Bug Fixes ~~~~~~~~~ -- None. +- Fix statistics export to use full 64 bit signed numbers instead of truncating + values to unsigned 32 bits. Export was truncating values since BIND 9.15.0. + :gl:`#4467` Known Issues ~~~~~~~~~~~~ diff --git a/lib/isc/include/isc/types.h b/lib/isc/include/isc/types.h index 7e0fc021df..dc76a87610 100644 --- a/lib/isc/include/isc/types.h +++ b/lib/isc/include/isc/types.h @@ -13,6 +13,7 @@ #pragma once +#include #include /*! \file isc/types.h @@ -74,10 +75,11 @@ typedef struct isc_rwlock isc_rwlock_t; /*%< Read Write Lock */ typedef struct isc_sockaddr isc_sockaddr_t; /*%< Socket Address */ typedef ISC_LIST(isc_sockaddr_t) isc_sockaddrlist_t; /*%< Socket Address List * */ -typedef struct isc_stats isc_stats_t; /*%< Statistics */ -typedef int_fast64_t isc_statscounter_t; -typedef struct isc_symtab isc_symtab_t; /*%< Symbol Table */ -typedef struct isc_task isc_task_t; /*%< Task */ +typedef struct isc_stats isc_stats_t; /*%< Statistics */ +typedef int_fast64_t isc_statscounter_t; +typedef atomic_int_fast64_t isc_atomic_statscounter_t; +typedef struct isc_symtab isc_symtab_t; /*%< Symbol Table */ +typedef struct isc_task isc_task_t; /*%< Task */ typedef ISC_LIST(isc_task_t) isc_tasklist_t; /*%< Task List */ typedef struct isc_taskmgr isc_taskmgr_t; /*%< Task Manager */ typedef struct isc_textregion isc_textregion_t; /*%< Text Region */ diff --git a/lib/isc/stats.c b/lib/isc/stats.c index 3e4676c2ac..183030e4f8 100644 --- a/lib/isc/stats.c +++ b/lib/isc/stats.c @@ -28,14 +28,22 @@ #define ISC_STATS_MAGIC ISC_MAGIC('S', 't', 'a', 't') #define ISC_STATS_VALID(x) ISC_MAGIC_VALID(x, ISC_STATS_MAGIC) -typedef atomic_int_fast64_t isc__atomic_statcounter_t; +/* + * Statistics are counted with an atomic int_fast64_t but exported to functions + * taking uint64_t (isc_stats_dumper_t). A 128-bit native and fast architecture + * doesn't exist in reality so these two are the same thing in practise. + * However, a silent truncation happening silently in the future is still not + * acceptable. + */ +STATIC_ASSERT(sizeof(isc_statscounter_t) <= sizeof(uint64_t), + "Exported statistics must fit into the statistic counter size"); struct isc_stats { unsigned int magic; isc_mem_t *mctx; isc_refcount_t references; int ncounters; - isc__atomic_statcounter_t *counters; + isc_atomic_statscounter_t *counters; }; static isc_result_t @@ -46,7 +54,7 @@ create_stats(isc_mem_t *mctx, int ncounters, isc_stats_t **statsp) { REQUIRE(statsp != NULL && *statsp == NULL); stats = isc_mem_get(mctx, sizeof(*stats)); - counters_alloc_size = sizeof(isc__atomic_statcounter_t) * ncounters; + counters_alloc_size = sizeof(isc_atomic_statscounter_t) * ncounters; stats->counters = isc_mem_get(mctx, counters_alloc_size); isc_refcount_init(&stats->references, 1); for (int i = 0; i < ncounters; i++) { @@ -82,7 +90,7 @@ isc_stats_detach(isc_stats_t **statsp) { if (isc_refcount_decrement(&stats->references) == 1) { isc_refcount_destroy(&stats->references); isc_mem_put(stats->mctx, stats->counters, - sizeof(isc__atomic_statcounter_t) * + sizeof(isc_atomic_statscounter_t) * stats->ncounters); isc_mem_putanddetach(&stats->mctx, stats, sizeof(*stats)); } @@ -125,7 +133,8 @@ isc_stats_dump(isc_stats_t *stats, isc_stats_dumper_t dump_fn, void *arg, REQUIRE(ISC_STATS_VALID(stats)); for (i = 0; i < stats->ncounters; i++) { - uint32_t counter = atomic_load_acquire(&stats->counters[i]); + isc_statscounter_t counter = + atomic_load_acquire(&stats->counters[i]); if ((options & ISC_STATSDUMP_VERBOSE) == 0 && counter == 0) { continue; } @@ -169,7 +178,7 @@ void isc_stats_resize(isc_stats_t **statsp, int ncounters) { isc_stats_t *stats; size_t counters_alloc_size; - isc__atomic_statcounter_t *newcounters; + isc_atomic_statscounter_t *newcounters; REQUIRE(statsp != NULL && *statsp != NULL); REQUIRE(ISC_STATS_VALID(*statsp)); @@ -182,7 +191,7 @@ isc_stats_resize(isc_stats_t **statsp, int ncounters) { } /* Grow number of counters. */ - counters_alloc_size = sizeof(isc__atomic_statcounter_t) * ncounters; + counters_alloc_size = sizeof(isc_atomic_statscounter_t) * ncounters; newcounters = isc_mem_get(stats->mctx, counters_alloc_size); for (int i = 0; i < ncounters; i++) { atomic_init(&newcounters[i], 0); @@ -192,7 +201,7 @@ isc_stats_resize(isc_stats_t **statsp, int ncounters) { atomic_store_release(&newcounters[i], counter); } isc_mem_put(stats->mctx, stats->counters, - sizeof(isc__atomic_statcounter_t) * stats->ncounters); + sizeof(isc_atomic_statscounter_t) * stats->ncounters); stats->counters = newcounters; stats->ncounters = ncounters; }