From: Evan Hunt Date: Wed, 7 Sep 2022 06:47:19 +0000 (-0700) Subject: add assertions to isc_buffer macros X-Git-Tag: v9.16.34~15^2 X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=2fdaa100c15a531cdbd3d31e365471b462e1681b;p=thirdparty%2Fbind9.git add assertions to isc_buffer macros if ISC_BUFFER_USEINLINE is defined, then macros are used to implement isc_buffer primitives (isc_buffer_init(), isc_buffer_region(), etc). otherwise, functions are used. previously, only the functions had DbC assertions, which made it possible for coding errors to go undetected. this commit makes the macro versions enforce the same requirements. --- diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 496a2a00ed2..d565356d960 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -595,7 +595,8 @@ unit:gcc:alpine3.16:amd64: gcc:oraclelinux7:amd64: variables: CC: gcc - CFLAGS: "${CFLAGS_COMMON}" + # -Wno-address suppresses isc_buffer macro warnings + CFLAGS: "${CFLAGS_COMMON} -Wno-address" EXTRA_CONFIGURE: "--with-libidn2" <<: *oraclelinux_7_amd64_image <<: *build_job diff --git a/lib/isc/buffer.c b/lib/isc/buffer.c index 0e9b5cec160..b284840b0b5 100644 --- a/lib/isc/buffer.c +++ b/lib/isc/buffer.c @@ -30,9 +30,6 @@ isc__buffer_init(isc_buffer_t *b, void *base, unsigned int length) { * Make 'b' refer to the 'length'-byte region starting at 'base'. * XXXDCL see the comment in buffer.h about base being const. */ - - REQUIRE(b != NULL); - ISC__BUFFER_INIT(b, base, length); } @@ -42,7 +39,6 @@ isc__buffer_initnull(isc_buffer_t *b) { * Initialize a new buffer which has no backing store. This can * later be grown as needed and swapped in place. */ - ISC__BUFFER_INIT(b, NULL, 0); } @@ -74,11 +70,6 @@ isc__buffer_invalidate(isc_buffer_t *b) { /* * Make 'b' an invalid buffer. */ - - REQUIRE(ISC_BUFFER_VALID(b)); - REQUIRE(!ISC_LINK_LINKED(b, link)); - REQUIRE(b->mctx == NULL); - ISC__BUFFER_INVALIDATE(b); } @@ -94,10 +85,6 @@ isc__buffer_region(isc_buffer_t *b, isc_region_t *r) { /* * Make 'r' refer to the region of 'b'. */ - - REQUIRE(ISC_BUFFER_VALID(b)); - REQUIRE(r != NULL); - ISC__BUFFER_REGION(b, r); } @@ -106,10 +93,6 @@ isc__buffer_usedregion(const isc_buffer_t *b, isc_region_t *r) { /* * Make 'r' refer to the used region of 'b'. */ - - REQUIRE(ISC_BUFFER_VALID(b)); - REQUIRE(r != NULL); - ISC__BUFFER_USEDREGION(b, r); } @@ -118,10 +101,6 @@ isc__buffer_availableregion(isc_buffer_t *b, isc_region_t *r) { /* * Make 'r' refer to the available region of 'b'. */ - - REQUIRE(ISC_BUFFER_VALID(b)); - REQUIRE(r != NULL); - ISC__BUFFER_AVAILABLEREGION(b, r); } @@ -130,10 +109,6 @@ isc__buffer_add(isc_buffer_t *b, unsigned int n) { /* * Increase the 'used' region of 'b' by 'n' bytes. */ - - REQUIRE(ISC_BUFFER_VALID(b)); - REQUIRE(b->used + n <= b->length); - ISC__BUFFER_ADD(b, n); } @@ -142,10 +117,6 @@ isc__buffer_subtract(isc_buffer_t *b, unsigned int n) { /* * Decrease the 'used' region of 'b' by 'n' bytes. */ - - REQUIRE(ISC_BUFFER_VALID(b)); - REQUIRE(b->used >= n); - ISC__BUFFER_SUBTRACT(b, n); } @@ -154,9 +125,6 @@ isc__buffer_clear(isc_buffer_t *b) { /* * Make the used region empty. */ - - REQUIRE(ISC_BUFFER_VALID(b)); - ISC__BUFFER_CLEAR(b); } @@ -165,10 +133,6 @@ isc__buffer_consumedregion(isc_buffer_t *b, isc_region_t *r) { /* * Make 'r' refer to the consumed region of 'b'. */ - - REQUIRE(ISC_BUFFER_VALID(b)); - REQUIRE(r != NULL); - ISC__BUFFER_CONSUMEDREGION(b, r); } @@ -177,10 +141,6 @@ isc__buffer_remainingregion(isc_buffer_t *b, isc_region_t *r) { /* * Make 'r' refer to the remaining region of 'b'. */ - - REQUIRE(ISC_BUFFER_VALID(b)); - REQUIRE(r != NULL); - ISC__BUFFER_REMAININGREGION(b, r); } @@ -189,10 +149,6 @@ isc__buffer_activeregion(isc_buffer_t *b, isc_region_t *r) { /* * Make 'r' refer to the active region of 'b'. */ - - REQUIRE(ISC_BUFFER_VALID(b)); - REQUIRE(r != NULL); - ISC__BUFFER_ACTIVEREGION(b, r); } @@ -201,10 +157,6 @@ isc__buffer_setactive(isc_buffer_t *b, unsigned int n) { /* * Sets the end of the active region 'n' bytes after current. */ - - REQUIRE(ISC_BUFFER_VALID(b)); - REQUIRE(b->current + n <= b->used); - ISC__BUFFER_SETACTIVE(b, n); } @@ -213,9 +165,6 @@ isc__buffer_first(isc_buffer_t *b) { /* * Make the consumed region empty. */ - - REQUIRE(ISC_BUFFER_VALID(b)); - ISC__BUFFER_FIRST(b); } @@ -224,10 +173,6 @@ isc__buffer_forward(isc_buffer_t *b, unsigned int n) { /* * Increase the 'consumed' region of 'b' by 'n' bytes. */ - - REQUIRE(ISC_BUFFER_VALID(b)); - REQUIRE(b->current + n <= b->used); - ISC__BUFFER_FORWARD(b, n); } @@ -236,10 +181,6 @@ isc__buffer_back(isc_buffer_t *b, unsigned int n) { /* * Decrease the 'consumed' region of 'b' by 'n' bytes. */ - - REQUIRE(ISC_BUFFER_VALID(b)); - REQUIRE(n <= b->current); - ISC__BUFFER_BACK(b, n); } @@ -292,14 +233,6 @@ isc_buffer_getuint8(isc_buffer_t *b) { void isc__buffer_putuint8(isc_buffer_t *b, uint8_t val) { - isc_result_t result; - REQUIRE(ISC_BUFFER_VALID(b)); - if (ISC_UNLIKELY(b->autore)) { - result = isc_buffer_reserve(&b, 1); - REQUIRE(result == ISC_R_SUCCESS); - } - REQUIRE(isc_buffer_availablelength(b) >= 1); - ISC__BUFFER_PUTUINT8(b, val); } @@ -326,27 +259,11 @@ isc_buffer_getuint16(isc_buffer_t *b) { void isc__buffer_putuint16(isc_buffer_t *b, uint16_t val) { - isc_result_t result; - REQUIRE(ISC_BUFFER_VALID(b)); - if (ISC_UNLIKELY(b->autore)) { - result = isc_buffer_reserve(&b, 2); - REQUIRE(result == ISC_R_SUCCESS); - } - REQUIRE(isc_buffer_availablelength(b) >= 2); - ISC__BUFFER_PUTUINT16(b, val); } void isc__buffer_putuint24(isc_buffer_t *b, uint32_t val) { - isc_result_t result; - REQUIRE(ISC_BUFFER_VALID(b)); - if (ISC_UNLIKELY(b->autore)) { - result = isc_buffer_reserve(&b, 3); - REQUIRE(result == ISC_R_SUCCESS); - } - REQUIRE(isc_buffer_availablelength(b) >= 3); - ISC__BUFFER_PUTUINT24(b, val); } @@ -375,14 +292,6 @@ isc_buffer_getuint32(isc_buffer_t *b) { void isc__buffer_putuint32(isc_buffer_t *b, uint32_t val) { - isc_result_t result; - REQUIRE(ISC_BUFFER_VALID(b)); - if (ISC_UNLIKELY(b->autore)) { - result = isc_buffer_reserve(&b, 4); - REQUIRE(result == ISC_R_SUCCESS); - } - REQUIRE(isc_buffer_availablelength(b) >= 4); - ISC__BUFFER_PUTUINT32(b, val); } @@ -433,39 +342,12 @@ isc__buffer_putuint48(isc_buffer_t *b, uint64_t val) { void isc__buffer_putmem(isc_buffer_t *b, const unsigned char *base, unsigned int length) { - isc_result_t result; - REQUIRE(ISC_BUFFER_VALID(b)); - if (ISC_UNLIKELY(b->autore)) { - result = isc_buffer_reserve(&b, length); - REQUIRE(result == ISC_R_SUCCESS); - } - REQUIRE(isc_buffer_availablelength(b) >= length); - ISC__BUFFER_PUTMEM(b, base, length); } void isc__buffer_putstr(isc_buffer_t *b, const char *source) { - unsigned int l; - unsigned char *cp; - isc_result_t result; - - REQUIRE(ISC_BUFFER_VALID(b)); - REQUIRE(source != NULL); - - /* - * Do not use ISC__BUFFER_PUTSTR(), so strlen is only done once. - */ - l = strlen(source); - if (ISC_UNLIKELY(b->autore)) { - result = isc_buffer_reserve(&b, l); - REQUIRE(result == ISC_R_SUCCESS); - } - REQUIRE(isc_buffer_availablelength(b) >= l); - - cp = isc_buffer_used(b); - memmove(cp, source, l); - b->used += l; + ISC__BUFFER_PUTSTR(b, source); } void diff --git a/lib/isc/include/isc/buffer.h b/lib/isc/include/isc/buffer.h index b5188975a76..f3becae1cf4 100644 --- a/lib/isc/include/isc/buffer.h +++ b/lib/isc/include/isc/buffer.h @@ -108,6 +108,7 @@ #include #include #include +#include #include #include @@ -795,6 +796,7 @@ ISC_LANG_ENDDECLS */ #define ISC__BUFFER_INIT(_b, _base, _length) \ do { \ + ISC_REQUIRE((_b) != NULL); \ (_b)->base = _base; \ (_b)->length = (_length); \ (_b)->used = 0; \ @@ -808,41 +810,54 @@ ISC_LANG_ENDDECLS #define ISC__BUFFER_INITNULL(_b) ISC__BUFFER_INIT(_b, NULL, 0) -#define ISC__BUFFER_INVALIDATE(_b) \ - do { \ - (_b)->magic = 0; \ - (_b)->base = NULL; \ - (_b)->length = 0; \ - (_b)->used = 0; \ - (_b)->current = 0; \ - (_b)->active = 0; \ +#define ISC__BUFFER_INVALIDATE(_b) \ + do { \ + ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \ + ISC_REQUIRE(!ISC_LINK_LINKED((_b), link)); \ + ISC_REQUIRE((_b)->mctx == NULL); \ + (_b)->magic = 0; \ + (_b)->base = NULL; \ + (_b)->length = 0; \ + (_b)->used = 0; \ + (_b)->current = 0; \ + (_b)->active = 0; \ } while (0) -#define ISC__BUFFER_REGION(_b, _r) \ - do { \ - (_r)->base = (_b)->base; \ - (_r)->length = (_b)->length; \ +#define ISC__BUFFER_REGION(_b, _r) \ + do { \ + ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \ + ISC_REQUIRE((_r) != NULL); \ + (_r)->base = (_b)->base; \ + (_r)->length = (_b)->length; \ } while (0) -#define ISC__BUFFER_USEDREGION(_b, _r) \ - do { \ - (_r)->base = (_b)->base; \ - (_r)->length = (_b)->used; \ +#define ISC__BUFFER_USEDREGION(_b, _r) \ + do { \ + ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \ + ISC_REQUIRE((_r) != NULL); \ + (_r)->base = (_b)->base; \ + (_r)->length = (_b)->used; \ } while (0) #define ISC__BUFFER_AVAILABLEREGION(_b, _r) \ do { \ + ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \ + ISC_REQUIRE((_r) != NULL); \ (_r)->base = isc_buffer_used(_b); \ (_r)->length = isc_buffer_availablelength(_b); \ } while (0) -#define ISC__BUFFER_ADD(_b, _n) \ - do { \ - (_b)->used += (_n); \ +#define ISC__BUFFER_ADD(_b, _n) \ + do { \ + ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \ + ISC_REQUIRE((_b)->used + (_n) <= (_b)->length); \ + (_b)->used += (_n); \ } while (0) #define ISC__BUFFER_SUBTRACT(_b, _n) \ do { \ + ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \ + ISC_REQUIRE((_b)->used >= (_n)); \ (_b)->used -= (_n); \ if ((_b)->current > (_b)->used) \ (_b)->current = (_b)->used; \ @@ -850,27 +865,34 @@ ISC_LANG_ENDDECLS (_b)->active = (_b)->used; \ } while (0) -#define ISC__BUFFER_CLEAR(_b) \ - do { \ - (_b)->used = 0; \ - (_b)->current = 0; \ - (_b)->active = 0; \ +#define ISC__BUFFER_CLEAR(_b) \ + do { \ + ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \ + (_b)->used = 0; \ + (_b)->current = 0; \ + (_b)->active = 0; \ } while (0) -#define ISC__BUFFER_CONSUMEDREGION(_b, _r) \ - do { \ - (_r)->base = (_b)->base; \ - (_r)->length = (_b)->current; \ +#define ISC__BUFFER_CONSUMEDREGION(_b, _r) \ + do { \ + ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \ + ISC_REQUIRE((_r) != NULL); \ + (_r)->base = (_b)->base; \ + (_r)->length = (_b)->current; \ } while (0) #define ISC__BUFFER_REMAININGREGION(_b, _r) \ do { \ + ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \ + ISC_REQUIRE((_r) != NULL); \ (_r)->base = isc_buffer_current(_b); \ (_r)->length = isc_buffer_remaininglength(_b); \ } while (0) #define ISC__BUFFER_ACTIVEREGION(_b, _r) \ do { \ + ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \ + ISC_REQUIRE((_r) != NULL); \ if ((_b)->current < (_b)->active) { \ (_r)->base = isc_buffer_current(_b); \ (_r)->length = isc_buffer_activelength(_b); \ @@ -880,28 +902,36 @@ ISC_LANG_ENDDECLS } \ } while (0) -#define ISC__BUFFER_SETACTIVE(_b, _n) \ - do { \ - (_b)->active = (_b)->current + (_n); \ +#define ISC__BUFFER_SETACTIVE(_b, _n) \ + do { \ + ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \ + ISC_REQUIRE((_b)->current + (_n) <= (_b)->used); \ + (_b)->active = (_b)->current + (_n); \ } while (0) -#define ISC__BUFFER_FIRST(_b) \ - do { \ - (_b)->current = 0; \ +#define ISC__BUFFER_FIRST(_b) \ + do { \ + ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \ + (_b)->current = 0; \ } while (0) -#define ISC__BUFFER_FORWARD(_b, _n) \ - do { \ - (_b)->current += (_n); \ +#define ISC__BUFFER_FORWARD(_b, _n) \ + do { \ + ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \ + ISC_REQUIRE((_b)->current + (_n) <= (_b)->used); \ + (_b)->current += (_n); \ } while (0) -#define ISC__BUFFER_BACK(_b, _n) \ - do { \ - (_b)->current -= (_n); \ +#define ISC__BUFFER_BACK(_b, _n) \ + do { \ + ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \ + ISC_REQUIRE((_n) <= (_b)->current); \ + (_b)->current -= (_n); \ } while (0) #define ISC__BUFFER_PUTMEM(_b, _base, _length) \ do { \ + ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \ if (ISC_UNLIKELY((_b)->autore)) { \ isc_buffer_t *_tmp = _b; \ ISC_REQUIRE(isc_buffer_reserve(&_tmp, _length) == \ @@ -919,6 +949,8 @@ ISC_LANG_ENDDECLS do { \ unsigned int _length; \ unsigned char *_cp; \ + ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \ + ISC_REQUIRE((_source) != NULL); \ _length = (unsigned int)strlen(_source); \ if (ISC_UNLIKELY((_b)->autore)) { \ isc_buffer_t *_tmp = _b; \ @@ -936,6 +968,7 @@ ISC_LANG_ENDDECLS unsigned char *_cp; \ /* evaluate (_val) only once */ \ uint8_t _val2 = (_val); \ + ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \ if (ISC_UNLIKELY((_b)->autore)) { \ isc_buffer_t *_tmp = _b; \ ISC_REQUIRE(isc_buffer_reserve(&_tmp, 1) == \ @@ -952,6 +985,7 @@ ISC_LANG_ENDDECLS unsigned char *_cp; \ /* evaluate (_val) only once */ \ uint16_t _val2 = (_val); \ + ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \ if (ISC_UNLIKELY((_b)->autore)) { \ isc_buffer_t *_tmp = _b; \ ISC_REQUIRE(isc_buffer_reserve(&_tmp, 2) == \ @@ -969,6 +1003,7 @@ ISC_LANG_ENDDECLS unsigned char *_cp; \ /* evaluate (_val) only once */ \ uint32_t _val2 = (_val); \ + ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \ if (ISC_UNLIKELY((_b)->autore)) { \ isc_buffer_t *_tmp = _b; \ ISC_REQUIRE(isc_buffer_reserve(&_tmp, 3) == \ @@ -987,6 +1022,7 @@ ISC_LANG_ENDDECLS unsigned char *_cp; \ /* evaluate (_val) only once */ \ uint32_t _val2 = (_val); \ + ISC_REQUIRE(ISC_BUFFER_VALID(_b)); \ if (ISC_UNLIKELY((_b)->autore)) { \ isc_buffer_t *_tmp = _b; \ ISC_REQUIRE(isc_buffer_reserve(&_tmp, 4) == \