From: Julian Seward Date: Tue, 22 Mar 2005 01:55:35 +0000 (+0000) Subject: Mucho buggering about jmp_with_stack and its kin. Jeez. Now the X-Git-Tag: svn/VALGRIND_3_0_0~916 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=9d7b4d9ae290d5ec2785ed348f83d27cd02970bc;p=thirdparty%2Fvalgrind.git Mucho buggering about jmp_with_stack and its kin. Jeez. Now the amd64 version is broken again. Writing code with undocumented assumptions should be a firable offense. At the very least. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@3402 --- diff --git a/coregrind/amd64-linux/syscalls.c b/coregrind/amd64-linux/syscalls.c index 9ed1eb4cd4..fb0516b871 100644 --- a/coregrind/amd64-linux/syscalls.c +++ b/coregrind/amd64-linux/syscalls.c @@ -246,8 +246,6 @@ static ULong *allocstack(ThreadId tid) void VGA_(main_thread_wrapper)(ThreadId tid) { ULong *rsp = allocstack(tid); - - VG_(printf)("m-t-r: %d\n", (int)tid); vg_assert(tid == VG_(master_tid)); call_on_new_stack_0_1( diff --git a/coregrind/amd64/jmp_with_stack.c b/coregrind/amd64/jmp_with_stack.c index dacf37a4ab..068b52450b 100644 --- a/coregrind/amd64/jmp_with_stack.c +++ b/coregrind/amd64/jmp_with_stack.c @@ -26,35 +26,6 @@ #include "ume.h" -void jmp_with_stack(void(*rip)(void), Addr rsp) -{ - asm volatile ( - "movq %1, %%rsp;" // set rsp - "pushq %%rax;" // push rsp - "xorq %%rax,%%rax;" // clear registers - "xorq %%rbx,%%rbx;" - "xorq %%rcx,%%rcx;" - "xorq %%rdx,%%rdx;" - "xorq %%rsi,%%rsi;" - "xorq %%rdi,%%rdi;" - "xorq %%rbp,%%rbp;" - "xorq %%r8, %%r8;" - "xorq %%r9, %%r9;" - "xorq %%r10,%%r10;" - "xorq %%r11,%%r11;" - "xorq %%r12,%%r12;" - "xorq %%r13,%%r13;" - "xorq %%r14,%%r14;" - "xorq %%r15,%%r15;" - "ret" // return into entry - : : "a" (rip), "r" (rsp)); - - // we should never get here - for(;;) - asm volatile("ud2"); -} - - #define ZERO_ALL_INT_REGS \ " movq $0, %rax\n" \ @@ -73,6 +44,7 @@ void jmp_with_stack(void(*rip)(void), Addr rsp) " movq $0, %r14\n" \ " movq $0, %r15\n" + /* Call f(), but first switch stacks, using 'stack' as the new stack, and use 'retaddr' as f's return-to address. Also, clear all the integer registers before entering f. */ diff --git a/coregrind/stage1.c b/coregrind/stage1.c index 2c10edb796..6ab8c93cd4 100644 --- a/coregrind/stage1.c +++ b/coregrind/stage1.c @@ -255,6 +255,7 @@ static int prmap(char *start, char *end, const char *perm, off_t off, int maj, return 1; } + static void main2(void) { int err, padfile; @@ -262,7 +263,7 @@ static void main2(void) extern char _end; int *esp; char buf[strlen(valgrind_lib) + sizeof(stage2) + 16]; - + printf("& local = %p\n", &esp); info.exe_end = PGROUNDDN(init_sp); #ifdef HAVE_PIE info.exe_base = ROUNDDN(info.exe_end - 0x02000000, 0x10000000); @@ -300,9 +301,16 @@ static void main2(void) foreach_map(prmap, /*dummy*/NULL); } - jmp_with_stack((void (*)(void))info.init_eip, (Addr)esp); + jump_and_switch_stacks( + (Addr) esp, /* stack */ + (Addr) info.init_eip /* where to */ + ); + + /*NOTREACHED*/ + assert(0); } + int main(int argc, char** argv) { struct rlimit rlim; @@ -328,7 +336,13 @@ int main(int argc, char** argv) setrlimit(RLIMIT_AS, &rlim); /* move onto another stack so we can play with the main one */ - jmp_with_stack(main2, (Addr)stack + sizeof(stack)); + jump_and_switch_stacks( + (Addr)stack + sizeof(stack), /* stack */ + main2 /* where to */ + ); + + /*NOTREACHED*/ + assert(0); } /*--------------------------------------------------------------------*/ diff --git a/coregrind/ume.h b/coregrind/ume.h index e91f0301fa..39818e204d 100644 --- a/coregrind/ume.h +++ b/coregrind/ume.h @@ -47,22 +47,18 @@ void foreach_map(int (*fn)(char *start, char *end, int maj, int min, int ino, void* extra), void* extra); -// Jump to a new 'ip' with the stack 'sp'. This is intended -// to simulate the initial CPU state when the kernel starts an program -// after exec; and so should clear all the other registers. +/* Jump to 'dst', but first set the stack pointer to 'stack'. Also, + clear all the integer registers before entering 'dst'. It's + important that the stack pointer is set to exactly 'stack' and not + (eg) stack - apparently_harmless_looking_small_offset. Basically + because the code at 'dst' might be wanting to scan the area above + 'stack' (viz, the auxv array), and putting spurious words on the + stack confuses it. +*/ extern __attribute__((noreturn)) -void jmp_with_stack(void (*eip)(void), Addr sp); - +void jump_and_switch_stacks ( Addr stack, Addr dst ); -/* Call f(), but first switch stacks, using 'stack' as the new stack, - and use 'retaddr' as f's return-to address. Also, clear all the - integer registers before entering f. */ -extern -__attribute__((noreturn)) -void call_on_new_stack_0_0 ( Addr stack, - Addr retaddr, - void (*f)(void) ); /* Call f(arg1), but first switch stacks, using 'stack' as the new stack, and use 'retaddr' as f's return-to address. Also, clear all diff --git a/coregrind/x86-linux/syscalls.c b/coregrind/x86-linux/syscalls.c index ff246a2635..a7120d5b0c 100644 --- a/coregrind/x86-linux/syscalls.c +++ b/coregrind/x86-linux/syscalls.c @@ -241,9 +241,15 @@ void VGA_(main_thread_wrapper)(ThreadId tid) vg_assert(tid == VG_(master_tid)); - *--esp = tid; /* set arg */ - *--esp = 0; /* bogus return address */ - jmp_with_stack((void (*)(void))VGA_(thread_wrapper), (Addr)esp); + call_on_new_stack_0_1( + (Addr)esp, /* stack */ + 0, /*bogus return address*/ + VGA_(thread_wrapper), /* fn to call */ + (Word)tid /* arg to give it */ + ); + + /*NOTREACHED*/ + vg_assert(0); } static Int start_thread(void *arg) diff --git a/coregrind/x86/jmp_with_stack.c b/coregrind/x86/jmp_with_stack.c index 7328e6998f..17ce451898 100644 --- a/coregrind/x86/jmp_with_stack.c +++ b/coregrind/x86/jmp_with_stack.c @@ -26,27 +26,71 @@ #include "ume.h" -/* - Jump to a particular IP with a particular SP. This is intended - to simulate the initial CPU state when the kernel starts an program - after exec; it therefore also clears all the other registers. - */ -void jmp_with_stack(void (*eip)(void), Addr esp) -{ - asm volatile ( - "movl %1, %%esp;" // set esp */ - "pushl %%eax;" // push esp */ - "xorl %%eax,%%eax;" // clear registers - "xorl %%ebx,%%ebx;" - "xorl %%ecx,%%ecx;" - "xorl %%edx,%%edx;" - "xorl %%esi,%%esi;" - "xorl %%edi,%%edi;" - "xorl %%ebp,%%ebp;" - "ret" // return into entry - : : "a" (eip), "r" (esp)); - - // we should never get here - for(;;) - asm volatile("ud2"); -} + +#define ZERO_ALL_INT_REGS \ + " movl $0, %eax\n" \ + " movl $0, %ebx\n" \ + " movl $0, %ecx\n" \ + " movl $0, %edx\n" \ + " movl $0, %esi\n" \ + " movl $0, %edi\n" \ + " movl $0, %ebp\n" + + +/* Jump to 'dst', but first set the stack pointer to 'stack'. Also, + clear all the integer registers before entering 'dst'. It's + important that the stack pointer is set to exactly 'stack' and not + (eg) stack - apparently_harmless_looking_small_offset. Basically + because the code at 'dst' might be wanting to scan the area above + 'stack' (viz, the auxv array), and putting spurious words on the + stack confuses it. +*/ +/* +__attribute__((noreturn)) +void jump_and_switch_stacks ( Addr stack, Addr dst ); + + 4(%esp) == stack + 8(%esp) == f +*/ +asm( +".global jump_and_switch_stacks\n" +"jump_and_switch_stacks:\n" +" movl %esp, %esi\n" /* remember old stack pointer */ +" movl 4(%esi), %esp\n" /* set stack */ +" pushl 8(%esi)\n" /* f to stack*/ + ZERO_ALL_INT_REGS +" ret\n" /* jump to f */ +" ud2\n" /* should never get here */ +); + + + +/* Call f(arg1), but first switch stacks, using 'stack' as the new + stack, and use 'retaddr' as f's return-to address. Also, clear all + the integer registers before entering f.*/ +/* +__attribute__((noreturn)) +void call_on_new_stack_0_1 ( Addr stack, + Addr retaddr, + void (*f)(Word), + Word arg1 ); + 4(%esp) == stack + 8(%esp) == retaddr + 12(%esp) == f + 16(%esp) == arg1 +*/ +asm( +".global call_on_new_stack_0_1\n" +"call_on_new_stack_0_1:\n" +" movl %esp, %esi\n" /* remember old stack pointer */ +" movl 4(%esi), %esp\n" /* set stack */ +" pushl 16(%esi)\n" /* arg1 to stack */ +" pushl 8(%esi)\n" /* retaddr to stack */ +" pushl 12(%esi)\n" /* f to stack */ + ZERO_ALL_INT_REGS +" ret\n" /* jump to f */ +" ud2\n" /* should never get here */ +); + + +#undef ZERO_ALL_INT_REGS