]> git.ipfire.org Git - thirdparty/valgrind.git/commitdiff
Get rid of VG_(x86_linux_REDIR_FOR__dl_sysinfo_int80) and do the x86-linux
authorJulian Seward <jseward@acm.org>
Sat, 29 Apr 2006 18:03:14 +0000 (18:03 +0000)
committerJulian Seward <jseward@acm.org>
Sat, 29 Apr 2006 18:03:14 +0000 (18:03 +0000)
stack unwind kludge another way.  This is believed to fix #108258.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@5868

coregrind/m_clientstate.c
coregrind/m_redir.c
coregrind/m_signals.c
coregrind/m_stacktrace.c
coregrind/m_trampoline.S
coregrind/pub_core_clientstate.h
coregrind/pub_core_trampoline.h

index b4b16c2fb8d28df80d4fc3aae37c2996c7ea6c4f..5da95d2c84f0c39c69b15911c1d4852f4d075fa6 100644 (file)
@@ -91,6 +91,12 @@ Int VG_(fd_hard_limit) = -1;
 /* Where is the __libc_freeres_wrapper routine we made? */
 Addr VG_(client___libc_freeres_wrapper) = 0;
 
+/* x86-linux only: where is glibc's _dl_sysinfo_int80 function?
+   Finding it isn't essential, but knowing where it is does sometimes
+   help produce better back traces.  See big comment in
+   VG_(get_StackTrace) in m_stacktrace.c for further info. */
+Addr VG_(client__dl_sysinfo_int80) = 0;
+
 
 /*--------------------------------------------------------------------*/
 /*--- end                                                          ---*/
index 853ad6624543cc8d42c4d85b2529a137dd0acbf6..adfa215d797ba2f3d3947f27dfa6672e8c75ab5f 100644 (file)
@@ -273,7 +273,8 @@ static Bool   is_plausible_guest_addr(Addr);
 static void   show_redir_state ( HChar* who );
 static void   show_active ( HChar* left, Active* act );
 
-static void   handle_maybe_load_notifier( HChar* symbol, Addr addr );
+static void   handle_maybe_load_notifier( const UChar* soname, 
+                                                HChar* symbol, Addr addr );
 
 
 /*------------------------------------------------------------*/
@@ -310,8 +311,11 @@ void VG_(redir_notify_new_SegInfo)( SegInfo* newsi )
    HChar    demangled_sopatt[N_DEMANGLED];
    HChar    demangled_fnpatt[N_DEMANGLED];
 
+   const UChar* newsi_soname;
+
    vg_assert(newsi);
-   vg_assert(VG_(seginfo_soname)(newsi) != NULL);
+   newsi_soname = VG_(seginfo_soname)(newsi);
+   vg_assert(newsi_soname != NULL);
 
    /* stay sane: we don't already have this. */
    for (ts = topSpecs; ts; ts = ts->next)
@@ -330,7 +334,7 @@ void VG_(redir_notify_new_SegInfo)( SegInfo* newsi )
       if (!ok) {
          /* It's not a full-scale redirect, but perhaps it is a load-notify
             fn?  Let the load-notify department see it. */
-         handle_maybe_load_notifier( sym_name, sym_addr );
+         handle_maybe_load_notifier( newsi_soname, sym_name, sym_addr );
          continue; 
       }
       spec = symtab_alloc(sizeof(Spec));
@@ -726,13 +730,6 @@ void VG_(redir_initialise) ( void )
    // The rest of this function just adds initial Specs.   
 
 #  if defined(VGP_x86_linux)
-   /* Redirect _dl_sysinfo_int80, which is glibc's default system call
-      routine, to our copy so that the special sysinfo unwind hack in
-      m_stacktrace.c will kick in. */
-   add_hardwired_spec(
-      "ld-linux.so.2", "_dl_sysinfo_int80",
-      (Addr)&VG_(x86_linux_REDIR_FOR__dl_sysinfo_int80) 
-   );
    /* If we're using memcheck, use this intercept right from the
       start, otherwise ld.so (glibc-2.3.5) makes a lot of noise. */
    if (0==VG_(strcmp)("Memcheck", VG_(details).name)) {
@@ -827,8 +824,25 @@ static Bool is_plausible_guest_addr(Addr a)
 /*--- NOTIFY-ON-LOAD FUNCTIONS                             ---*/
 /*------------------------------------------------------------*/
 
-static void handle_maybe_load_notifier( HChar* symbol, Addr addr )
+static 
+void handle_maybe_load_notifier( const UChar* soname, 
+                                       HChar* symbol, Addr addr )
 {
+#  if defined(VGP_x86_linux)
+   /* x86-linux only: if we see _dl_sysinfo_int80, note its address.
+      See comment on declaration of VG_(client__dl_sysinfo_int80) for
+      the reason.  As far as I can tell, the relevant symbol is always
+      in object with soname "ld-linux.so.2". */
+   if (symbol && symbol[0] == '_' 
+              && 0 == VG_(strcmp)(symbol, "_dl_sysinfo_int80")
+              && 0 == VG_(strcmp)(soname, "ld-linux.so.2")) {
+      if (VG_(client__dl_sysinfo_int80) == 0)
+         VG_(client__dl_sysinfo_int80) = addr;
+   }
+#  endif
+
+   /* Normal load-notifier handling after here.  First, ignore all
+      symbols lacking the right prefix. */
    if (0 != VG_(strncmp)(symbol, VG_NOTIFY_ON_LOAD_PREFIX, 
                                  VG_NOTIFY_ON_LOAD_PREFIX_LEN))
       /* Doesn't have the right prefix */
@@ -836,9 +850,6 @@ static void handle_maybe_load_notifier( HChar* symbol, Addr addr )
 
    if (VG_(strcmp)(symbol, VG_STRINGIFY(VG_NOTIFY_ON_LOAD(freeres))) == 0)
       VG_(client___libc_freeres_wrapper) = addr;
-// else
-// if (VG_(strcmp)(symbol, STR(VG_WRAPPER(pthread_startfunc_wrapper))) == 0)
-//    VG_(pthread_startfunc_wrapper)((Addr)(si->offset + sym->st_value));
    else
       vg_assert2(0, "unrecognised load notification function: %s", symbol);
 }
index 05916223b2aae0b24387b4650317b7ded006da6f..53c2dc2dc271ffa8b8318e73abb55f8adfd3bd7c 100644 (file)
@@ -905,9 +905,11 @@ void push_signal_frame ( ThreadId tid, const vki_siginfo_t *siginfo )
    vg_assert(VG_(is_valid_tid)(tid));
    tst = & VG_(threads)[tid];
 
-   if (VG_(clo_trace_signals))
+   if (VG_(clo_trace_signals)) {
       VG_(message)(Vg_DebugMsg, 
          "push_signal_frame (thread %d): signal %d", tid, sigNo);
+      VG_(get_and_pp_StackTrace)(tid, 10);
+   }
 
    if (/* this signal asked to run on an alt stack */
        (scss.scss_per_sig[sigNo].scss_flags & VKI_SA_ONSTACK )
index 3d307fc45a96351209b55a69d8e5dd2ddad22613..be0fef7cbb8f2a9fd086578f5e864bbfe0a7f4d7 100644 (file)
@@ -38,6 +38,7 @@
 #include "pub_core_machine.h"
 #include "pub_core_options.h"
 #include "pub_core_stacktrace.h"
+#include "pub_core_clientstate.h"   // VG_(client__dl_sysinfo_int80)
 #include "pub_core_trampoline.h"
 
 /*------------------------------------------------------------*/
@@ -344,16 +345,26 @@ UInt VG_(get_StackTrace) ( ThreadId tid, StackTrace ips, UInt n_ips )
    Addr stack_highest_word = VG_(threads)[tid].client_stack_highest_word;
 
 #  if defined(VGP_x86_linux)
-   /* Nasty little hack to deal with sysinfo syscalls - if libc is
-      using the sysinfo page for syscalls (the TLS version does), then
-      ip will always appear to be in that page when doing a syscall,
-      not the actual libc function doing the syscall.  This check sees
-      if IP is within the syscall code, and pops the return address
-      off the stack so that ip is placed within the library function
-      calling the syscall.  This makes stack backtraces much more
-      useful.  */
-   if (ip >= (Addr)&VG_(trampoline_stuff_start) 
-       && ip < (Addr)&VG_(trampoline_stuff_end)
+   /* Nasty little hack to deal with syscalls - if libc is using its
+      _dl_sysinfo_int80 function for syscalls (the TLS version does),
+      then ip will always appear to be in that function when doing a
+      syscall, not the actual libc function doing the syscall.  This
+      check sees if IP is within that function, and pops the return
+      address off the stack so that ip is placed within the library
+      function calling the syscall.  This makes stack backtraces much
+      more useful.
+
+      The function is assumed to look like this (from glibc-2.3.6 sources):
+         _dl_sysinfo_int80:
+            int $0x80
+            ret
+      That is 3 (2+1) bytes long.  We could be more thorough and check
+      the 3 bytes of the function are as expected, but I can't be
+      bothered.
+   */
+   if (VG_(client__dl_sysinfo_int80) != 0 /* we know its address */
+       && ip >= VG_(client__dl_sysinfo_int80)
+       && ip < VG_(client__dl_sysinfo_int80)+3
        && VG_(am_is_valid_for_client)(sp, sizeof(Addr), VKI_PROT_READ)) {
       ip = *(Addr *)sp;
       sp += sizeof(Addr);
index 3416eb4d8664eae2308e7677238feb0a592916ca..5965e8714e5686f9f471d28ec2f356ebb3ac23a6 100644 (file)
@@ -55,7 +55,9 @@ VG_(trampoline_stuff_start):
 .global VG_(x86_linux_SUBST_FOR_sigreturn)
 VG_(x86_linux_SUBST_FOR_sigreturn):
         /* This is a very specific sequence which GDB uses to
-           recognize signal handler frames. */
+           recognize signal handler frames.  Also gcc: see
+           x86_fallback_frame_state() in
+           gcc-4.1.0/gcc/config/i386/linux-unwind.h */
         popl    %eax
         movl    $__NR_sigreturn, %eax
         int     $0x80
@@ -68,12 +70,6 @@ VG_(x86_linux_SUBST_FOR_rt_sigreturn):
         int     $0x80
         ud2
 
-.global VG_(x86_linux_REDIR_FOR__dl_sysinfo_int80)
-VG_(x86_linux_REDIR_FOR__dl_sysinfo_int80):
-       /* We can point our sysinfo stuff here */
-       int     $0x80
-       ret
-
 /* There's no particular reason that this needs to be handwritten
    assembly, but since that's what this file contains, here's a
    simple index implementation (written in C and compiled by gcc.)
index a526532a4b75e920973875e86ff72750c9075f22..e9740f232e48780cec22e42b2b848d7f84f308d5 100644 (file)
@@ -82,6 +82,13 @@ extern Int VG_(fd_hard_limit);
 /* Where is the __libc_freeres_wrapper routine we made? */
 extern Addr VG_(client___libc_freeres_wrapper);
 
+/* x86-linux only: where is ld.so's _dl_sysinfo_int80 function?
+   Finding it isn't essential, but knowing where it is does sometimes
+   help produce better back traces.  See big comment in
+   VG_(get_StackTrace) in m_stacktrace.c for further info. */
+extern Addr VG_(client__dl_sysinfo_int80);
+
+
 #endif   // __PUB_CORE_CLIENTSTATE_H
 
 /*--------------------------------------------------------------------*/
index 134a569d81ea570193f7da78d804f7e3b9862589..9d2c55074c83cffc18cf6dd23ceb7dc3327c81a9 100644 (file)
 // stubs for signal returns.  Note, all the code within runs on the
 // simulated CPU.  The vsyscall stubs are gotten to by use of the 
 // redirect mechanism.
+//
+// Note: generally, putting replacement functions in here is a bad
+// idea, since any Dwarf frame-unwind info attached to them will not
+// be seen by the unwinder in gcc's runtime support.  This means
+// unwinding during exception handling by gcc tends to fail if it
+// encounters one of these replacement functions.  A better place to
+// put them is in one of the .so's preloaded into the client, since
+// the client's ld.so will know about it and so gcc's unwinder
+// (somehow) is able to get hold of it.
 //--------------------------------------------------------------------
 
 /* These two delimit our handwritten assembly code, so we can tell
@@ -50,7 +59,6 @@ extern void VG_(trampoline_stuff_end);
 #if defined(VGP_x86_linux)
 extern void VG_(x86_linux_SUBST_FOR_sigreturn);
 extern void VG_(x86_linux_SUBST_FOR_rt_sigreturn);
-extern void VG_(x86_linux_REDIR_FOR__dl_sysinfo_int80);
 extern Char* VG_(x86_linux_REDIR_FOR_index) ( const Char*, Int );
 #endif