On 10/03/2023 15:09, Jiri Olsa wrote:
> On Fri, Mar 10, 2023 at 12:43:31PM +0000, Alan Maguire wrote:
>> 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..
>
> great, I saw the patchset, will check
>
>>
>>> - 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?
>
> I was trying to attach to all syscalls and noticed some where missing,
> it looks like the alias was used in this place for few years
>
I looked into this one; in the DWARF associated with these functions I see:
<1><756d2>: Abbrev Number: 24 (DW_TAG_subprogram)
<756d3> DW_AT_external : 1
<756d3> DW_AT_name : (indirect string, offset: 0x8552d): __x64_sys_fork
<756d7> DW_AT_decl_file : 7
<756d8> DW_AT_decl_line : 58
<756d9> DW_AT_decl_column : 1
<756da> DW_AT_prototyped : 1
<756da> DW_AT_type : <0x73abd>
<756de> DW_AT_declaration : 1
<756de> DW_AT_sibling : <0x756e8>
<2><756e2>: Abbrev Number: 18 (DW_TAG_formal_parameter)
<756e3> DW_AT_type : <0x73d42>
So it's marked as a declaration, and BTF encoding has skipped declarations
for a while now as they don't have parameter names; I checked out v1.24 of
dwarves and only __do_sys_fork() is encoded in BTF there too. Looks like
the last relevant change around the syscall stubs was in 2020:
d2b5de495ee9 ("x86/entry: Refactor SYSCALL_DEFINE0 macros")
Alan
