lkml.org 
[lkml]   [2018]   [Mar]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    SubjectRe: [PATCH v5 0/2] Remove false-positive VLAs when using max()
    On Fri, Mar 16, 2018 at 9:14 PM, Linus Torvalds
    <torvalds@linux-foundation.org> wrote:
    > On Fri, Mar 16, 2018 at 1:03 PM, Miguel Ojeda
    > <miguel.ojeda.sandonis@gmail.com> wrote:
    >>>
    >>> Kees - is there some online "gcc-4.4 checker" somewhere? This does
    >>> seem to work with my gcc. I actually tested some of those files you
    >>> pointed at now.
    >>
    >> I use this one:
    >>
    >> https://godbolt.org/
    >
    > Well, my *test* code works on that one and -Wvla -Werror.
    >
    > It does not work with gcc-4.1.x, but works with gcc-4.4.x.
    >
    > I can't seem to see the errors any way, I wonder if
    > __builtin_choose_expr() simply didn't exist back then.
    >
    > Odd that you can't view warnings/errors with it.
    >
    > But it's possible that it fails on more complex stuff in the kernel.
    >
    > I've done a "allmodconfig" build with that patch, and the only issue
    > it found was that (real) type issue in tpm_tis_core.h.

    Just tested your max() with a Python script I wrote yesterday to try a
    lot of combinations for this thing. Your version gives some warnings
    in some cases, like:

    warning: signed and unsigned type in conditional expression [-Wsign-compare]
    #define __cmp(a,b,op) ((a)op(b)?(a):(b))

    warning: comparison between signed and unsigned integer expressions
    [-Wsign-compare]
    #define const_max(a,b) __careful_cmp(a,b,>)

    warning: comparison of distinct pointer types lacks a cast
    (!!(sizeof((typeof(a)*)1==(typeof(b)*)1)))

    But it fails on something like (with warnings):

    int a[const_max(-30, 60u)];

    Sorry... :-(

    Has anyone taken a look at the last one I sent? Patch attached with
    the draft changes on the kernel. It compiles fine the cases Kees
    cleaned up in the other patch, but also works without a explicit type,
    for mixed types, and for both positive and negative values.

    Cheers,
    Miguel
    diff --git a/include/linux/kernel.h b/include/linux/kernel.h
    index 3fd291503576..f83658a4f15d 100644
    --- a/include/linux/kernel.h
    +++ b/include/linux/kernel.h
    @@ -819,6 +819,49 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
    __UNIQUE_ID(max1_), __UNIQUE_ID(max2_), \
    x, y)

    +size_t __error_not_const_arg(void) \
    +__compiletime_error("const_max() used with non-compile-time constant arg");
    +size_t __error_too_big(void) \
    +__compiletime_error("const_max() used with an arg too big");
    +
    +#define INTMAXT_MAX LLONG_MAX
    +typedef int64_t intmax_t;
    +
    +#define const_cmp(x, y, op) \
    + __builtin_choose_expr( \
    + !__builtin_constant_p(x) || !__builtin_constant_p(y), \
    + __error_not_const_arg(), \
    + __builtin_choose_expr( \
    + (x) > INTMAXT_MAX || (y) > INTMAXT_MAX, \
    + __error_too_big(), \
    + __builtin_choose_expr( \
    + (intmax_t)(x) op (intmax_t)(y), \
    + (x), \
    + (y) \
    + ) \
    + ) \
    + )
    +
    +/**
    + * const_min - returns minimum of two compile-time constant values
    + * @x: first compile-time constant value
    + * @y: second compile-time constant value
    + *
    + * The result has the type of the winner of the comparison. Works for any
    + * value up to LLONG_MAX.
    + */
    +#define const_min(x, y) const_cmp((x), (y), <=)
    +
    +/**
    + * const_max - returns maximum of two compile-time constant values
    + * @x: first compile-time constant value
    + * @y: second compile-time constant value
    + *
    + * The result has the type of the winner of the comparison. Works for any
    + * value up to LLONG_MAX.
    + */
    +#define const_max(x, y) const_cmp((x), (y), >=)
    +
    /**
    * min3 - return minimum of three values
    * @x: first value
    \
     
     \ /
      Last update: 2018-03-17 02:51    [W:2.829 / U:0.468 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site