This is an archive of the discontinued LLVM Phabricator instance.

[libc] add syscall function
ClosedPublic

Authored by michaelrj on Sep 29 2022, 3:52 PM.

Details

Summary

Add the syscall wrapper function and tests. It's implemented using a
macro to guarantee the minimum number of arguments.

Diff Detail

Event Timeline

michaelrj created this revision.Sep 29 2022, 3:52 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptSep 29 2022, 3:52 PM
michaelrj requested review of this revision.Sep 29 2022, 3:52 PM

rebase to include new additions

Can we call it syscall_impl instead of internal_syscall ?

libc/include/llvm-libc-macros/linux/unistd-macros.h
22

For better type checking, we should do something like this:

#define __syscall_helper(sysno, arg1, arg2, arg3, arg4, arg5, arg6, ...) \
    __llvm_libc_syscall((long)(sysno), (long)(arg1), (long)(arg2), (long)(arg3), \
                        (long)(arg4), (long)(arg5), (long)(arg6))
#define syscall(...) __syscall_helper(__VA_ARGS__, 1, 2, 3, 4, 5, 6, 7)
libc/src/unistd/linux/syscall.cpp
19

This need not be a vararg function - it can take exactly 7 arguments, the syscall number plus 6 other args.

libc/test/src/unistd/syscall_test.cpp
28

This is very misleading as there is no member named syscall in the namespace __llvm_libc. So, can you add a comment below line 22 explaining how the macro substitution helps in actually picking the namespace qualified __llvm_libc_syscall.

lntue accepted this revision.Sep 30 2022, 11:42 AM
This revision is now accepted and ready to land.Sep 30 2022, 11:42 AM
michaelrj updated this revision to Diff 464379.Sep 30 2022, 1:20 PM
michaelrj marked 3 inline comments as done.

rename internal function, redo macro definition for type safety

sivachandra accepted this revision.Sep 30 2022, 2:11 PM
sivachandra added inline comments.
libc/include/llvm-libc-macros/linux/unistd-macros.h
22

If you don't add a 7th argument below on line 25, will the macro on line 22 work when invoked like this: syscall(SYS_getpid)?

michaelrj updated this revision to Diff 464420.Sep 30 2022, 3:45 PM
michaelrj marked an inline comment as done.

minor fix

libc/include/llvm-libc-macros/linux/unistd-macros.h
22

As far as I can tell it's fine in C++20, but possibly not in other versions (which is why my tests were passing), so I've added the 7th argument back as 0, to make it clearer that 1 matches to arg1, and so on.

This revision was landed with ongoing or failed builds.Sep 30 2022, 3:46 PM
This revision was automatically updated to reflect the committed changes.