Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 24 Mar 2021 18:33:39 +0100
From:   Peter Zijlstra <peterz@infradead.org>
To:     Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Cc:     Sami Tolvanen <samitolvanen@google.com>,
        Rasmus Villemoes <linux@rasmusvillemoes.dk>,
        Steven Rostedt <rostedt@goodmis.org>,
        Arnd Bergmann <arnd@kernel.org>,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        Jason Baron <jbaron@akamai.com>,
        Ingo Molnar <mingo@redhat.com>,
        Juri Lelli <juri.lelli@redhat.com>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Ard Biesheuvel <ardb@kernel.org>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
        Daniel Bristot de Oliveira <bristot@redhat.com>,
        Frederic Weisbecker <frederic@kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] static_call: fix function type mismatch
Message-ID: <YFt382FImjQQ+10f@hirez.programming.kicks-ass.net>
References: <20210322170711.1855115-1-arnd@kernel.org>
 <20210322153214.25d869b1@gandalf.local.home>
 <YFkCZuOwe37d2bV+@hirez.programming.kicks-ass.net>
 <CAK8P3a2sz4emewH_HA+nsf0e5tP6qtAxhBOFucmzW4OPDJASdQ@mail.gmail.com>
 <20210322172921.56350a69@gandalf.local.home>
 <YFmdJlESrCh4iC9A@hirez.programming.kicks-ass.net>
 <0f4679d6-44a4-d045-f249-a9cffb126fd4@rasmusvillemoes.dk>
 <CABCJKuf1-GWda9_BiBO=nNP_drh3a8471G+LEqPzdVrLBhVqZQ@mail.gmail.com>
 <b2d77e78-751e-283c-8cff-e9c4f16e27ef@prevas.dk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <b2d77e78-751e-283c-8cff-e9c4f16e27ef@prevas.dk>
Precedence: bulk

On Wed, Mar 24, 2021 at 05:45:52PM +0100, Rasmus Villemoes wrote:
> Sorry, I think I misread the code. The static calls are indeed
> initialized with a function with the right prototype. Try adding
> "preempt=full" on the command line so that we exercise these lines
> 
>                static_call_update(cond_resched,
> (typeof(&__cond_resched)) __static_call_return0);
>                 static_call_update(might_resched,
> (typeof(&__cond_resched)) __static_call_return0);
> 
> I would expect that to blow up, since we end up calling a long (*)(void)
> function using a function pointer of type int (*)(void).

Note that on x86 there won't actually be any calling of function
pointers. See what arch/x86/kernel/static_call.c does :-)

But I think some of this code might need some __va_function() love when
combined with CFI.

But yes, this is why I think something like -fcdecl might be a good
idea, that ought to tell the compiler about the calling convention,
which ought to be enough for the compiler to figure out that this magic
really is ok.

Notable things we rely on:

 - caller cleanup of stack; the function caller sets up any stack
   arguments and is also responsible for cleanin up the stack once the
   function returns.

 - the return value is in a register.

Per the first we can call a function that has a partial (empty per
extremum) argument list. Per the second we can call a function with a
different return type as long as they all fit in the same register.

The calling of a 'long (*)()' function for a 'int (*)()' type then
becomes idential to something like: 'int x = (long)y', and that is
something C is perfectly fine with.

We then slightly push things with the other __static_call_return0()
usage in the kernel, where we basically end up with: 'void *x =
(long)y', which is something C really rather would have a cast on.