On Fri, Nov 30, 2018 at 11:16:34PM +0100, Rasmus Villemoes wrote:
> On 29/11/2018 20.22, Josh Poimboeuf wrote:
> > On Thu, Nov 29, 2018 at 02:16:48PM -0500, Steven Rostedt wrote:
> >>> and honestly, the way "static_call()" works now, can you guarantee
> >>> that the call-site doesn't end up doing that, and calling the
> >>> trampoline function for two different static calls from one indirect
> >>> call?
> >>>
> >>> See what I'm talking about? Saying "callers are wrapped in macros"
> >>> doesn't actually protect you from the compiler doing things like that.
> >>>
> >>> In contrast, if the call was wrapped in an inline asm, we'd *know* the
> >>> compiler couldn't turn a "call wrapper(%rip)" into anything else.
> >>
> >> But then we need to implement all numbers of parameters.
> >
> > I actually have an old unfinished patch which (ab)used C macros to
> > detect the number of parameters and then setup the asm constraints
> > accordingly. At the time, the goal was to optimize the BUG code.
> >
> > I had wanted to avoid this kind of approach for static calls, because
> > "ugh", but now it's starting to look much more appealing.
> >
> > Behold:
> >
> > diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
> > index aa6b2023d8f8..d63e9240da77 100644
> > --- a/arch/x86/include/asm/bug.h
> > +++ b/arch/x86/include/asm/bug.h
> > @@ -32,10 +32,59 @@
> >
> > #ifdef CONFIG_DEBUG_BUGVERBOSE
> >
> > -#define _BUG_FLAGS(ins, flags)
> > \
> > +#define __BUG_ARGS_0(ins, ...) \
> > +({\
> > + asm volatile("1:\t" ins "\n"); \
> > +})
> > +#define __BUG_ARGS_1(ins, ...) \
> > +({\
> > + asm volatile("1:\t" ins "\n" \
> > + : : "D" (ARG1(__VA_ARGS__))); \
> > +})
> > +#define __BUG_ARGS_2(ins, ...) \
> > +({\
> > + asm volatile("1:\t" ins "\n" \
> > + : : "D" (ARG1(__VA_ARGS__)), \
> > + "S" (ARG2(__VA_ARGS__))); \
> > +})
> > +#define __BUG_ARGS_3(ins, ...) \
> > +({\
> > + asm volatile("1:\t" ins "\n" \
> > + : : "D" (ARG1(__VA_ARGS__)), \
> > + "S" (ARG2(__VA_ARGS__)), \
> > + "d" (ARG3(__VA_ARGS__))); \
> > +})
>
> wouldn't you need to tie all these to (unused) outputs as well as adding
> the remaining caller-saved registers to the clobber list? Maybe not for
> the WARN machinery(?), but at least for stuff that should look like a
> normal call to gcc? Then there's %rax which is either a clobber or an
> output, and if there's not to be a separate static_call_void(), one
> would need to do some __builtin_choose_expr(__same_type(void, f(...)), ...).
Yes, this is a crappy unfinished patch. It should be ignored, and
perhaps even mercilessly mocked :-)
paravirt_types.h already does something similar today, and it's at least
more correct than this.
What I was trying to show was that you can use macros to count
arguments, like this:
_BUG_ARGS(ins, NUM_ARGS(__VA_ARGS__), __VA_ARGS__);
which can make a macro look and act like a function call. Though as
Steven pointed out, the concept falls apart after 6 arguments.
--
Josh
