From: Pavan Nikhilesh <pbhagavat...@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: sta...@dpdk.org

Signed-off-by: Tyler Retzlaff <roret...@linux.microsoft.com>
Signed-off-by: David Marchand <david.march...@redhat.com>
Signed-off-by: Pavan Nikhilesh <pbhagavat...@marvell.com>
---
 Retainded original author signoff, split patches as some of the changes
 involved in modifying drivers.

 app/test/test_common.c       | 27 ++++++++++++++++++++++++++-
 lib/eal/include/rte_common.h | 28 +++++++++++++++++++---------
 2 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/app/test/test_common.c b/app/test/test_common.c
index 12bd1cad90..0dbb87e741 100644
--- a/app/test/test_common.c
+++ b/app/test/test_common.c
@@ -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
diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
index d5a32c66a5..a142596587 100644
--- a/lib/eal/include/rte_common.h
+++ b/lib/eal/include/rte_common.h
@@ -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;                     \
        })

 /**
--
2.17.1

Reply via email to