[v3,1/2] eal: fix side effects in align mul macros
Checks
Commit Message
From: Pavan Nikhilesh <pbhagavatula@marvell.com>
Avoid expanding parameters inside the macro multiple times.
For example:
RTE_ALIGN_MUL_NEAR(rte_rdtsc() - start, CYC_PER_10MHZ);
Here rte_rdtsc() call is expanded multiple times in the macro
causing it to return different values that leads to unintended
side effects.
Also, update common_autotest to detect macro side effects.
Fixes: f56e551485d5 ("eal: add macro to align value to the nearest multiple")
Cc: stable@dpdk.org
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
Signed-off-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
v3 Changes:
- Fix examples/bbdev_app compilation.
v2 Changes:
- Fix mlx5 compilation.
app/test/test_common.c | 27 ++++++++++++++++++++++++++-
lib/eal/include/rte_common.h | 28 +++++++++++++++++++---------
2 files changed, 45 insertions(+), 10 deletions(-)
--
2.17.1
Comments
On Tue, 11 May 2021 01:10:06 +0530
<pbhagavatula@marvell.com> wrote:
> Avoid expanding parameters inside the macro multiple times.
> For example:
> RTE_ALIGN_MUL_NEAR(rte_rdtsc() - start, CYC_PER_10MHZ);
> Here rte_rdtsc() call is expanded multiple times in the macro
> causing it to return different values that leads to unintended
> side effects.
This was already fixed.
@@ -18,6 +18,13 @@
{printf(x "() test failed!\n");\
return -1;}
+static uintptr_t callcount;
+static uintptr_t
+inc_callcount(void)
+{
+ return callcount++;
+}
+
/* this is really a sanity check */
static int
test_macros(int __rte_unused unused_parm)
@@ -29,7 +36,19 @@ test_macros(int __rte_unused unused_parm)
{printf(#x "() test failed!\n");\
return -1;}
+#define TEST_SIDE_EFFECT_2(macro, type1, type2) \
+ do { \
+ callcount = 0; \
+ (void)macro((type1)inc_callcount(), (type2)inc_callcount()); \
+ if (callcount != 2) { \
+ printf(#macro " has side effects: callcount=%u\n", \
+ (unsigned int)callcount); \
+ ret = -1; \
+ } \
+ } while (0)
+
uintptr_t unused = 0;
+ int ret = 0;
RTE_SET_USED(unused);
@@ -47,7 +66,13 @@ test_macros(int __rte_unused unused_parm)
if (strncmp(RTE_STR(test), "test", sizeof("test")))
FAIL_MACRO(RTE_STR);
- return 0;
+ TEST_SIDE_EFFECT_2(RTE_PTR_ADD, void *, size_t);
+ TEST_SIDE_EFFECT_2(RTE_PTR_DIFF, void *, void *);
+ TEST_SIDE_EFFECT_2(RTE_PTR_SUB, void *, size_t);
+ TEST_SIDE_EFFECT_2(RTE_ALIGN_MUL_CEIL, unsigned int, unsigned int);
+ TEST_SIDE_EFFECT_2(RTE_ALIGN_MUL_FLOOR, unsigned int, unsigned int);
+ TEST_SIDE_EFFECT_2(RTE_ALIGN_MUL_NEAR, unsigned int, unsigned int);
+ return ret;
}
static int
@@ -329,27 +329,37 @@ static void __attribute__((destructor(RTE_PRIO(prio)), used)) func(void)
* value will be of the same type as the first parameter and will be no lower
* than the first parameter.
*/
-#define RTE_ALIGN_MUL_CEIL(v, mul) \
- ((((v) + (typeof(v))(mul) - 1) / ((typeof(v))(mul))) * (typeof(v))(mul))
+#define RTE_ALIGN_MUL_CEIL(v, mul) \
+ __extension__({ \
+ typeof(v) _vc = (v); \
+ typeof(v) _mc = (mul); \
+ ((_vc + _mc - 1) / _mc) * _mc; \
+ })
/**
* Macro to align a value to the multiple of given value. The resultant
* value will be of the same type as the first parameter and will be no higher
* than the first parameter.
*/
-#define RTE_ALIGN_MUL_FLOOR(v, mul) \
- (((v) / ((typeof(v))(mul))) * (typeof(v))(mul))
+#define RTE_ALIGN_MUL_FLOOR(v, mul) \
+ __extension__({ \
+ typeof(v) _vf = (v); \
+ typeof(v) _mf = (mul); \
+ (_vf / _mf) * _mf; \
+ })
/**
* Macro to align value to the nearest multiple of the given value.
* The resultant value might be greater than or less than the first parameter
* whichever difference is the lowest.
*/
-#define RTE_ALIGN_MUL_NEAR(v, mul) \
- ({ \
- typeof(v) ceil = RTE_ALIGN_MUL_CEIL(v, mul); \
- typeof(v) floor = RTE_ALIGN_MUL_FLOOR(v, mul); \
- (ceil - (v)) > ((v) - floor) ? floor : ceil; \
+#define RTE_ALIGN_MUL_NEAR(v, mul) \
+ __extension__({ \
+ typeof(v) _v = (v); \
+ typeof(v) _m = (mul); \
+ typeof(v) floor = RTE_ALIGN_MUL_FLOOR(_v, _m); \
+ typeof(v) ceil = RTE_ALIGN_MUL_CEIL(_v, _m); \
+ (ceil - _v) > (_v - floor) ? floor : ceil; \
})
/**