diff mbox

[v6] kernel.h: Retain constant expression output for max()/min()

Message ID 20180326221554.GA45166@beast (mailing list archive)
State New, archived
Headers show

Commit Message

Kees Cook March 26, 2018, 10:15 p.m. UTC 
In the effort to remove all VLAs from the kernel[1], it is desirable to
build with -Wvla. However, this warning is overly pessimistic, in that
it is only happy with stack array sizes that are declared as constant
expressions, and not constant values. One case of this is the evaluation
of the max() macro which, due to its construction, ends up converting
constant expression arguments into a constant value result.

All attempts to rewrite this macro with __builtin_constant_p() failed with
older compilers (e.g. gcc 4.4)[2]. However, Martin Uecker constructed[3] a
mind-shattering solution that works everywhere. Cthulhu fhtagn!

This patch updates the min()/max() macros to evaluate to a constant
expression when called on constant expression arguments. This removes
several false-positive stack VLA warnings from an x86 allmodconfig
build when -Wvla is added:

$ diff -u before.txt after.txt | grep ^-
-drivers/input/touchscreen/cyttsp4_core.c:871:2: warning: ISO C90 forbids variable length array ‘ids’ [-Wvla]
-fs/btrfs/tree-checker.c:344:4: warning: ISO C90 forbids variable length array ‘namebuf’ [-Wvla]
-lib/vsprintf.c:747:2: warning: ISO C90 forbids variable length array ‘sym’ [-Wvla]
-net/ipv4/proc.c:403:2: warning: ISO C90 forbids variable length array ‘buff’ [-Wvla]
-net/ipv6/proc.c:198:2: warning: ISO C90 forbids variable length array ‘buff’ [-Wvla]
-net/ipv6/proc.c:218:2: warning: ISO C90 forbids variable length array ‘buff64’ [-Wvla]

This also updates the one case where different enums were being compared
and explicitly casts them to int (which matches the old side-effect of
the single-evaluation code).

[1] https://lkml.org/lkml/2018/3/7/621
[2] https://lkml.org/lkml/2018/3/10/170
[3] https://lkml.org/lkml/2018/3/20/845

Co-Developed-by: Linus Torvalds <torvalds@linux-foundation.org>
Co-Developed-by: Martin Uecker <Martin.Uecker@med.uni-goettingen.de>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
Let's see if this one sticks!
---
 drivers/char/tpm/tpm_tis_core.h |  8 ++---
 include/linux/kernel.h          | 71 ++++++++++++++++++++++++-----------------
 2 files changed, 45 insertions(+), 34 deletions(-)

Comments

Linus Torvalds March 27, 2018, 12:52 a.m. UTC | #1 
On Mon, Mar 26, 2018 at 12:15 PM, Kees Cook <keescook@chromium.org> wrote:
>
> This patch updates the min()/max() macros to evaluate to a constant
> expression when called on constant expression arguments.

Ack.

I'm of two minds whether that "__is_constant()" macro should be
explained or not.

A small voice in my head says "that wants a comment".

But a bigger voice disagrees.

It is a work of art, and maybe the best documentation is just the
name. It does what it says it does.

Art shouldn't be explained. It should be appreciated.

Nobody sane really should care about how it works, and if somebody
cares it is "left as an exercise to the reader".

      Linus
Ingo Molnar March 27, 2018, 5:47 a.m. UTC | #2 
* Kees Cook <keescook@chromium.org> wrote:

> In the effort to remove all VLAs from the kernel[1], it is desirable to
> build with -Wvla. However, this warning is overly pessimistic, in that
> it is only happy with stack array sizes that are declared as constant
> expressions, and not constant values. One case of this is the evaluation
> of the max() macro which, due to its construction, ends up converting
> constant expression arguments into a constant value result.
> 
> All attempts to rewrite this macro with __builtin_constant_p() failed with
> older compilers (e.g. gcc 4.4)[2]. However, Martin Uecker constructed[3] a
> mind-shattering solution that works everywhere. Cthulhu fhtagn!
> 
> This patch updates the min()/max() macros to evaluate to a constant
> expression when called on constant expression arguments. This removes
> several false-positive stack VLA warnings from an x86 allmodconfig
> build when -Wvla is added:

Cool!

Acked-by: Ingo Molnar <mingo@kernel.org>

How many warnings are left in an allmodconfig build?

Thanks,

	Ingo
David Laight March 27, 2018, 8:55 a.m. UTC | #3 
From: Kees Cook

> Sent: 26 March 2018 23:16

...
> +#define __typecheck(x, y) \

> +		(!!(sizeof((typeof(x)*)1 == (typeof(y)*)1)))


The two 1 should probably be at least 8 before the compiler starts
bleating about accesses to misaligned addresses being undefined.

	David
Miguel Ojeda March 27, 2018, 10:03 a.m. UTC | #4 
On Tue, Mar 27, 2018 at 12:15 AM, Kees Cook <keescook@chromium.org> wrote:
> In the effort to remove all VLAs from the kernel[1], it is desirable to
> build with -Wvla. However, this warning is overly pessimistic, in that
> it is only happy with stack array sizes that are declared as constant
> expressions, and not constant values. One case of this is the evaluation
> of the max() macro which, due to its construction, ends up converting
> constant expression arguments into a constant value result.
>
> All attempts to rewrite this macro with __builtin_constant_p() failed with
> older compilers (e.g. gcc 4.4)[2]. However, Martin Uecker constructed[3] a
> mind-shattering solution that works everywhere. Cthulhu fhtagn!
>
> This patch updates the min()/max() macros to evaluate to a constant
> expression when called on constant expression arguments. This removes
> several false-positive stack VLA warnings from an x86 allmodconfig
> build when -Wvla is added:
>
> $ diff -u before.txt after.txt | grep ^-
> -drivers/input/touchscreen/cyttsp4_core.c:871:2: warning: ISO C90 forbids variable length array ‘ids’ [-Wvla]
> -fs/btrfs/tree-checker.c:344:4: warning: ISO C90 forbids variable length array ‘namebuf’ [-Wvla]
> -lib/vsprintf.c:747:2: warning: ISO C90 forbids variable length array ‘sym’ [-Wvla]
> -net/ipv4/proc.c:403:2: warning: ISO C90 forbids variable length array ‘buff’ [-Wvla]
> -net/ipv6/proc.c:198:2: warning: ISO C90 forbids variable length array ‘buff’ [-Wvla]
> -net/ipv6/proc.c:218:2: warning: ISO C90 forbids variable length array ‘buff64’ [-Wvla]
>
> This also updates the one case where different enums were being compared
> and explicitly casts them to int (which matches the old side-effect of
> the single-evaluation code).
>
> [1] https://lkml.org/lkml/2018/3/7/621
> [2] https://lkml.org/lkml/2018/3/10/170
> [3] https://lkml.org/lkml/2018/3/20/845
>
> Co-Developed-by: Linus Torvalds <torvalds@linux-foundation.org>
> Co-Developed-by: Martin Uecker <Martin.Uecker@med.uni-goettingen.de>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> Let's see if this one sticks!
> ---
>  drivers/char/tpm/tpm_tis_core.h |  8 ++---
>  include/linux/kernel.h          | 71 ++++++++++++++++++++++++-----------------
>  2 files changed, 45 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> index d5c6a2e952b3..f6e1dbe212a7 100644
> --- a/drivers/char/tpm/tpm_tis_core.h
> +++ b/drivers/char/tpm/tpm_tis_core.h
> @@ -62,10 +62,10 @@ enum tis_defaults {
>  /* Some timeout values are needed before it is known whether the chip is
>   * TPM 1.0 or TPM 2.0.
>   */
> -#define TIS_TIMEOUT_A_MAX      max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_A)
> -#define TIS_TIMEOUT_B_MAX      max(TIS_LONG_TIMEOUT, TPM2_TIMEOUT_B)
> -#define TIS_TIMEOUT_C_MAX      max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C)
> -#define TIS_TIMEOUT_D_MAX      max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_D)
> +#define TIS_TIMEOUT_A_MAX      max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_A)
> +#define TIS_TIMEOUT_B_MAX      max_t(int, TIS_LONG_TIMEOUT, TPM2_TIMEOUT_B)
> +#define TIS_TIMEOUT_C_MAX      max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C)
> +#define TIS_TIMEOUT_D_MAX      max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_D)
>
>  #define        TPM_ACCESS(l)                   (0x0000 | ((l) << 12))
>  #define        TPM_INT_ENABLE(l)               (0x0008 | ((l) << 12))
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 3fd291503576..a2c1b2a382dd 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -783,41 +783,58 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
>  #endif /* CONFIG_TRACING */
>
>  /*
> - * min()/max()/clamp() macros that also do
> - * strict type-checking.. See the
> - * "unnecessary" pointer comparison.
> + * min()/max()/clamp() macros must accomplish three things:
> + *
> + * - avoid multiple evaluations of the arguments (so side-effects like
> + *   "x++" happen only once) when non-constant.
> + * - perform strict type-checking (to generate warnings instead of
> + *   nasty runtime surprises). See the "unnecessary" pointer comparison
> + *   in __typecheck().
> + * - retain result as a constant expressions when called with only
> + *   constant expressions (to avoid tripping VLA warnings in stack
> + *   allocation usage).
> + */
> +#define __typecheck(x, y) \
> +               (!!(sizeof((typeof(x)*)1 == (typeof(y)*)1)))
> +
> +/*
> + * This returns a constant expression while determining if an argument is
> + * a constant expression, most importantly without evaluating the argument.
> + * Glory to Martin Uecker <Martin.Uecker@med.uni-goettingen.de>
>   */
> -#define __min(t1, t2, min1, min2, x, y) ({             \
> -       t1 min1 = (x);                                  \
> -       t2 min2 = (y);                                  \
> -       (void) (&min1 == &min2);                        \
> -       min1 < min2 ? min1 : min2; })
> +#define __is_constant(x) \
> +       (sizeof(int) == sizeof(*(1 ? ((void*)((long)(x) * 0l)) : (int*)1)))

I think Linus suggested __is_constant, but what about __is_constexpr
instead? It makes the intention a bit more clear and follows the C++'s
specifier.

Acked-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>

Cheers,
Miguel

> +
> +#define __no_side_effects(x, y) \
> +               (__is_constant(x) && __is_constant(y))
> +
> +#define __safe_cmp(x, y) \
> +               (__typecheck(x, y) && __no_side_effects(x, y))
> +
> +#define __cmp(x, y, op)        ((x) op (y) ? (x) : (y))
> +
> +#define __cmp_once(x, y, op) ({                \
> +               typeof(x) __x = (x);    \
> +               typeof(y) __y = (y);    \
> +               __cmp(__x, __y, op); })
> +
> +#define __careful_cmp(x, y, op)                                \
> +               __builtin_choose_expr(__safe_cmp(x, y), \
> +                                     __cmp(x, y, op), __cmp_once(x, y, op))
>
>  /**
>   * min - return minimum of two values of the same or compatible types
>   * @x: first value
>   * @y: second value
>   */
> -#define min(x, y)                                      \
> -       __min(typeof(x), typeof(y),                     \
> -             __UNIQUE_ID(min1_), __UNIQUE_ID(min2_),   \
> -             x, y)
> -
> -#define __max(t1, t2, max1, max2, x, y) ({             \
> -       t1 max1 = (x);                                  \
> -       t2 max2 = (y);                                  \
> -       (void) (&max1 == &max2);                        \
> -       max1 > max2 ? max1 : max2; })
> +#define min(x, y)      __careful_cmp(x, y, <)
>
>  /**
>   * max - return maximum of two values of the same or compatible types
>   * @x: first value
>   * @y: second value
>   */
> -#define max(x, y)                                      \
> -       __max(typeof(x), typeof(y),                     \
> -             __UNIQUE_ID(max1_), __UNIQUE_ID(max2_),   \
> -             x, y)
> +#define max(x, y)      __careful_cmp(x, y, >)
>
>  /**
>   * min3 - return minimum of three values
> @@ -869,10 +886,7 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
>   * @x: first value
>   * @y: second value
>   */
> -#define min_t(type, x, y)                              \
> -       __min(type, type,                               \
> -             __UNIQUE_ID(min1_), __UNIQUE_ID(min2_),   \
> -             x, y)
> +#define min_t(t, x, y) __careful_cmp((t)(x), (t)(y), <)
>
>  /**
>   * max_t - return maximum of two values, using the specified type
> @@ -880,10 +894,7 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
>   * @x: first value
>   * @y: second value
>   */
> -#define max_t(type, x, y)                              \
> -       __max(type, type,                               \
> -             __UNIQUE_ID(min1_), __UNIQUE_ID(min2_),   \
> -             x, y)
> +#define max_t(t, x, y) __careful_cmp((t)(x), (t)(y), >)
>
>  /**
>   * clamp_t - return a value clamped to a given range using a given type
> --
> 2.7.4
>
>
> --
> Kees Cook
> Pixel Security
kernel test robot March 27, 2018, 10:43 a.m. UTC | #5 
Hi Kees,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.16-rc7 next-20180326]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Kees-Cook/kernel-h-Retain-constant-expression-output-for-max-min/20180327-110916
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org)
   include/linux/kernel.h:889: warning: Excess function parameter 'type' description in 'min_t'
   include/linux/kernel.h:897: warning: Excess function parameter 'type' description in 'max_t'
   include/linux/kernel.h:889: warning: Excess function parameter 'type' description in 'min_t'
   include/linux/kernel.h:897: warning: Excess function parameter 'type' description in 'max_t'
   include/linux/crypto.h:477: warning: Function parameter or member 'cra_u.ablkcipher' not described in 'crypto_alg'
   include/linux/crypto.h:477: warning: Function parameter or member 'cra_u.blkcipher' not described in 'crypto_alg'
   include/linux/crypto.h:477: warning: Function parameter or member 'cra_u.cipher' not described in 'crypto_alg'
   include/linux/crypto.h:477: warning: Function parameter or member 'cra_u.compress' not described in 'crypto_alg'
   include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.ibss' not described in 'wireless_dev'
   include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.connect' not described in 'wireless_dev'
   include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.keys' not described in 'wireless_dev'
   include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.ie' not described in 'wireless_dev'
   include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.ie_len' not described in 'wireless_dev'
   include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.bssid' not described in 'wireless_dev'
   include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.prev_bssid' not described in 'wireless_dev'
   include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.ssid' not described in 'wireless_dev'
   include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.default_key' not described in 'wireless_dev'
   include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.default_mgmt_key' not described in 'wireless_dev'
   include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.prev_bssid_valid' not described in 'wireless_dev'
   include/net/mac80211.h:2263: warning: Function parameter or member 'radiotap_timestamp.units_pos' not described in 'ieee80211_hw'
   include/net/mac80211.h:2263: warning: Function parameter or member 'radiotap_timestamp.accuracy' not described in 'ieee80211_hw'
   include/net/mac80211.h:950: warning: Function parameter or member 'rates' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'control.rts_cts_rate_idx' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'control.use_rts' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'control.use_cts_prot' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'control.short_preamble' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'control.skip_table' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'control.jiffies' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'control.vif' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'control.hw_key' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'control.flags' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'control.enqueue_time' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'ack' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'ack.cookie' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'status.rates' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'status.ack_signal' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'status.ampdu_ack_len' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'status.ampdu_len' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'status.antenna' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'status.tx_time' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'status_driver_data' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'driver_rates' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'pad' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:950: warning: Function parameter or member 'rate_driver_data' not described in 'ieee80211_tx_info'
   net/mac80211/sta_info.h:584: warning: Function parameter or member 'rx_stats_avg' not described in 'sta_info'
   net/mac80211/sta_info.h:584: warning: Function parameter or member 'rx_stats_avg.signal' not described in 'sta_info'
   net/mac80211/sta_info.h:584: warning: Function parameter or member 'rx_stats_avg.chain_signal' not described in 'sta_info'
   net/mac80211/sta_info.h:584: warning: Function parameter or member 'status_stats.filtered' not described in 'sta_info'
   net/mac80211/sta_info.h:584: warning: Function parameter or member 'status_stats.retry_failed' not described in 'sta_info'
   net/mac80211/sta_info.h:584: warning: Function parameter or member 'status_stats.retry_count' not described in 'sta_info'
   net/mac80211/sta_info.h:584: warning: Function parameter or member 'status_stats.lost_packets' not described in 'sta_info'
   net/mac80211/sta_info.h:584: warning: Function parameter or member 'status_stats.last_tdls_pkt_time' not described in 'sta_info'
   net/mac80211/sta_info.h:584: warning: Function parameter or member 'msdu_retries' not described in 'sta_info'
   net/mac80211/sta_info.h:584: warning: Function parameter or member 'msdu_failed' not described in 'sta_info'
   net/mac80211/sta_info.h:584: warning: Function parameter or member 'status_stats.last_ack' not described in 'sta_info'
   net/mac80211/sta_info.h:584: warning: Function parameter or member 'tx_stats.packets' not described in 'sta_info'
   net/mac80211/sta_info.h:584: warning: Function parameter or member 'tx_stats.bytes' not described in 'sta_info'
   net/mac80211/sta_info.h:584: warning: Function parameter or member 'tx_stats.last_rate' not described in 'sta_info'
   net/mac80211/sta_info.h:584: warning: Function parameter or member 'msdu' not described in 'sta_info'
>> include/linux/kernel.h:890: warning: Function parameter or member 't' not described in 'min_t'
   include/linux/kernel.h:890: warning: Excess function parameter 'type' description in 'min_t'
>> include/linux/kernel.h:898: warning: Function parameter or member 't' not described in 'max_t'
   include/linux/kernel.h:898: warning: Excess function parameter 'type' description in 'max_t'
   include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.cb' not described in 'dma_buf'
   include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.poll' not described in 'dma_buf'
   include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.active' not described in 'dma_buf'
   include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.cb' not described in 'dma_buf'
   include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.poll' not described in 'dma_buf'
   include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.active' not described in 'dma_buf'
   include/linux/dma-fence-array.h:54: warning: Function parameter or member 'work' not described in 'dma_fence_array'
   include/linux/gpio/driver.h:142: warning: Function parameter or member 'request_key' not described in 'gpio_irq_chip'
   include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.sign' not described in 'iio_chan_spec'
   include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.realbits' not described in 'iio_chan_spec'
   include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.storagebits' not described in 'iio_chan_spec'
   include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.shift' not described in 'iio_chan_spec'
   include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.repeat' not described in 'iio_chan_spec'
   include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.endianness' not described in 'iio_chan_spec'
   include/linux/iio/hw-consumer.h:1: warning: no structured comments found
   include/linux/device.h:294: warning: Function parameter or member 'coredump' not described in 'device_driver'
   include/linux/input/sparse-keymap.h:46: warning: Function parameter or member 'sw' not described in 'key_entry'
   include/linux/mtd/rawnand.h:709: warning: Function parameter or member 'timings.sdr' not described in 'nand_data_interface'
   include/linux/mtd/rawnand.h:774: warning: Function parameter or member 'buf' not described in 'nand_op_data_instr'
   include/linux/mtd/rawnand.h:774: warning: Function parameter or member 'buf.in' not described in 'nand_op_data_instr'
   include/linux/mtd/rawnand.h:774: warning: Function parameter or member 'buf.out' not described in 'nand_op_data_instr'
   include/linux/mtd/rawnand.h:820: warning: Function parameter or member 'ctx' not described in 'nand_op_instr'
   include/linux/mtd/rawnand.h:820: warning: Function parameter or member 'ctx.cmd' not described in 'nand_op_instr'
   include/linux/mtd/rawnand.h:820: warning: Function parameter or member 'ctx.addr' not described in 'nand_op_instr'
   include/linux/mtd/rawnand.h:820: warning: Function parameter or member 'ctx.data' not described in 'nand_op_instr'
   include/linux/mtd/rawnand.h:820: warning: Function parameter or member 'ctx.waitrdy' not described in 'nand_op_instr'
   include/linux/mtd/rawnand.h:967: warning: Function parameter or member 'ctx' not described in 'nand_op_parser_pattern_elem'
   include/linux/mtd/rawnand.h:967: warning: Function parameter or member 'ctx.addr' not described in 'nand_op_parser_pattern_elem'
   include/linux/mtd/rawnand.h:967: warning: Function parameter or member 'ctx.data' not described in 'nand_op_parser_pattern_elem'
   include/linux/mtd/rawnand.h:1281: warning: Function parameter or member 'manufacturer.desc' not described in 'nand_chip'
   include/linux/mtd/rawnand.h:1281: warning: Function parameter or member 'manufacturer.priv' not described in 'nand_chip'
   include/linux/regulator/driver.h:221: warning: Function parameter or member 'resume_early' not described in 'regulator_ops'
   drivers/regulator/core.c:4299: warning: Excess function parameter 'state' description in 'regulator_suspend_late'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw0' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw1' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw2' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw3' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.eadm' not described in 'irb'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_pin' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_unpin' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_res_obj' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_get_sg_table' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_import_sg_table' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_vmap' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_vunmap' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_mmap' not described in 'drm_driver'
   include/drm/drm_connector.h:370: warning: Function parameter or member 'margins.left' not described in 'drm_tv_connector_state'
   include/drm/drm_connector.h:370: warning: Function parameter or member 'margins.right' not described in 'drm_tv_connector_state'
   include/drm/drm_connector.h:370: warning: Function parameter or member 'margins.top' not described in 'drm_tv_connector_state'
   include/drm/drm_connector.h:370: warning: Function parameter or member 'margins.bottom' not described in 'drm_tv_connector_state'
   include/drm/drm_vblank.h:63: warning: Function parameter or member 'event.base' not described in 'drm_pending_vblank_event'
   include/drm/drm_vblank.h:63: warning: Function parameter or member 'event.vbl' not described in 'drm_pending_vblank_event'
   include/drm/drm_vblank.h:63: warning: Function parameter or member 'event.seq' not described in 'drm_pending_vblank_event'
   include/linux/skbuff.h:846: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff'
   include/linux/skbuff.h:846: warning: Function parameter or member 'skb_mstamp' not described in 'sk_buff'
   include/linux/skbuff.h:846: warning: Function parameter or member '__cloned_offset' not described in 'sk_buff'
   include/linux/skbuff.h:846: warning: Function parameter or member 'head_frag' not described in 'sk_buff'
   include/linux/skbuff.h:846: warning: Function parameter or member '__unused' not described in 'sk_buff'
   include/linux/skbuff.h:846: warning: Function parameter or member '__pkt_type_offset' not described in 'sk_buff'
   include/linux/skbuff.h:846: warning: Function parameter or member 'pfmemalloc' not described in 'sk_buff'
   include/linux/skbuff.h:846: warning: Function parameter or member 'encapsulation' not described in 'sk_buff'
   include/linux/skbuff.h:846: warning: Function parameter or member 'encap_hdr_csum' not described in 'sk_buff'
   include/linux/skbuff.h:846: warning: Function parameter or member 'csum_valid' not described in 'sk_buff'
   include/linux/skbuff.h:846: warning: Function parameter or member 'csum_complete_sw' not described in 'sk_buff'
   include/linux/skbuff.h:846: warning: Function parameter or member 'csum_level' not described in 'sk_buff'
   include/linux/skbuff.h:846: warning: Function parameter or member 'inner_protocol_type' not described in 'sk_buff'
   include/linux/skbuff.h:846: warning: Function parameter or member 'remcsum_offload' not described in 'sk_buff'
   include/linux/skbuff.h:846: warning: Function parameter or member 'offload_fwd_mark' not described in 'sk_buff'
   include/linux/skbuff.h:846: warning: Function parameter or member 'offload_mr_fwd_mark' not described in 'sk_buff'
   include/linux/skbuff.h:846: warning: Function parameter or member 'sender_cpu' not described in 'sk_buff'
   include/linux/skbuff.h:846: warning: Function parameter or member 'reserved_tailroom' not described in 'sk_buff'
   include/linux/skbuff.h:846: warning: Function parameter or member 'inner_ipproto' not described in 'sk_buff'
   include/net/sock.h:234: warning: Function parameter or member 'skc_addrpair' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_portpair' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_ipv6only' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_net_refcnt' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_v6_daddr' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_v6_rcv_saddr' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_cookie' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_listener' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_tw_dr' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_rcv_wnd' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_tw_rcv_nxt' not described in 'sock_common'
   include/net/sock.h:487: warning: Function parameter or member 'sk_backlog.rmem_alloc' not described in 'sock'
   include/net/sock.h:487: warning: Function parameter or member 'sk_backlog.len' not described in 'sock'
   include/net/sock.h:487: warning: Function parameter or member 'sk_backlog.head' not described in 'sock'
   include/net/sock.h:487: warning: Function parameter or member 'sk_backlog.tail' not described in 'sock'
   include/net/sock.h:487: warning: Function parameter or member 'sk_wq_raw' not described in 'sock'
   include/net/sock.h:487: warning: Function parameter or member 'tcp_rtx_queue' not described in 'sock'
   include/linux/netdevice.h:1940: warning: Function parameter or member 'adj_list.upper' not described in 'net_device'
   include/linux/netdevice.h:1940: warning: Function parameter or member 'adj_list.lower' not described in 'net_device'
   include/linux/netdevice.h:1940: warning: Function parameter or member 'gso_partial_features' not described in 'net_device'
   include/linux/netdevice.h:1940: warning: Function parameter or member 'switchdev_ops' not described in 'net_device'
   include/linux/netdevice.h:1940: warning: Function parameter or member 'l3mdev_ops' not described in 'net_device'
   include/linux/netdevice.h:1940: warning: Function parameter or member 'xfrmdev_ops' not described in 'net_device'
   include/linux/netdevice.h:1940: warning: Function parameter or member 'name_assign_type' not described in 'net_device'
   include/linux/netdevice.h:1940: warning: Function parameter or member 'ieee802154_ptr' not described in 'net_device'
   include/linux/netdevice.h:1940: warning: Function parameter or member 'mpls_ptr' not described in 'net_device'
   include/linux/netdevice.h:1940: warning: Function parameter or member 'xdp_prog' not described in 'net_device'

vim +890 include/linux/kernel.h

589a9785e Johannes Berg   2016-10-07 @890  
e8c97af0c Randy Dunlap    2017-10-13  891  /**
e8c97af0c Randy Dunlap    2017-10-13  892   * max_t - return maximum of two values, using the specified type
e8c97af0c Randy Dunlap    2017-10-13  893   * @type: data type to use
e8c97af0c Randy Dunlap    2017-10-13  894   * @x: first value
e8c97af0c Randy Dunlap    2017-10-13  895   * @y: second value
e8c97af0c Randy Dunlap    2017-10-13  896   */
2ac4e7c3a Kees Cook       2018-03-26  897  #define max_t(t, x, y)	__careful_cmp((t)(x), (t)(y), >)
bdf4bbaae Harvey Harrison 2008-04-30 @898  

:::::: The code at line 890 was first introduced by commit
:::::: 589a9785ee3a7cb85f1dedc3dad1c9754c691880 min/max: remove sparse warnings when they're nested

:::::: TO: Johannes Berg <johannes.berg@intel.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Uecker, Martin March 27, 2018, noon UTC | #6 
To give credit where credit is due, this hack was inspired by 
an equally insane (but different) use of the ?: operator to choose 
the right return type for type-generic macros in tgmath.h.

https://sourceware.org/git/?p=glibc.git;a=blob;f=math/tgmath.h;h=a709a5
9d0fa1168ef03349561169fc5bd27d65aa;hb=d8742dd82f6a00601155c69bad3012e90
5591e1f

(recommendation: don't look)

Martin


Am Montag, den 26.03.2018, 14:52 -1000 schrieb Linus Torvalds:
> On Mon, Mar 26, 2018 at 12:15 PM, Kees Cook <keescook@chromium.org>

> wrote:

> > 

> > This patch updates the min()/max() macros to evaluate to a constant

> > expression when called on constant expression arguments.

> 

> Ack.

> 

> I'm of two minds whether that "__is_constant()" macro should be

> explained or not.

> 

> A small voice in my head says "that wants a comment".

> 

> But a bigger voice disagrees.

> 

> It is a work of art, and maybe the best documentation is just the

> name. It does what it says it does.

> 

> Art shouldn't be explained. It should be appreciated.

> 

> Nobody sane really should care about how it works, and if somebody

> cares it is "left as an exercise to the reader".

> 

>       Linus
kernel test robot March 27, 2018, 6:39 p.m. UTC | #7 
Hi Kees,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.16-rc7 next-20180327]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Kees-Cook/kernel-h-Retain-constant-expression-output-for-max-min/20180327-110916
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


vim +124 drivers/infiniband/hw/vmw_pvrdma/pvrdma_doorbell.c

29c8d9eb Adit Ranadive 2016-10-02  114  
29c8d9eb Adit Ranadive 2016-10-02  115  void pvrdma_uar_free(struct pvrdma_dev *dev, struct pvrdma_uar_map *uar)
29c8d9eb Adit Ranadive 2016-10-02  116  {
29c8d9eb Adit Ranadive 2016-10-02  117  	struct pvrdma_id_table *tbl = &dev->uar_table.tbl;
29c8d9eb Adit Ranadive 2016-10-02  118  	unsigned long flags;
29c8d9eb Adit Ranadive 2016-10-02  119  	u32 obj;
29c8d9eb Adit Ranadive 2016-10-02  120  
29c8d9eb Adit Ranadive 2016-10-02  121  	obj = uar->index & (tbl->max - 1);
29c8d9eb Adit Ranadive 2016-10-02  122  	spin_lock_irqsave(&tbl->lock, flags);
29c8d9eb Adit Ranadive 2016-10-02  123  	clear_bit(obj, tbl->table);
29c8d9eb Adit Ranadive 2016-10-02 @124  	tbl->last = min(tbl->last, obj);

:::::: The code at line 124 was first introduced by commit
:::::: 29c8d9eba550c6d73d17cc1618a9f5f2a7345aa1 IB: Add vmw_pvrdma driver

:::::: TO: Adit Ranadive <aditr@vmware.com>
:::::: CC: Doug Ledford <dledford@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Kees Cook March 31, 2018, 4:57 a.m. UTC | #8 
On Mon, Mar 26, 2018 at 10:47 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Kees Cook <keescook@chromium.org> wrote:
>
>> In the effort to remove all VLAs from the kernel[1], it is desirable to
>> build with -Wvla. However, this warning is overly pessimistic, in that
>> it is only happy with stack array sizes that are declared as constant
>> expressions, and not constant values. One case of this is the evaluation
>> of the max() macro which, due to its construction, ends up converting
>> constant expression arguments into a constant value result.
>>
>> All attempts to rewrite this macro with __builtin_constant_p() failed with
>> older compilers (e.g. gcc 4.4)[2]. However, Martin Uecker constructed[3] a
>> mind-shattering solution that works everywhere. Cthulhu fhtagn!
>>
>> This patch updates the min()/max() macros to evaluate to a constant
>> expression when called on constant expression arguments. This removes
>> several false-positive stack VLA warnings from an x86 allmodconfig
>> build when -Wvla is added:
>
> Cool!
>
> Acked-by: Ingo Molnar <mingo@kernel.org>
>
> How many warnings are left in an allmodconfig build?

For -Wvla? Out of the original 112 files with VLAs, 42 haven't had a
patch applied yet. Doing a linux-next allmodconfig build with the
max() patch and my latest ecc patch, we've gone from 316 warning
instances to 205. More than half of those are in
include/crypto/skcipher.h and include/crypto/hash.h.

-Kees
Ingo Molnar March 31, 2018, 5:34 a.m. UTC | #9 
* Kees Cook <keescook@chromium.org> wrote:

> On Mon, Mar 26, 2018 at 10:47 PM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > * Kees Cook <keescook@chromium.org> wrote:
> >
> >> In the effort to remove all VLAs from the kernel[1], it is desirable to
> >> build with -Wvla. However, this warning is overly pessimistic, in that
> >> it is only happy with stack array sizes that are declared as constant
> >> expressions, and not constant values. One case of this is the evaluation
> >> of the max() macro which, due to its construction, ends up converting
> >> constant expression arguments into a constant value result.
> >>
> >> All attempts to rewrite this macro with __builtin_constant_p() failed with
> >> older compilers (e.g. gcc 4.4)[2]. However, Martin Uecker constructed[3] a
> >> mind-shattering solution that works everywhere. Cthulhu fhtagn!
> >>
> >> This patch updates the min()/max() macros to evaluate to a constant
> >> expression when called on constant expression arguments. This removes
> >> several false-positive stack VLA warnings from an x86 allmodconfig
> >> build when -Wvla is added:
> >
> > Cool!
> >
> > Acked-by: Ingo Molnar <mingo@kernel.org>
> >
> > How many warnings are left in an allmodconfig build?
> 
> For -Wvla? Out of the original 112 files with VLAs, 42 haven't had a
> patch applied yet. Doing a linux-next allmodconfig build with the
> max() patch and my latest ecc patch, we've gone from 316 warning
> instances to 205. More than half of those are in
> include/crypto/skcipher.h and include/crypto/hash.h.

Great - once the number of warnings is zero, is the plan to enable the warning 
unconditionally?

Thanks,

	Ingo
Kees Cook March 31, 2018, 1:37 p.m. UTC | #10 
On Fri, Mar 30, 2018 at 10:34 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Kees Cook <keescook@chromium.org> wrote:
>
>> On Mon, Mar 26, 2018 at 10:47 PM, Ingo Molnar <mingo@kernel.org> wrote:
>> >
>> > * Kees Cook <keescook@chromium.org> wrote:
>> >
>> >> In the effort to remove all VLAs from the kernel[1], it is desirable to
>> >> build with -Wvla. However, this warning is overly pessimistic, in that
>> >> it is only happy with stack array sizes that are declared as constant
>> >> expressions, and not constant values. One case of this is the evaluation
>> >> of the max() macro which, due to its construction, ends up converting
>> >> constant expression arguments into a constant value result.
>> >>
>> >> All attempts to rewrite this macro with __builtin_constant_p() failed with
>> >> older compilers (e.g. gcc 4.4)[2]. However, Martin Uecker constructed[3] a
>> >> mind-shattering solution that works everywhere. Cthulhu fhtagn!
>> >>
>> >> This patch updates the min()/max() macros to evaluate to a constant
>> >> expression when called on constant expression arguments. This removes
>> >> several false-positive stack VLA warnings from an x86 allmodconfig
>> >> build when -Wvla is added:
>> >
>> > Cool!
>> >
>> > Acked-by: Ingo Molnar <mingo@kernel.org>
>> >
>> > How many warnings are left in an allmodconfig build?
>>
>> For -Wvla? Out of the original 112 files with VLAs, 42 haven't had a
>> patch applied yet. Doing a linux-next allmodconfig build with the
>> max() patch and my latest ecc patch, we've gone from 316 warning
>> instances to 205. More than half of those are in
>> include/crypto/skcipher.h and include/crypto/hash.h.
>
> Great - once the number of warnings is zero, is the plan to enable the warning
> unconditionally?

That's the plan, yes. Like inbox-zero but for VLAs. ;)

-Kees
diff mbox

Patch

diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index d5c6a2e952b3..f6e1dbe212a7 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -62,10 +62,10 @@  enum tis_defaults {
 /* Some timeout values are needed before it is known whether the chip is
  * TPM 1.0 or TPM 2.0.
  */
-#define TIS_TIMEOUT_A_MAX	max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_A)
-#define TIS_TIMEOUT_B_MAX	max(TIS_LONG_TIMEOUT, TPM2_TIMEOUT_B)
-#define TIS_TIMEOUT_C_MAX	max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C)
-#define TIS_TIMEOUT_D_MAX	max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_D)
+#define TIS_TIMEOUT_A_MAX	max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_A)
+#define TIS_TIMEOUT_B_MAX	max_t(int, TIS_LONG_TIMEOUT, TPM2_TIMEOUT_B)
+#define TIS_TIMEOUT_C_MAX	max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C)
+#define TIS_TIMEOUT_D_MAX	max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_D)
 
 #define	TPM_ACCESS(l)			(0x0000 | ((l) << 12))
 #define	TPM_INT_ENABLE(l)		(0x0008 | ((l) << 12))
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 3fd291503576..a2c1b2a382dd 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -783,41 +783,58 @@  static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
 #endif /* CONFIG_TRACING */
 
 /*
- * min()/max()/clamp() macros that also do
- * strict type-checking.. See the
- * "unnecessary" pointer comparison.
+ * min()/max()/clamp() macros must accomplish three things:
+ *
+ * - avoid multiple evaluations of the arguments (so side-effects like
+ *   "x++" happen only once) when non-constant.
+ * - perform strict type-checking (to generate warnings instead of
+ *   nasty runtime surprises). See the "unnecessary" pointer comparison
+ *   in __typecheck().
+ * - retain result as a constant expressions when called with only
+ *   constant expressions (to avoid tripping VLA warnings in stack
+ *   allocation usage).
+ */
+#define __typecheck(x, y) \
+		(!!(sizeof((typeof(x)*)1 == (typeof(y)*)1)))
+
+/*
+ * This returns a constant expression while determining if an argument is
+ * a constant expression, most importantly without evaluating the argument.
+ * Glory to Martin Uecker <Martin.Uecker@med.uni-goettingen.de>
  */
-#define __min(t1, t2, min1, min2, x, y) ({		\
-	t1 min1 = (x);					\
-	t2 min2 = (y);					\
-	(void) (&min1 == &min2);			\
-	min1 < min2 ? min1 : min2; })
+#define __is_constant(x) \
+	(sizeof(int) == sizeof(*(1 ? ((void*)((long)(x) * 0l)) : (int*)1)))
+
+#define __no_side_effects(x, y) \
+		(__is_constant(x) && __is_constant(y))
+
+#define __safe_cmp(x, y) \
+		(__typecheck(x, y) && __no_side_effects(x, y))
+
+#define __cmp(x, y, op)	((x) op (y) ? (x) : (y))
+
+#define __cmp_once(x, y, op) ({		\
+		typeof(x) __x = (x);	\
+		typeof(y) __y = (y);	\
+		__cmp(__x, __y, op); })
+
+#define __careful_cmp(x, y, op)				\
+		__builtin_choose_expr(__safe_cmp(x, y),	\
+				      __cmp(x, y, op), __cmp_once(x, y, op))
 
 /**
  * min - return minimum of two values of the same or compatible types
  * @x: first value
  * @y: second value
  */
-#define min(x, y)					\
-	__min(typeof(x), typeof(y),			\
-	      __UNIQUE_ID(min1_), __UNIQUE_ID(min2_),	\
-	      x, y)
-
-#define __max(t1, t2, max1, max2, x, y) ({		\
-	t1 max1 = (x);					\
-	t2 max2 = (y);					\
-	(void) (&max1 == &max2);			\
-	max1 > max2 ? max1 : max2; })
+#define min(x, y)	__careful_cmp(x, y, <)
 
 /**
  * max - return maximum of two values of the same or compatible types
  * @x: first value
  * @y: second value
  */
-#define max(x, y)					\
-	__max(typeof(x), typeof(y),			\
-	      __UNIQUE_ID(max1_), __UNIQUE_ID(max2_),	\
-	      x, y)
+#define max(x, y)	__careful_cmp(x, y, >)
 
 /**
  * min3 - return minimum of three values
@@ -869,10 +886,7 @@  static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
  * @x: first value
  * @y: second value
  */
-#define min_t(type, x, y)				\
-	__min(type, type,				\
-	      __UNIQUE_ID(min1_), __UNIQUE_ID(min2_),	\
-	      x, y)
+#define min_t(t, x, y)	__careful_cmp((t)(x), (t)(y), <)
 
 /**
  * max_t - return maximum of two values, using the specified type
@@ -880,10 +894,7 @@  static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
  * @x: first value
  * @y: second value
  */
-#define max_t(type, x, y)				\
-	__max(type, type,				\
-	      __UNIQUE_ID(min1_), __UNIQUE_ID(min2_),	\
-	      x, y)
+#define max_t(t, x, y)	__careful_cmp((t)(x), (t)(y), >)
 
 /**
  * clamp_t - return a value clamped to a given range using a given type