From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f195.google.com (mail-wr0-f195.google.com [209.85.128.195]) by dpdk.org (Postfix) with ESMTP id 0C2031C69B for ; Fri, 13 Apr 2018 17:39:16 +0200 (CEST) Received: by mail-wr0-f195.google.com with SMTP id l13so8184369wrb.5 for ; Fri, 13 Apr 2018 08:39:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=kq6W06MAsaJ03nhIDzO6coc9Pd6sqclOYNR9tkCkpyE=; b=liLy4G2Vd9YfOzMyo+pU+oGQDcY2FJyvdNssFYA3VALthHd22gKtlGLpqXnvhdX6XQ ug5OECrntLiw466TXbuTXmI4dakrd+ZgA2B0ffuP2q2uUdcZpijQlf3rm3wcz60MQY3Y q1wr6O8ViG90ASlwfoUlDGHPN6Y3hlUdSiSqdkrlSDYwzpgJBgXpBiUOgUaNo7NQFw12 ZidgAJZfbBtzYyJ+YpwZUO3EOGl94J+AQoqOOgZ+NWLH4fnJ/wWtOtxEe4ZnwW8oUdTd dyKSY8137VCksvvnX01SB0PJR3PgdORkeXwtZvzrK+cXR4PpQneiNecrGjz8fzmiDVZ3 s65Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=kq6W06MAsaJ03nhIDzO6coc9Pd6sqclOYNR9tkCkpyE=; b=Lxnl4PJn5v/sYZGMolJP9+QFWSQGizDSM9my/F+g1LNnSOsuGt2JcZmQkIcYcEZahP qJ+2qhcR79KDptYNefyIdVYlgEBfcpsTvQOpIZ3FdUJucsaq7RlM2yGMxAQ0fMMl8CqU gmEy4AMY1byXtHXYR6PSkCe6Ljf+fN0cF0WaSSKgW9MlUdlLPG/0Ak9o4yO16C7be43D wcSK3bUr4fvwkvS/U4rdDV130pRiWd2tmeeKtXHATjEzywTiyUUAVBXE9vYKg97ismvJ y2HEAgdnGzxbmPbPTPNTQmcJhAl/ADXDbrgkOXBPLeTyyG5+me5cXxlLrYGH+J0mA7wI zJfw== X-Gm-Message-State: ALQs6tAA79mMIH56wtkSXyM4dANmAdyVe2A+//v29pjPwceko/+eJe2b JlSaMthAEi6iNZ8OiR6By+bO2w== X-Google-Smtp-Source: AIpwx48KJMHsEqwm7MBG/unKq4tjN99i7xYckqu2VZONVcfF+PnNTXdGI8AqSlpiBsC4JqFyQRATfg== X-Received: by 10.28.236.145 with SMTP id h17mr3810126wmi.43.1523633956301; Fri, 13 Apr 2018 08:39:16 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id c14sm2698835wmi.28.2018.04.13.08.39.15 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 13 Apr 2018 08:39:15 -0700 (PDT) Date: Fri, 13 Apr 2018 17:39:02 +0200 From: Adrien Mazarguil To: Anatoly Burakov Cc: dev@dpdk.org Message-ID: <20180413153749.28208-2-adrien.mazarguil@6wind.com> References: <20180413153749.28208-1-adrien.mazarguil@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180413153749.28208-1-adrien.mazarguil@6wind.com> X-Mailer: git-send-email 2.11.0 Subject: [dpdk-dev] [PATCH 2/2] eal: fix signed integers in fbarray X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Apr 2018 15:39:17 -0000 While debugging startup issues encountered with Clang (see "eal: fix undefined behavior in fbarray"), I noticed that fbarray stores indices, sizes and masks on signed integers involved in bitwise operations. Such operations almost invariably cause undefined behavior with values that cannot be represented by the result type, as is often the case with bit-masks and left-shifts. This patch replaces them with unsigned integers as a safety measure and promotes a few internal variables to larger types for consistency. Fixes: c44d09811b40 ("eal: add shared indexed file-backed array") Cc: Anatoly Burakov Signed-off-by: Adrien Mazarguil --- lib/librte_eal/common/eal_common_fbarray.c | 101 ++++++++++++----------- lib/librte_eal/common/include/rte_fbarray.h | 33 ++++---- 2 files changed, 70 insertions(+), 64 deletions(-) diff --git a/lib/librte_eal/common/eal_common_fbarray.c b/lib/librte_eal/common/eal_common_fbarray.c index 11aa3f22a..26c88be88 100644 --- a/lib/librte_eal/common/eal_common_fbarray.c +++ b/lib/librte_eal/common/eal_common_fbarray.c @@ -21,7 +21,7 @@ #include "rte_fbarray.h" #define MASK_SHIFT 6ULL -#define MASK_ALIGN (1 << MASK_SHIFT) +#define MASK_ALIGN (1ULL << MASK_SHIFT) #define MASK_LEN_TO_IDX(x) ((x) >> MASK_SHIFT) #define MASK_LEN_TO_MOD(x) ((x) - RTE_ALIGN_FLOOR(x, MASK_ALIGN)) #define MASK_GET_IDX(idx, mod) ((idx << MASK_SHIFT) + mod) @@ -32,12 +32,12 @@ */ struct used_mask { - int n_masks; + unsigned int n_masks; uint64_t data[]; }; static size_t -calc_mask_size(int len) +calc_mask_size(unsigned int len) { /* mask must be multiple of MASK_ALIGN, even though length of array * itself may not be aligned on that boundary. @@ -48,7 +48,7 @@ calc_mask_size(int len) } static size_t -calc_data_size(size_t page_sz, int elt_sz, int len) +calc_data_size(size_t page_sz, unsigned int elt_sz, unsigned int len) { size_t data_sz = elt_sz * len; size_t msk_sz = calc_mask_size(len); @@ -56,7 +56,7 @@ calc_data_size(size_t page_sz, int elt_sz, int len) } static struct used_mask * -get_used_mask(void *data, int elt_sz, int len) +get_used_mask(void *data, unsigned int elt_sz, unsigned int len) { return (struct used_mask *) RTE_PTR_ADD(data, elt_sz * len); } @@ -86,13 +86,14 @@ resize_and_map(int fd, void *addr, size_t len) } static int -find_next_n(const struct rte_fbarray *arr, int start, int n, bool used) +find_next_n(const struct rte_fbarray *arr, unsigned int start, unsigned int n, + bool used) { const struct used_mask *msk = get_used_mask(arr->data, arr->elt_sz, arr->len); - int msk_idx, lookahead_idx, first, first_mod; - int last, last_mod, last_msk; - uint64_t ignore_msk; + unsigned int msk_idx, lookahead_idx, first, first_mod; + unsigned int last, last_mod; + uint64_t last_msk, ignore_msk; /* * mask only has granularity of MASK_ALIGN, but start may not be aligned @@ -108,11 +109,11 @@ find_next_n(const struct rte_fbarray *arr, int start, int n, bool used) */ last = MASK_LEN_TO_IDX(arr->len); last_mod = MASK_LEN_TO_MOD(arr->len); - last_msk = ~(-(1ULL) << last_mod); + last_msk = ~(-1ULL << last_mod); for (msk_idx = first; msk_idx < msk->n_masks; msk_idx++) { uint64_t cur_msk, lookahead_msk; - int run_start, clz, left; + unsigned int run_start, clz, left; bool found = false; /* * The process of getting n consecutive bits for arbitrary n is @@ -155,9 +156,9 @@ find_next_n(const struct rte_fbarray *arr, int start, int n, bool used) } /* if n can fit in within a single mask, do a search */ - if (n <= MASK_ALIGN) { + if ((unsigned int)n <= MASK_ALIGN) { uint64_t tmp_msk = cur_msk; - int s_idx; + unsigned int s_idx; for (s_idx = 0; s_idx < n - 1; s_idx++) tmp_msk &= tmp_msk >> 1ULL; /* we found what we were looking for */ @@ -188,7 +189,7 @@ find_next_n(const struct rte_fbarray *arr, int start, int n, bool used) for (lookahead_idx = msk_idx + 1; lookahead_idx < msk->n_masks; lookahead_idx++) { - int s_idx, need; + unsigned int s_idx, need; lookahead_msk = msk->data[lookahead_idx]; /* if we're looking for free space, invert the mask */ @@ -196,7 +197,7 @@ find_next_n(const struct rte_fbarray *arr, int start, int n, bool used) lookahead_msk = ~lookahead_msk; /* figure out how many consecutive bits we need here */ - need = RTE_MIN(left, MASK_ALIGN); + need = RTE_MIN((unsigned int)left, MASK_ALIGN); for (s_idx = 0; s_idx < need - 1; s_idx++) lookahead_msk &= lookahead_msk >> 1ULL; @@ -234,13 +235,13 @@ find_next_n(const struct rte_fbarray *arr, int start, int n, bool used) } static int -find_next(const struct rte_fbarray *arr, int start, bool used) +find_next(const struct rte_fbarray *arr, unsigned int start, bool used) { const struct used_mask *msk = get_used_mask(arr->data, arr->elt_sz, arr->len); - int idx, first, first_mod; - int last, last_mod, last_msk; - uint64_t ignore_msk; + unsigned int idx, first, first_mod; + unsigned int last, last_mod; + uint64_t last_msk, ignore_msk; /* * mask only has granularity of MASK_ALIGN, but start may not be aligned @@ -290,13 +291,14 @@ find_next(const struct rte_fbarray *arr, int start, bool used) } static int -find_contig(const struct rte_fbarray *arr, int start, bool used) +find_contig(const struct rte_fbarray *arr, unsigned int start, bool used) { const struct used_mask *msk = get_used_mask(arr->data, arr->elt_sz, arr->len); - int idx, first, first_mod; - int last, last_mod, last_msk; - int need_len, result = 0; + unsigned int idx, first, first_mod; + unsigned int last, last_mod; + uint64_t last_msk; + unsigned int need_len, result = 0; /* array length may not be aligned, so calculate ignore mask for last * mask index. @@ -309,7 +311,7 @@ find_contig(const struct rte_fbarray *arr, int start, bool used) first_mod = MASK_LEN_TO_MOD(start); for (idx = first; idx < msk->n_masks; idx++, result += need_len) { uint64_t cur = msk->data[idx]; - int run_len; + unsigned int run_len; need_len = MASK_ALIGN; @@ -350,15 +352,15 @@ find_contig(const struct rte_fbarray *arr, int start, bool used) } static int -set_used(struct rte_fbarray *arr, int idx, bool used) +set_used(struct rte_fbarray *arr, unsigned int idx, bool used) { struct used_mask *msk = get_used_mask(arr->data, arr->elt_sz, arr->len); uint64_t msk_bit = 1ULL << MASK_LEN_TO_MOD(idx); - int msk_idx = MASK_LEN_TO_IDX(idx); + unsigned int msk_idx = MASK_LEN_TO_IDX(idx); bool already_used; int ret = -1; - if (arr == NULL || idx < 0 || idx >= arr->len) { + if (arr == NULL || idx >= arr->len) { rte_errno = EINVAL; return -1; } @@ -402,7 +404,8 @@ fully_validate(const char *name, unsigned int elt_sz, unsigned int len) } int __rte_experimental -rte_fbarray_init(struct rte_fbarray *arr, const char *name, int len, int elt_sz) +rte_fbarray_init(struct rte_fbarray *arr, const char *name, unsigned int len, + unsigned int elt_sz) { size_t page_sz, mmap_len; char path[PATH_MAX]; @@ -601,10 +604,10 @@ rte_fbarray_destroy(struct rte_fbarray *arr) } void * __rte_experimental -rte_fbarray_get(const struct rte_fbarray *arr, int idx) +rte_fbarray_get(const struct rte_fbarray *arr, unsigned int idx) { void *ret = NULL; - if (arr == NULL || idx < 0) { + if (arr == NULL) { rte_errno = EINVAL; return NULL; } @@ -620,26 +623,26 @@ rte_fbarray_get(const struct rte_fbarray *arr, int idx) } int __rte_experimental -rte_fbarray_set_used(struct rte_fbarray *arr, int idx) +rte_fbarray_set_used(struct rte_fbarray *arr, unsigned int idx) { return set_used(arr, idx, true); } int __rte_experimental -rte_fbarray_set_free(struct rte_fbarray *arr, int idx) +rte_fbarray_set_free(struct rte_fbarray *arr, unsigned int idx) { return set_used(arr, idx, false); } int __rte_experimental -rte_fbarray_is_used(struct rte_fbarray *arr, int idx) +rte_fbarray_is_used(struct rte_fbarray *arr, unsigned int idx) { struct used_mask *msk; int msk_idx; uint64_t msk_bit; int ret = -1; - if (arr == NULL || idx < 0 || idx >= arr->len) { + if (arr == NULL || idx >= arr->len) { rte_errno = EINVAL; return -1; } @@ -659,11 +662,11 @@ rte_fbarray_is_used(struct rte_fbarray *arr, int idx) } int __rte_experimental -rte_fbarray_find_next_free(struct rte_fbarray *arr, int start) +rte_fbarray_find_next_free(struct rte_fbarray *arr, unsigned int start) { int ret = -1; - if (arr == NULL || start < 0 || start >= arr->len) { + if (arr == NULL || start >= arr->len) { rte_errno = EINVAL; return -1; } @@ -683,11 +686,11 @@ rte_fbarray_find_next_free(struct rte_fbarray *arr, int start) } int __rte_experimental -rte_fbarray_find_next_used(struct rte_fbarray *arr, int start) +rte_fbarray_find_next_used(struct rte_fbarray *arr, unsigned int start) { int ret = -1; - if (arr == NULL || start < 0 || start >= arr->len) { + if (arr == NULL || start >= arr->len) { rte_errno = EINVAL; return -1; } @@ -707,12 +710,12 @@ rte_fbarray_find_next_used(struct rte_fbarray *arr, int start) } int __rte_experimental -rte_fbarray_find_next_n_free(struct rte_fbarray *arr, int start, int n) +rte_fbarray_find_next_n_free(struct rte_fbarray *arr, unsigned int start, + unsigned int n) { int ret = -1; - if (arr == NULL || start < 0 || start >= arr->len || - n < 0 || n > arr->len) { + if (arr == NULL || start >= arr->len || n > arr->len) { rte_errno = EINVAL; return -1; } @@ -732,12 +735,12 @@ rte_fbarray_find_next_n_free(struct rte_fbarray *arr, int start, int n) } int __rte_experimental -rte_fbarray_find_next_n_used(struct rte_fbarray *arr, int start, int n) +rte_fbarray_find_next_n_used(struct rte_fbarray *arr, unsigned int start, + unsigned int n) { int ret = -1; - if (arr == NULL || start < 0 || start >= arr->len || - n < 0 || n > arr->len) { + if (arr == NULL || start >= arr->len || n > arr->len) { rte_errno = EINVAL; return -1; } @@ -757,11 +760,11 @@ rte_fbarray_find_next_n_used(struct rte_fbarray *arr, int start, int n) } int __rte_experimental -rte_fbarray_find_contig_free(struct rte_fbarray *arr, int start) +rte_fbarray_find_contig_free(struct rte_fbarray *arr, unsigned int start) { int ret = -1; - if (arr == NULL || start < 0 || start >= arr->len) { + if (arr == NULL || start >= arr->len) { rte_errno = EINVAL; return -1; } @@ -786,11 +789,11 @@ rte_fbarray_find_contig_free(struct rte_fbarray *arr, int start) } int __rte_experimental -rte_fbarray_find_contig_used(struct rte_fbarray *arr, int start) +rte_fbarray_find_contig_used(struct rte_fbarray *arr, unsigned int start) { int ret = -1; - if (arr == NULL || start < 0 || start >= arr->len) { + if (arr == NULL || start >= arr->len) { rte_errno = EINVAL; return -1; } @@ -834,7 +837,7 @@ void __rte_experimental rte_fbarray_dump_metadata(struct rte_fbarray *arr, FILE *f) { struct used_mask *msk; - int i; + unsigned int i; if (arr == NULL || f == NULL) { rte_errno = EINVAL; diff --git a/lib/librte_eal/common/include/rte_fbarray.h b/lib/librte_eal/common/include/rte_fbarray.h index 97df945ea..3e61fffee 100644 --- a/lib/librte_eal/common/include/rte_fbarray.h +++ b/lib/librte_eal/common/include/rte_fbarray.h @@ -44,9 +44,9 @@ extern "C" { struct rte_fbarray { char name[RTE_FBARRAY_NAME_LEN]; /**< name associated with an array */ - int count; /**< number of entries stored */ - int len; /**< current length of the array */ - int elt_sz; /**< size of each element */ + unsigned int count; /**< number of entries stored */ + unsigned int len; /**< current length of the array */ + unsigned int elt_sz; /**< size of each element */ void *data; /**< data pointer */ rte_rwlock_t rwlock; /**< multiprocess lock */ }; @@ -76,8 +76,8 @@ struct rte_fbarray { * - -1 on failure, with ``rte_errno`` indicating reason for failure. */ int __rte_experimental -rte_fbarray_init(struct rte_fbarray *arr, const char *name, int len, - int elt_sz); +rte_fbarray_init(struct rte_fbarray *arr, const char *name, unsigned int len, + unsigned int elt_sz); /** @@ -154,7 +154,7 @@ rte_fbarray_detach(struct rte_fbarray *arr); * - NULL on failure, with ``rte_errno`` indicating reason for failure. */ void * __rte_experimental -rte_fbarray_get(const struct rte_fbarray *arr, int idx); +rte_fbarray_get(const struct rte_fbarray *arr, unsigned int idx); /** @@ -188,7 +188,7 @@ rte_fbarray_find_idx(const struct rte_fbarray *arr, const void *elt); * - -1 on failure, with ``rte_errno`` indicating reason for failure. */ int __rte_experimental -rte_fbarray_set_used(struct rte_fbarray *arr, int idx); +rte_fbarray_set_used(struct rte_fbarray *arr, unsigned int idx); /** @@ -205,7 +205,7 @@ rte_fbarray_set_used(struct rte_fbarray *arr, int idx); * - -1 on failure, with ``rte_errno`` indicating reason for failure. */ int __rte_experimental -rte_fbarray_set_free(struct rte_fbarray *arr, int idx); +rte_fbarray_set_free(struct rte_fbarray *arr, unsigned int idx); /** @@ -223,7 +223,7 @@ rte_fbarray_set_free(struct rte_fbarray *arr, int idx); * - -1 on failure, with ``rte_errno`` indicating reason for failure. */ int __rte_experimental -rte_fbarray_is_used(struct rte_fbarray *arr, int idx); +rte_fbarray_is_used(struct rte_fbarray *arr, unsigned int idx); /** @@ -240,7 +240,7 @@ rte_fbarray_is_used(struct rte_fbarray *arr, int idx); * - -1 on failure, with ``rte_errno`` indicating reason for failure. */ int __rte_experimental -rte_fbarray_find_next_free(struct rte_fbarray *arr, int start); +rte_fbarray_find_next_free(struct rte_fbarray *arr, unsigned int start); /** @@ -257,7 +257,7 @@ rte_fbarray_find_next_free(struct rte_fbarray *arr, int start); * - -1 on failure, with ``rte_errno`` indicating reason for failure. */ int __rte_experimental -rte_fbarray_find_next_used(struct rte_fbarray *arr, int start); +rte_fbarray_find_next_used(struct rte_fbarray *arr, unsigned int start); /** @@ -277,7 +277,8 @@ rte_fbarray_find_next_used(struct rte_fbarray *arr, int start); * - -1 on failure, with ``rte_errno`` indicating reason for failure. */ int __rte_experimental -rte_fbarray_find_next_n_free(struct rte_fbarray *arr, int start, int n); +rte_fbarray_find_next_n_free(struct rte_fbarray *arr, unsigned int start, + unsigned int n); /** @@ -297,7 +298,8 @@ rte_fbarray_find_next_n_free(struct rte_fbarray *arr, int start, int n); * - -1 on failure, with ``rte_errno`` indicating reason for failure. */ int __rte_experimental -rte_fbarray_find_next_n_used(struct rte_fbarray *arr, int start, int n); +rte_fbarray_find_next_n_used(struct rte_fbarray *arr, unsigned int start, + unsigned int n); /** @@ -314,7 +316,8 @@ rte_fbarray_find_next_n_used(struct rte_fbarray *arr, int start, int n); * - -1 on failure, with ``rte_errno`` indicating reason for failure. */ int __rte_experimental -rte_fbarray_find_contig_free(struct rte_fbarray *arr, int start); +rte_fbarray_find_contig_free(struct rte_fbarray *arr, + unsigned int start); /** @@ -331,7 +334,7 @@ rte_fbarray_find_contig_free(struct rte_fbarray *arr, int start); * - -1 on failure, with ``rte_errno`` indicating reason for failure. */ int __rte_experimental -rte_fbarray_find_contig_used(struct rte_fbarray *arr, int start); +rte_fbarray_find_contig_used(struct rte_fbarray *arr, unsigned int start); /** -- 2.11.0