Julius Werner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32027
Change subject: WIP: Make common macros double-evaluation safe ......................................................................
WIP: Make common macros double-evaluation safe
Just shaking the Jenkins tree to see what falls out for now...
Change-Id: Iea35156c9aa9f6f2c7b8f00991418b746f44315d --- M payloads/libpayload/include/compiler.h M payloads/libpayload/include/libpayload.h M payloads/libpayload/include/stdlib.h M src/commonlib/include/commonlib/compiler.h M src/commonlib/include/commonlib/helpers.h 5 files changed, 113 insertions(+), 24 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/32027/1
diff --git a/payloads/libpayload/include/compiler.h b/payloads/libpayload/include/compiler.h index a830239..4ca9bad 100644 --- a/payloads/libpayload/include/compiler.h +++ b/payloads/libpayload/include/compiler.h @@ -26,4 +26,18 @@ #define __always_unused __attribute__((unused)) #define __must_check __attribute__((warn_unused_result))
+/* This evaluates to the first expression, unless that is constant in which case + it evalutates to the second. This is useful to write typeof() macros in a way + that they don't get hung up on the type of literal expressions (e.g. compare + an integer literal which is signed by default with a size_t variable). It is + generally a safe thing to do because if the constant doesn't fit into the + chosen type, you'll get a -Werror=overflow at compile time. */ +#define __SELECT_NON_CONST(expr, fallback_expr) __builtin_choose_expr( \ + __builtin_constant_p(expr), fallback_expr, expr) + +/* This creates a unique local variable name for use in macros. */ +#define __TMPNAME_3(i) __tmpname_##i +#define __TMPNAME_2(i) __TMPNAME_3(i) +#define __TMPNAME __TMPNAME_2(__COUNTER__) + #endif diff --git a/payloads/libpayload/include/libpayload.h b/payloads/libpayload/include/libpayload.h index 0b9ab0d..c889605 100644 --- a/payloads/libpayload/include/libpayload.h +++ b/payloads/libpayload/include/libpayload.h @@ -66,10 +66,32 @@ #include <pci.h> #include <archive.h>
-#define MIN(a,b) ((a) < (b) ? (a) : (b)) -#define MAX(a,b) ((a) > (b) ? (a) : (b)) +#define __CMP_UNSAFE(a, b, op) ((a) op (b) ? (a) : (b)) + +#define __CMP_SAFE(a, b, op, var_a, var_b) ({ \ + __typeof__(__SELECT_NON_CONST(a, b)) var_a = a; \ + __typeof__(__SELECT_NON_CONST(b, a)) var_b = b; \ + var_a op var_b ? var_a : var_b; \ +}) + +#define __CMP(a, b, op) (__builtin_constant_p(a) && __builtin_constant_p(b) ? \ + __CMP_UNSAFE(a, b, op) : __CMP_SAFE(a, b, op, __TMPNAME, __TMPNAME)) + +#define MIN(a, b) __CMP(a, b, <) +#define MAX(a, b) __CMP(a, b, >) + +/* __typeof__ doesn't work on bitfield members, so provide these alternatives */ +#define MIN_UNSAFE(a, b) __CMP_UNSAFE(a, b, <) +#define MAX_UNSAFE(a, b) __CMP_UNSAFE(a, b, >) + #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
+#define DIV_ROUND_UP(x, y) ({ \ + __typeof__(x) _div_local_x = x; \ + __typeof__(y) _div_local_y = y; \ + ((_div_local_x) + (_div_local_y) - 1) / (_div_local_y); \ +}) + static inline u32 div_round_up(u32 n, u32 d) { return (n + d - 1) / d; }
#define LITTLE_ENDIAN 1234 diff --git a/payloads/libpayload/include/stdlib.h b/payloads/libpayload/include/stdlib.h index aaacc87..0b42ce2 100644 --- a/payloads/libpayload/include/stdlib.h +++ b/payloads/libpayload/include/stdlib.h @@ -35,8 +35,12 @@ #include <stddef.h> #include <string.h>
+#define __ALIGN_MASK(x, mask) ({ \ + typeof(mask) _align_mask_local_mask = mask; \ + ((x) + _align_mask_local_mask) & ~_align_mask_local_mask; \ +}) + #define ALIGN(x,a) __ALIGN_MASK(x,(typeof(x))(a)-1UL) -#define __ALIGN_MASK(x,mask) (((x)+(mask))&~(mask)) #define ALIGN_UP(x,a) ALIGN((x),(a)) #define ALIGN_DOWN(x,a) ((x) & ~((typeof(x))(a)-1UL)) #define IS_ALIGNED(x,a) (((x) & ((typeof(x))(a)-1UL)) == 0) diff --git a/src/commonlib/include/commonlib/compiler.h b/src/commonlib/include/commonlib/compiler.h index 2e0c454..e3d0f95 100644 --- a/src/commonlib/include/commonlib/compiler.h +++ b/src/commonlib/include/commonlib/compiler.h @@ -48,4 +48,18 @@ #define __always_inline inline __attribute__((always_inline)) #endif
+/* This evaluates to the first expression, unless that is constant in which case + it evalutates to the second. This is useful to write typeof() macros in a way + that they don't get hung up on the type of literal expressions (e.g. compare + an integer literal which is signed by default with a size_t variable). It is + generally a safe thing to do because if the constant doesn't fit into the + chosen type, you'll get a -Werror=overflow at compile time. */ +#define __SELECT_NON_CONST(expr, fallback_expr) __builtin_choose_expr( \ + __builtin_constant_p(expr), fallback_expr, expr) + +/* This creates a unique local variable name for use in macros. */ +#define __TMPNAME_3(i) __tmpname_##i +#define __TMPNAME_2(i) __TMPNAME_3(i) +#define __TMPNAME __TMPNAME_2(__COUNTER__) + #endif diff --git a/src/commonlib/include/commonlib/helpers.h b/src/commonlib/include/commonlib/helpers.h index adc43ca..65c122e 100644 --- a/src/commonlib/include/commonlib/helpers.h +++ b/src/commonlib/include/commonlib/helpers.h @@ -23,41 +23,76 @@ #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0])) #endif
+#define __ALIGN_MASK(x, mask) ({ \ + __typeof__(mask) _align_mask_local_mask = mask; \ + ((x) + _align_mask_local_mask) & ~_align_mask_local_mask; \ +}) + #define ALIGN(x, a) __ALIGN_MASK(x, (__typeof__(x))(a)-1UL) -#define __ALIGN_MASK(x, mask) (((x)+(mask))&~(mask)) #define ALIGN_UP(x, a) ALIGN((x), (a)) #define ALIGN_DOWN(x, a) ((x) & ~((__typeof__(x))(a)-1UL)) #define IS_ALIGNED(x, a) (((x) & ((__typeof__(x))(a)-1UL)) == 0)
+#define __CMP_UNSAFE(a, b, op) ((a) op (b) ? (a) : (b)) + +#define __CMP_SAFE(a, b, op, var_a, var_b) ({ \ + __typeof__(__SELECT_NON_CONST(a, b)) var_a = a; \ + __typeof__(__SELECT_NON_CONST(b, a)) var_b = b; \ + var_a op var_b ? var_a : var_b; \ +}) + +#define __CMP(a, b, op) __builtin_choose_expr( \ + __builtin_constant_p(a) && __builtin_constant_p(b), \ + __CMP_UNSAFE(a, b, op), __CMP_SAFE(a, b, op, __TMPNAME, __TMPNAME)) + #ifndef MIN -#define MIN(a, b) ((a) < (b) ? (a) : (b)) +#define MIN(a, b) __CMP(a, b, <) #endif #ifndef MAX -#define MAX(a, b) ((a) > (b) ? (a) : (b)) +#define MAX(a, b) __CMP(a, b, >) #endif -#define ABS(a) (((a) < 0) ? (-(a)) : (a)) -#define IS_POWER_OF_2(x) (((x) & ((x) - 1)) == 0) -#define DIV_ROUND_UP(x, y) (((x) + (y) - 1) / (y)) -#define SWAP(a, b) do { \ - typeof(a) tmp = a; \ - a = (typeof(a)) b; \ - b = (typeof(b)) tmp; \ - } while (0) + +/* __typeof__ doesn't work on bitfield members, so provide these alternatives */ +#define MIN_UNSAFE(a, b) __CMP_UNSAFE(a, b, <) +#define MAX_UNSAFE(a, b) __CMP_UNSAFE(a, b, >) + +#define ABS(a) ({ \ + __typeof__(a) _abs_local_a = a; \ + ((_abs_local_a) < 0) ? (-(_abs_local_a)) : (_abs_local_a); \ +}) + +#define IS_POWER_OF_2(x) ({ \ + __typeof__(x) _power_local_x = x; \ + ((_power_local_x) & ((_power_local_x) - 1)) == 0; \ +}) + +#define DIV_ROUND_UP(x, y) ({ \ + __typeof__(x) _div_local_x = x; \ + __typeof__(y) _div_local_y = y; \ + ((_div_local_x) + (_div_local_y) - 1) / (_div_local_y); \ +}) + +#define SWAP(a, b) do { \ + __typeof__(&a) _swap_local_a = &a; \ + __typeof__(&b) _swap_local_b = &b; \ + __typeof__(a) _swap_local_tmp = *_swap_local_a; \ + *_swap_local_a = *_swap_local_b; \ + *_swap_local_b = _swap_local_tmp; \ +} while (0) + /* * Divide positive or negative dividend by positive divisor and round * to closest integer. Result is undefined for negative divisors and * for negative dividends if the divisor variable type is unsigned. */ -#define DIV_ROUND_CLOSEST(x, divisor)( \ -{ \ - typeof(x) __x = x; \ - typeof(divisor) __d = divisor; \ - (((typeof(x))-1) > 0 || \ - ((typeof(divisor))-1) > 0 || (__x) > 0) ? \ - (((__x) + ((__d) / 2)) / (__d)) : \ - (((__x) - ((__d) / 2)) / (__d)); \ -} \ -) +#define DIV_ROUND_CLOSEST(x, divisor)({ \ + __typeof__(x) _div_local_x = x; \ + __typeof__(divisor) _div_local_d = divisor; \ + (((__typeof__(x))-1) > 0 || \ + ((__typeof__(divisor))-1) > 0 || (_div_local_x) > 0) ? \ + (((_div_local_x) + ((_div_local_d) / 2)) / (_div_local_d)) : \ + (((_div_local_x) - ((_div_local_d) / 2)) / (_div_local_d)); \ +})
/* Standard units. */ #define KiB (1<<10)