| Message ID | 20221222035134.3467659-1-ammar.faizi@intel.com |
|---|---|
| State | New |
| Headers | show |
Hi Ammar, On Thu, Dec 22, 2022 at 08:46:15PM +0700, Ammar Faizi wrote: > I agree with following the @envp pointer to get the auxv. I was > trying to wire up a new function '__start' (with double underscores) > written in C that accepts @argc, @argv and @envp. Then it calls 'main'. > Then we call '__start' instead of 'main' from '_start'. This way, we > can arrange nolibc-defined data without touching Assembly much in > '__start' (before main). > > But then I noticed that it wouldn't work because we may have users > who define the 'main' function differently, e.g.: > > int main(void); > int main(int argc, char **argv); > int main(int argc, char **argv, char **envp); > > So '__start' can't call main. We still need to call the main from the > inline Assembly (from '_start'). Yes, and quite frankly I prefer to make that the least complicated. Doing just a simple loop in the _start code is trivial. The main concern was to store the data. Till now we had an optional .bss section, we didn't save environ and errno was optional. But let's be honest, while it does allow for writing the smallest programs, most programs will have at least one global variable and will get this section anyway, so we don't save anything in practice. This concern used to be valid when I was making tiny executables when running on floppies where each byte mattered, but now that's pointless. Thus what I'm proposing is to switch to weak symbol definitions for errno, environ, and auxv. I did a quick test to make sure that the same symbol was properly used when accessed from two units and that's OK, I'm seeing the same instance for all of them (which is better than the current situation where errno is static, hence per-unit). My quick-and-dirty test looks like this: diff --git a/arch-x86_64.h b/arch-x86_64.h index e780fdf..73f7b5f 100644 --- a/arch-x86_64.h +++ b/arch-x86_64.h @@ -209,6 +209,9 @@ struct sys_stat_struct { _ret; \ }) +char **environ __attribute__((weak,unused)); +long *auxv __attribute__((weak,unused)); + /* startup code */ /* * x86-64 System V ABI mandates: @@ -218,11 +221,17 @@ struct sys_stat_struct { */ asm(".section .text\n" ".weak _start\n" "_start:\n" "pop %rdi\n" // argc (first arg, %rdi) "mov %rsp, %rsi\n" // argv[] (second arg, %rsi) "lea 8(%rsi,%rdi,8),%rdx\n" // then a NULL then envp (third arg, %rdx) + "mov %rdx, environ\n" // save environ "xor %ebp, %ebp\n" // zero the stack frame + "mov %rdx, %rax\n" // search for auxv (follows NULL after last en> + "0: add $8, %rax\n" + " cmp -8(%rax), %rbp\n" + " jnz 0b\n" + "mov %rax, auxv\n" // save auxv "and $-16, %rsp\n" // x86 ABI : esp must be 16-byte aligned befor> "call main\n" // main() returns the status code, we'll exit > "mov %eax, %edi\n" // retrieve exit code (32 bit) diff --git a/errno.h b/errno.h index df0e473..9781077 100644 --- a/errno.h +++ b/errno.h @@ -29,7 +29,8 @@ #include <asm/errno.h> /* this way it will be removed if unused */ -static int errno; +//static int errno; +int errno __attribute__((weak)); #ifndef NOLIBC_IGNORE_ERRNO #define SET_ERRNO(v) do { errno = (v); } while (0) $ cat a.c #include "nolibc.h" extern void b(void); int main(int argc, char **argv, char **envp) { //environ = envp; errno = 1234; printf("main(): errno=%d env(TERM)=%s auxv=%p auxv[0].t=0x%lx auxv[0].v=0x%lx\n", errno, getenv("TERM"), auxv, auxv?auxv[0]:0, auxv?auxv[1]:0); b(); return 0; } $ cat b.c #include "nolibc.h" void b(void) { long *v = auxv; printf("b(): errno=%d env(TERM)=%s auxv=%p auxv[0].t=0x%lx auxv[0].v=0x%lx\n", errno, getenv("TERM"), auxv, auxv?auxv[0]:0, auxv?auxv[1]:0); printf("auxv:\n"); while (v && v[0]) { printf(" 0x%lx: 0x%lx\n", v[0], v[1]); v += 2; } } $ gcc -Os -fno-asynchronous-unwind-tables -include /g/public/nolibc/nolibc.h -Wall -nostdlib -static -o ab a.c b.c $ nm --size ab 0000000000000004 V errno 0000000000000008 V auxv 0000000000000008 V environ 0000000000000014 W memset 0000000000000018 W memcpy 0000000000000018 W raise 000000000000001b W abort 0000000000000030 W memmove 0000000000000053 t u64toa_r 0000000000000053 t u64toa_r 0000000000000082 T main 00000000000000a4 T b 0000000000000289 t printf 000000000000028c t printf.constprop.0 $ ./ab main(): errno=1234 env(TERM)=xterm auxv=0x7ffdd0c31df8 auxv[0].t=0x21 auxv[0].v=0x7ffdd0d56000 b(): errno=1234 env(TERM)=xterm auxv=0x7ffdd0c31df8 auxv[0].t=0x21 auxv[0].v=0x7ffdd0d56000 auxv: 0x21: 0x7ffdd0d56000 0x10: 0xbfebfbff 0x6: 0x1000 0x11: 0x64 0x3: 0x400040 0x4: 0x38 0x5: 0x7 0x7: 0x0 0x8: 0x0 0x9: 0x401082 0xb: 0x1fd 0xc: 0x1fd 0xd: 0x64 0xe: 0x64 0x17: 0x0 0x19: 0x7ffdd0c31f39 0x1a: 0x2 0x1f: 0x7ffdd0c33ff3 0xf: 0x7ffdd0c31f49 Note that I could verify that some of the entries above are valid (e.g. "x86_64" in 0xf = AT_PLATFORM). Thus now my focus will be on storing these variables where relevant for all archs, so that your getauxval() implementation works on top of it. It will be much cleaner and will also improve programs' ease of implementation and reliability. Cheers, Willy PS: maybe we should trim the Cc list for future exchanges.
On 12/27/22 1:26 PM, Willy Tarreau wrote: > Yes, and quite frankly I prefer to make that the least complicated. > Doing just a simple loop in the _start code is trivial. The main > concern was to store the data. Till now we had an optional .bss > section, we didn't save environ and errno was optional. But let's > be honest, while it does allow for writing the smallest programs, > most programs will have at least one global variable and will get > this section anyway, so we don't save anything in practice. This > concern used to be valid when I was making tiny executables when > running on floppies where each byte mattered, but now that's pointless. > > Thus what I'm proposing is to switch to weak symbol definitions for > errno, environ, and auxv. I did a quick test to make sure that the same > symbol was properly used when accessed from two units and that's OK, I'm > seeing the same instance for all of them (which is better than the current > situation where errno is static, hence per-unit). Looks good to me. > Thus now my focus will be on storing these variables where relevant > for all archs, so that your getauxval() implementation works on top > of it. It will be much cleaner and will also improve programs' ease > of implementation and reliability. Are you going to wire up a patchset for it? If so, I'll wait for it. When it's already committed, I'll base this series on top it. Or I take your series locally then submit your patches and mine in a single series. What do you prefer?
On 12/27/22 8:32 PM, Ammar Faizi wrote: >> Thus now my focus will be on storing these variables where relevant >> for all archs, so that your getauxval() implementation works on top >> of it. It will be much cleaner and will also improve programs' ease >> of implementation and reliability. > > Are you going to wire up a patchset for it? > > If so, I'll wait for it. When it's already committed, I'll base this > series on top it. > > Or I take your series locally then submit your patches and mine in a > single series. > > What do you prefer? Side note: I only know x86 Assembly. So unfortunately, I can't get them working on all arch(s).
On Tue, Dec 27, 2022 at 08:32:57PM +0700, Ammar Faizi wrote: > > Thus now my focus will be on storing these variables where relevant > > for all archs, so that your getauxval() implementation works on top > > of it. It will be much cleaner and will also improve programs' ease > > of implementation and reliability. > > Are you going to wire up a patchset for it? > > If so, I'll wait for it. When it's already committed, I'll base this > series on top it. > > Or I take your series locally then submit your patches and mine in a > single series. > > What do you prefer? I'll try to do it but do not want to make you wait too long in case it gets delayed. In the worst case we should only postpone the getauxval() patch and not the other ones. BTW, do you think your arch-specific changes for sigaction() will be easily portable to other architectures ? I feel a bit wary of starting to have different features per architecture given the purpose of the lib, so the more uniform the coverage the better. I'll get back to you soon. Thanks, Willy
On Tue, Dec 27, 2022 at 08:36:41PM +0700, Ammar Faizi wrote: > On 12/27/22 8:32 PM, Ammar Faizi wrote: > > > Thus now my focus will be on storing these variables where relevant > > > for all archs, so that your getauxval() implementation works on top > > > of it. It will be much cleaner and will also improve programs' ease > > > of implementation and reliability. > > > > Are you going to wire up a patchset for it? > > > > If so, I'll wait for it. When it's already committed, I'll base this > > series on top it. > > > > Or I take your series locally then submit your patches and mine in a > > single series. > > > > What do you prefer? > > Side note: > I only know x86 Assembly. So unfortunately, I can't get them working > on all arch(s). The nice thing about assembly is that once you know one, others are easy to learn to permit you to write code that you can test. You can have a look at MIPS for delayed slots, SPARC for register banks, ARM for instructions that do multiple operations at once and you'll have seen most of the basics that you'll ever need. Also all of these are RISC based and cannot load a full-length register in a single instruction, that's possibly the most confusing thing when you come from x86. And it's also very interesting to see differences in constraints ;-) Willy
On 12/28/22 1:49 AM, Willy Tarreau wrote: > I'll try to do it but do not want to make you wait too long in case it > gets delayed. In the worst case we should only postpone the getauxval() > patch and not the other ones. I will split it into 2 patchset then. > BTW, do you think your arch-specific changes for sigaction() will be > easily portable to other architectures ? I feel a bit wary of starting > to have different features per architecture given the purpose of the > lib, so the more uniform the coverage the better. The 'rt_sigaction()' itself doesn't seem to be an arch specific, but the way it resumes the execution needs to call 'rt_sigreturn()' which is arch specific. I took a look at the kernel source code, most architectures read 'struct rt_sigframe' from the stack pointer. https://github.com/torvalds/linux/blob/631aa744423173bf921191ba695bbc7c1aabd9e0/arch/x86/kernel/signal_32.c#L145 https://github.com/torvalds/linux/blob/631aa744423173bf921191ba695bbc7c1aabd9e0/arch/x86/kernel/signal_64.c#L243-L271 https://github.com/torvalds/linux/blob/a6b450573b912316ad36262bfc70e7c3870c56d1/arch/arm64/kernel/signal.c#L668-L699 https://github.com/torvalds/linux/blob/a6b450573b912316ad36262bfc70e7c3870c56d1/arch/arm64/kernel/signal32.c#L259 https://github.com/torvalds/linux/blob/eb67d239f3aa1711afb0a42eab50459d9f3d672e/arch/riscv/kernel/signal.c#L101 On the x86-64 arch, the implementation is just like this: __arch_restore_rt: # # ((%rsp - sizeof(long)) must point to 'struct rt_sigframe') # # 'struct rt_sigframe' is automatically constructed by # the kernel when a signal is caught. # movl $0xf, %eax // __NR_rt_sigreturn == 0xf syscall I believe aarch64 and RISCV don't behave differently, but different registers. Not sure what PowerPC does here, it seems a bit different: https://github.com/torvalds/linux/blob/1612c382ffbdf1f673caec76502b1c00e6d35363/arch/powerpc/kernel/signal_64.c#L744 I haven't taken a look at other archs. What do you think? Is it affordable for nolibc to implement all of these?
On 12/28/22 1:58 AM, Willy Tarreau wrote: > The nice thing about assembly is that once you know one, others are > easy to learn to permit you to write code that you can test. You can > have a look at MIPS for delayed slots, SPARC for register banks, ARM > for instructions that do multiple operations at once and you'll have > seen most of the basics that you'll ever need. Also all of these are > RISC based and cannot load a full-length register in a single instruction, > that's possibly the most confusing thing when you come from x86. And > it's also very interesting to see differences in constraints ;-) Sounds fun. I'll try to get involved with other arch(s). But before that, I have to prepare the environment. At least virtualization that emulates those arch(s).
On Wed, Dec 28, 2022 at 07:01:36PM +0700, Ammar Faizi wrote: > On 12/28/22 1:49 AM, Willy Tarreau wrote: > > I'll try to do it but do not want to make you wait too long in case it > > gets delayed. In the worst case we should only postpone the getauxval() > > patch and not the other ones. > > I will split it into 2 patchset then. OK thanks! I've pushed for you an update which starts to do what I proposed. Errno and environ are now marked weak for all archs, and _auxv is set for i386, x86_64, arm64 and arm for now: https://git.kernel.org/pub/scm/linux/kernel/git/wtarreau/nolibc.git/log/?h=20221227-nolibc-weak-2 You can already use it to implement getauxval(), it will normally work for these archs. > > BTW, do you think your arch-specific changes for sigaction() will be > > easily portable to other architectures ? I feel a bit wary of starting > > to have different features per architecture given the purpose of the > > lib, so the more uniform the coverage the better. > > The 'rt_sigaction()' itself doesn't seem to be an arch specific, but > the way it resumes the execution needs to call 'rt_sigreturn()' which > is arch specific. I took a look at the kernel source code, most > architectures read 'struct rt_sigframe' from the stack pointer. > > https://github.com/torvalds/linux/blob/631aa744423173bf921191ba695bbc7c1aabd9e0/arch/x86/kernel/signal_32.c#L145 > https://github.com/torvalds/linux/blob/631aa744423173bf921191ba695bbc7c1aabd9e0/arch/x86/kernel/signal_64.c#L243-L271 > https://github.com/torvalds/linux/blob/a6b450573b912316ad36262bfc70e7c3870c56d1/arch/arm64/kernel/signal.c#L668-L699 > https://github.com/torvalds/linux/blob/a6b450573b912316ad36262bfc70e7c3870c56d1/arch/arm64/kernel/signal32.c#L259 > https://github.com/torvalds/linux/blob/eb67d239f3aa1711afb0a42eab50459d9f3d672e/arch/riscv/kernel/signal.c#L101 > > On the x86-64 arch, the implementation is just like this: > > __arch_restore_rt: > # > # ((%rsp - sizeof(long)) must point to 'struct rt_sigframe') > # > # 'struct rt_sigframe' is automatically constructed by > # the kernel when a signal is caught. > # > movl $0xf, %eax // __NR_rt_sigreturn == 0xf > syscall I think we could avoid the asm specific stuff is we get rid of the frame pointer. Please look below: __attribute__((weak,unused,noreturn,optimize("omit-frame-pointer"),section(".text.nolibc_rt_sigreturn"))) void sys_rt_sigreturn() { my_syscall0(__NR_rt_sigreturn); __builtin_unreachable(); } It gives me the correct code for x86_64 and i586. I don't know if other architectures will want to add a prologue. I tried with "naked" but it's ignored by the compiler since the function is not purely asm. Not very important but given that we already have everything to perform our calls it would make sense to stay on this. By the way, for the sake of consistency with other syscalls, I do think the function (or label if we can't do otherwise) should be called "sys_rt_sigreturn" as it just performs a syscall. > I believe aarch64 and RISCV don't behave differently, but different > registers. > > Not sure what PowerPC does here, it seems a bit different: > https://github.com/torvalds/linux/blob/1612c382ffbdf1f673caec76502b1c00e6d35363/arch/powerpc/kernel/signal_64.c#L744 It looks similar to me, it's just that the kernel side differs but I think it's the same. > I haven't taken a look at other archs. > > What do you think? Is it affordable for nolibc to implement all of > these? Yes I think so. I suspect that we might need to have a few arch-specific implementations, but we've already had this case a few times and we could easily use a pair of #define/#ifdef to skip the generic version. Best regards, Willy
On 12/28/22 8:35 PM, Willy Tarreau wrote: > OK thanks! > > I've pushed for you an update which starts to do what I proposed. Errno > and environ are now marked weak for all archs, and _auxv is set for i386, > x86_64, arm64 and arm for now: > > https://git.kernel.org/pub/scm/linux/kernel/git/wtarreau/nolibc.git/log/?h=20221227-nolibc-weak-2 > > You can already use it to implement getauxval(), it will normally work > for these archs. Will do and be back with two patch series. > I think we could avoid the asm specific stuff is we get rid of the frame > pointer. Please look below: > > __attribute__((weak,unused,noreturn,optimize("omit-frame-pointer"),section(".text.nolibc_rt_sigreturn"))) > void sys_rt_sigreturn() > { > my_syscall0(__NR_rt_sigreturn); > __builtin_unreachable(); > } Wow! You just taught me that we can force optimize a function with optimize("omit-frame-pointer") attribute. Nice to know this one! I compile-tested it and it indeed gives the correct code on x86-64. Hopefully this approach works for all archs. > It gives me the correct code for x86_64 and i586. I don't know if other > architectures will want to add a prologue. I tried with "naked" but it's > ignored by the compiler since the function is not purely asm. Not very > important but given that we already have everything to perform our calls > it would make sense to stay on this. By the way, for the sake of > consistency with other syscalls, I do think the function (or label if > we can't do otherwise) should be called "sys_rt_sigreturn" as it just > performs a syscall. Will call that 'sys_rt_sigreturn' in the next series. Thanks!
On Thu, Dec 29, 2022 at 6:42 PM Ammar Faizi wrote: > On 12/28/22 8:35 PM, Willy Tarreau wrote: > > It gives me the correct code for x86_64 and i586. I don't know if other > > architectures will want to add a prologue. I tried with "naked" but it's > > ignored by the compiler since the function is not purely asm. Not very > > important but given that we already have everything to perform our calls > > it would make sense to stay on this. By the way, for the sake of > > consistency with other syscalls, I do think the function (or label if > > we can't do otherwise) should be called "sys_rt_sigreturn" as it just > > performs a syscall. > > Will call that 'sys_rt_sigreturn' in the next series.
On Tue, Jan 03, 2023 at 10:51:35AM +0700, Alviro Iskandar Setiawan wrote: > On Thu, Dec 29, 2022 at 6:42 PM Ammar Faizi wrote: > > On 12/28/22 8:35 PM, Willy Tarreau wrote: > > > It gives me the correct code for x86_64 and i586. I don't know if other > > > architectures will want to add a prologue. I tried with "naked" but it's > > > ignored by the compiler since the function is not purely asm. Not very > > > important but given that we already have everything to perform our calls > > > it would make sense to stay on this. By the way, for the sake of > > > consistency with other syscalls, I do think the function (or label if > > > we can't do otherwise) should be called "sys_rt_sigreturn" as it just > > > performs a syscall. > > > > Will call that 'sys_rt_sigreturn' in the next series. > > >From glibc source code says: > GDB needs some intimate knowledge about it to recognize them as signal > trampolines, and make backtraces through signal handlers work right. > Important are both the names (__restore_rt) and the exact instruction > sequence. > > link: https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/x86_64/sigaction.c;h=4e6d9cc32e1e18746726fa430d092de9a19ba6c6;hb=b4a5d26d8835d972995f0a0a2f805a8845bafa0b#l34 > > glibc does this: > > " .type __" #name ",@function\n" \ > "__" #name ":\n" \ > " movq $" #syscall ", %rax\n" \ > " syscall\n" \ > > where > > #name = "restore_rt" > #syscall = __NR_rt_sigreturn > > I think it should be called "__restore_rt" instead of "sys_rt_sigreturn"? > glibc also has unwind information, but we probably don't need to care > with that much OK, I wasn't aware of this. Of course, if there are some strict rules for this, let's follow them! Thanks, Willy
On 1/3/23 10:54 AM, Willy Tarreau wrote: > On Tue, Jan 03, 2023 at 10:51:35AM +0700, Alviro Iskandar Setiawan wrote: >> GDB needs some intimate knowledge about it to recognize them as signal >> trampolines, and make backtraces through signal handlers work right. >> Important are both the names (__restore_rt) and the exact instruction >> sequence. Will follow it, thanks!
On Sun, Jan 8, 2023 at 8:10 PM Ammar Faizi wrote: > 3) More selftests. > > This series also adds selftests for: > - fork(2) > - sigaction(2) > - signal(2) [...] > Ammar Faizi (4): > nolibc/sys: Implement `sigaction(2)` function > nolibc/sys: Implement `signal(2)` function > selftests/nolibc: Add `fork(2)` selftest > selftests/nolibc: Add `sigaction(2)` selftest The signal() self test is missing? btw, it's too crowded here, let's create a new thread for this if you ever send another version
On Sun, Jan 08, 2023 at 08:31:37PM +0700, Ammar Faizi wrote: > On Sun, Jan 08, 2023 at 08:28:33PM +0700, Alviro Iskandar Setiawan wrote: > > The signal() self test is missing? > > Ugh, yes. I didn't send the signal() selftest. Will do. Bah, it turned out I squashed the signal() test into "selftests/nolibc: Add `sigaction(2)` selftes" patch. It was unintentional. Will split it properly in v3.