Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220821000737.328590235@goodmis.org> <20220821000844.510643400@goodmis.org>
 <CAHk-=wjsxu782N0P+oMu35N7rJAOeh3buQFWJaZHZTNmVSB=3Q@mail.gmail.com>
 <5700ac75-f6a9-877e-4011-9b314f12b5ab@acm.org> <CAHk-=wjqkWEr0MRO5hWuBoTDgNUj4qQK8V_Y36et=61mdPztJw@mail.gmail.com>
 <02daa3d6-2847-d7e0-e23e-411076c6d4db@rasmusvillemoes.dk> <0163b361-14bf-7b4c-751a-14f1a004b1a9@acm.org>
 <CAHk-=wjMLb30d0WT_RyKBCX+JBkg3QQU6pCYkrV8f58Ya4Rgzw@mail.gmail.com>
 <CAHk-=wiwr2Ff_1SKzRkjSbNLFYfk4KurvZhLuwVuTT-m9w5_6A@mail.gmail.com>
 <YwWIQ/3BDQHOiTek@ZenIV> <YwWWoQXmVc8uasBh@ZenIV>
In-Reply-To: <YwWWoQXmVc8uasBh@ZenIV>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Tue, 23 Aug 2022 22:56:21 -0700
Message-ID: <CAHk-=whmx4Pqw70NDmujGMQY1BrVi54pLnB5b7qpPThjiHatOw@mail.gmail.com>
Subject: Re: [for-linus][PATCH 01/10] tracing: Suppress sparse warnings
 triggered by is_signed_type()
To:     Al Viro <viro@zeniv.linux.org.uk>
Cc:     Bart Van Assche <bvanassche@acm.org>,
        Rasmus Villemoes <linux@rasmusvillemoes.dk>,
        Steven Rostedt <rostedt@goodmis.org>,
        linux-kernel@vger.kernel.org, Ingo Molnar <mingo@kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Christoph Hellwig <hch@lst.de>,
        Luc Van Oostenryck <luc.vanoostenryck@gmail.com>,
        Jens Axboe <axboe@kernel.dk>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Aug 23, 2022 at 8:10 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> Any operations like ordered comparisons would trigger unrestrict() on
> these suckers, which would warn and convert to underlying type.
>
> Your addition of "ordered comparison with 0 or -1" evades unrestrict().

No. Look. Try this modification to that test, and use
'./test-linearize' to see what sparse turns it into without my patch
to keep the signedness.

    static long test(void)
    {
        return (le32) 0xffffffff;
    }

yes, yes, it warns (twice, actually), but it also then generates

        ret.64      $-1

for that return.

Why? Because it thinks that 'le32' is a signed 32-bit thing due to the
clearing of the MOD_UNSIGNED bit, so when it casts it to 'long' it
will sign-extend it.

So the sign confusion exists and is visible regardless of the added
ordered comparison.

Now, we normally don't *notice* any of this, because we obviously
don't rely on sparse generating any code. And we _do_ cast those
bitwise things in many places, although we use "__force" to show that
it's intentional. Including, very much, those kinds of widening casts
where the signedness matters.

See for example very much the csum code:

    __wsum csum_partial(const void *buff, int len, __wsum wsum)
    {
        unsigned int sum = (__force unsigned int)wsum;

which is *exactly* that kind of code where it's fundamentally
important that 'wsum' is an unsigned type, and casting it to 'unsigned
int' does not sign-extend it.

So no. This has absolutely nothing to do with the new ordered comparisons.

Those bitwise types have always been integers, just with special rules
for warning about mis-using them.

And the sign handling has always been wrong.

It just so happens that me using 'test-linearize' to double-check what
sparse does for that signedness check *uncovered* that pre-existing
bug.

It was not introduced by the new code, and the ordered comparisons are
not AT ALL different from the equality comparisons, except for the
fact that they actually care about the signedness being right.

           Linus