Re: [RFC dwarves] syscall functions in BTF

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Fri, Mar 10, 2023 at 12:43:31PM +0000, Alan Maguire escreveu:
> On 10/03/2023 10:07, Jiri Olsa wrote:
> > hi,
> > with latest pahole fixes we get rid of some syscall functions (with
> > __x64_sys_ prefix) and it seems to fall down to 2 cases:
> > 
> > - weak syscall functions generated in kernel/sys_ni.c prevent these syscalls
> >   to be generated in BTF. The reason is the __COND_SYSCALL macro uses
> >   '__unused' for regs argument:
> > 
> >         #define __COND_SYSCALL(abi, name)                                      \
> >                __weak long __##abi##_##name(const struct pt_regs *__unused);   \
> >                __weak long __##abi##_##name(const struct pt_regs *__unused)    \
> >                {                                                               \
> >                        return sys_ni_syscall();                                \
> >                }
> > 
> >   and having weak function with different argument name will rule out the
> >   syscall from BTF functions
> > 
> >   the patch below workarounds this by using the same argument name,
> >   but I guess the real fix would be to check the whole type not just
> >   the argument name.. or ignore weak function if there's non weak one
> > 
> >   I guess there will be more cases like this in kernel
> > 
> >
 
> Thanks for the report Jiri! I'm working on reusing the dwarves_fprintf.c
> code to use string comparisons of function prototypes (minus parameter names!)
> instead as a more robust comparison.  Hope to have something working soon..

Humm, that could be an option, a simple strcmp after snprintf'ing the
function prototype, but there is also the type__compare_members_types()
approach, used to order types in pahole, the same could be done for
function prototypes?

I.e. to compare a function prototype for functions with the same name we
would check its return value type, the number of arguments and then each
of the arguments, continuing to consider the names as an heuristic that
functions with all being so far equal having different argument names
may indicate different functions, but if there is no name in both
functions, look at its type instead, where we then would use
type__compare_members_types() for structs/unions?

- Arnaldo
  
> > - we also do not get any syscall with no arguments, because they are
> >   generated as aliases to __do_<syscall> function:
> > 
> >         $ nm ./vmlinux | grep _sys_fork
> >         ffffffff81174890 t __do_sys_fork
> >         ffffffff81174890 T __ia32_sys_fork
> >         ffffffff81174880 T __pfx___x64_sys_fork
> >         ffffffff81174890 T __x64_sys_fork
> > 
> >   with:
> >         #define __SYS_STUB0(abi, name)                                          \
> >                 long __##abi##_##name(const struct pt_regs *regs);              \
> >                 ALLOW_ERROR_INJECTION(__##abi##_##name, ERRNO);                 \
> >                 long __##abi##_##name(const struct pt_regs *regs)               \
> >                         __alias(__do_##name);
> > 
> >   the problem seems to be that there's no DWARF data for aliased symbol,
> >   so pahole won't see any __x64_sys_fork record
> >   I'm not sure how to fix this one
> > 
> 
> Is this one a new issue, or did you just spot it when looking at the other case?
> 
> Thanks!
> 
> Alan
> 
> >   technically we can always connect to __do_sys_fork, but we'd need to
> >   have special cases for such syscalls.. would be great to have all with
> >   '__x64_sys_' prefix
> > 
> > 
> > thoughts?
> > 
> > thanks,
> > jirka
> > 
> > 
> > ---
> > diff --git a/arch/x86/include/asm/syscall_wrapper.h b/arch/x86/include/asm/syscall_wrapper.h
> > index fd2669b1cb2d..e02dab630577 100644
> > --- a/arch/x86/include/asm/syscall_wrapper.h
> > +++ b/arch/x86/include/asm/syscall_wrapper.h
> > @@ -80,8 +80,8 @@ extern long __ia32_sys_ni_syscall(const struct pt_regs *regs);
> >  	}
> >  
> >  #define __COND_SYSCALL(abi, name)					\
> > -	__weak long __##abi##_##name(const struct pt_regs *__unused);	\
> > -	__weak long __##abi##_##name(const struct pt_regs *__unused)	\
> > +	__weak long __##abi##_##name(const struct pt_regs *regs);	\
> > +	__weak long __##abi##_##name(const struct pt_regs *regs)	\
> >  	{								\
> >  		return sys_ni_syscall();				\
> >  	}
> > 

-- 

- Arnaldo