Bug 295428 - coregrind/m_main.c has incorrect x86 assembly for darwin
Summary: coregrind/m_main.c has incorrect x86 assembly for darwin
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (show other bugs)
Version: unspecified
Platform: Fink Packages macOS
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-06 17:25 UTC by Jack Howarth
Modified: 2012-03-07 16:38 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jack Howarth 2012-03-06 17:25:23 UTC
User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10_6_8) AppleWebKit/534.52.7 (KHTML, like Gecko) Version/5.1.2 Safari/534.52.7
Build Identifier: 

When building either valgrind 3.7.0 or svn with clang from Xcode 4.3, the resulting i386 static executables will segfault. The darwin linker developer has diagnosed this as...

It looks like the stack needs to be 16-byte aligned at the point of a call. So the stack when it's calling "_start_in_C_darwin" in valgrind's ASM code isn't aligned at 16-bytes. The typical way a call happens is:
* Stack aligned at 16-bytes.
* Push the return address -- stack offset by 4-bytes in i386
* At start of called function, push the frame pointer (%ebp) -- stack offset by 4-bytes on i386
* Store the stack pointer (%esp) into the frame pointer (%ebp)
* Create storage for local objects (adjust the %esp)

I believe that valgrind will need to fix their ASM code in '_start' to work in this manner.

The recommended fix to the assembly is...

      .text
       .align 2,0x90
       .globl __start

__start:
       movl  $_vgPlain_interim_stack, %eax
       addl  $8192, %eax
       addl  $(4096 * 256), %eax
   subl  $16, %eax
   andl  $~15, %eax
       xchgl %eax, %esp
# the problem is here.  The have switched to their stack which they have forced
# to be 16-byte aligned, but then they push a parameter which misaligns  the stack.
# The standard thing to do is to decrement the stack before the push:

        subl  $12,%esp

# that will cause the stack to be aligned again after the parameter push.

       pushl %eax
       call  __start_in_C_darwin
       int $3
       int $3


Reproducible: Always

Steps to Reproduce:
1.  Using the proposed fix from https://bugs.kde.org/show_bug.cgi?id=295427 execute...
setenv CC clang
setenv CCX clang++
../valgrind/configure --prefix=/Users/howarth/valgrind_dist --without-mpicc
make
2. now execute
cd memcheck
./memcheck-x86-darwin
Actual Results:  
The resulting i386 static executable created by clang segfaults.

Expected Results:  
I expected to see the memcheck-x86-darwin i386 static executable output...

valgrind: You cannot run './memcheck-amd64-darwin' directly.
valgrind: You should use $prefix/bin/valgrind.


Using the recommended fix from the darwin linker developer of...


Index: coregrind/m_main.c
===================================================================
--- coregrind/m_main.c	(revision 12418)
+++ coregrind/m_main.c	(working copy)
@@ -2842,6 +2842,7 @@ asm("\n"
     "\tandl  $~15, %eax\n"
     /* install it, and collect the original one */
     "\txchgl %eax, %esp\n"
+    "\tsubl  $12, %esp\n"
     /* call _start_in_C_darwin, passing it the startup %esp */
     "\tpushl %eax\n"
     "\tcall  __start_in_C_darwin\n"

the clang built i386 static executables no longer segfault.
Comment 1 Julian Seward 2012-03-07 15:57:14 UTC
Committed, r12424.  Thanks for the diagnosis + fix.
Comment 2 Julian Seward 2012-03-07 16:38:43 UTC
... and ... actually put the new instruction in the
right place this time, in r12425 !