Improvements to the error-collecting machinery:

- Don't waste a potentially huge amount of time calling describe_addr
  on addresses in errors we aren't going to show.

- If an invalid address is just below %ESP, say that it might be due
  to a gcc bug.  Increase the window in which this is allowed to
  1024 bytes below %ESP.

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

diff --git a/coregrind/vg_errcontext.c b/coregrind/vg_errcontext.c
index bf0cc5c394c79aff3241538a318f1c1d1939be2e..aabbe430985ab98faa05ae405fcf074bb42c566d 100644 (file)
--- a/coregrind/vg_errcontext.c
+++ b/coregrind/vg_errcontext.c
@@ -189,6 +189,7 @@ static void clear_AddrInfo ( AddrInfo* ai )
    ai->rwoffset   = 0;
    ai->lastchange = NULL;
    ai->stack_tid  = VG_INVALID_THREADID;
+   ai->maybe_gcc  = False;
 }
 
 static void clear_ErrContext ( ErrContext* ec )
@@ -227,7 +228,9 @@ Bool vg_eq_ExeContext ( Bool top_2_only,
 static Bool eq_AddrInfo ( Bool cheap_addr_cmp,
                           AddrInfo* ai1, AddrInfo* ai2 )
 {
-   if (ai1->akind != ai2->akind) 
+   if (ai1->akind != Undescribed 
+       && ai2->akind != Undescribed
+       && ai1->akind != ai2->akind) 
       return False;
    if (ai1->akind == Freed || ai1->akind == Mallocd) {
       if (ai1->blksize != ai2->blksize)
@@ -288,8 +291,16 @@ static void pp_AddrInfo ( Addr a, AddrInfo* ai )
                       ai->stack_tid, a);
          break;
       case Unknown:
-         VG_(message)(Vg_UserMsg, 
-                      "   Address 0x%x is not stack'd, malloc'd or free'd", a);
+         if (ai->maybe_gcc) {
+            VG_(message)(Vg_UserMsg, 
+               "   Address 0x%x is just below %%esp.  Possibly a bug in GCC/G++",
+               a);
+            VG_(message)(Vg_UserMsg, 
+               "   v 2.96 or 3.0.X.  To suppress, use: --workaround-gcc296-bugs=yes");
+        } else {
+            VG_(message)(Vg_UserMsg, 
+               "   Address 0x%x is not stack'd, malloc'd or free'd", a);
+         }
          break;
       case Freed: case Mallocd: case UserG: case UserS: {
          UInt delta;
@@ -546,7 +557,10 @@ static void VG_(maybe_add_context) ( ErrContext* ec )
 
    /* Didn't see it.  Copy and add. */
 
-   /* OK, we're really going to collect it. */
+   /* OK, we're really going to collect it.  First, describe any addr
+      info in the error. */
+   if (ec->addrinfo.akind == Undescribed)
+      VG_(describe_addr) ( ec->addr, &ec->addrinfo );
 
    p = VG_(malloc)(VG_AR_ERRCTXT, sizeof(ErrContext));
    *p = *ec;
@@ -604,12 +618,15 @@ void VG_(record_value_error) ( Int size )
 void VG_(record_address_error) ( Addr a, Int size, Bool isWrite )
 {
    ErrContext ec;
+   Bool       just_below_esp;
    if (vg_ignore_errors) return;
 
+   just_below_esp 
+      = VG_(is_just_below_ESP)( VG_(baseBlock)[VGOFF_(m_esp)], a );
+
    /* If this is caused by an access immediately below %ESP, and the
       user asks nicely, we just ignore it. */
-   if (VG_(clo_workaround_gcc296_bugs) 
-       && VG_(is_just_below_ESP)( VG_(baseBlock)[VGOFF_(m_esp)], a ))
+   if (VG_(clo_workaround_gcc296_bugs) && just_below_esp)
       return;
 
    clear_ErrContext( &ec );
@@ -625,7 +642,8 @@ void VG_(record_address_error) ( Addr a, Int size, Bool isWrite )
    ec.m_eip = VG_(baseBlock)[VGOFF_(m_eip)];
    ec.m_esp = VG_(baseBlock)[VGOFF_(m_esp)];
    ec.m_ebp = VG_(baseBlock)[VGOFF_(m_ebp)];
-   VG_(describe_addr) ( a, &ec.addrinfo );
+   ec.addrinfo.akind     = Undescribed;
+   ec.addrinfo.maybe_gcc = just_below_esp;
    VG_(maybe_add_context) ( &ec );
 }
 
@@ -648,7 +666,7 @@ void VG_(record_free_error) ( ThreadState* tst, Addr a )
    ec.m_eip   = tst->m_eip;
    ec.m_esp   = tst->m_esp;
    ec.m_ebp   = tst->m_ebp;
-   VG_(describe_addr) ( a, &ec.addrinfo );
+   ec.addrinfo.akind = Undescribed;
    VG_(maybe_add_context) ( &ec );
 }
 
@@ -666,7 +684,7 @@ void VG_(record_freemismatch_error) ( ThreadState* tst, Addr a )
    ec.m_eip   = tst->m_eip;
    ec.m_esp   = tst->m_esp;
    ec.m_ebp   = tst->m_ebp;
-   VG_(describe_addr) ( a, &ec.addrinfo );
+   ec.addrinfo.akind = Undescribed;
    VG_(maybe_add_context) ( &ec );
 }
 
@@ -685,7 +703,7 @@ void VG_(record_jump_error) ( ThreadState* tst, Addr a )
    ec.m_eip   = tst->m_eip;
    ec.m_esp   = tst->m_esp;
    ec.m_ebp   = tst->m_ebp;
-   VG_(describe_addr) ( a, &ec.addrinfo );
+   ec.addrinfo.akind = Undescribed;
    VG_(maybe_add_context) ( &ec );
 }
 
@@ -704,7 +722,7 @@ void VG_(record_param_err) ( ThreadState* tst, Addr a, Bool isWriteLack,
    ec.m_eip   = tst->m_eip;
    ec.m_esp   = tst->m_esp;
    ec.m_ebp   = tst->m_ebp;
-   VG_(describe_addr) ( a, &ec.addrinfo );
+   ec.addrinfo.akind = Undescribed;
    ec.syscall_param = msg;
    ec.isWriteableLack = isWriteLack;
    VG_(maybe_add_context) ( &ec );
@@ -724,7 +742,7 @@ void VG_(record_user_err) ( ThreadState* tst, Addr a, Bool isWriteLack )
    ec.m_eip   = tst->m_eip;
    ec.m_esp   = tst->m_esp;
    ec.m_ebp   = tst->m_ebp;
-   VG_(describe_addr) ( a, &ec.addrinfo );
+   ec.addrinfo.akind = Undescribed;
    ec.isWriteableLack = isWriteLack;
    VG_(maybe_add_context) ( &ec );
 }

diff --git a/coregrind/vg_include.h b/coregrind/vg_include.h
index 61d5517aa53ba11c62cba598078c3f219d5f9cae..b7302549564c89881ff2510c62d86adcc52589ad 100644 (file)
--- a/coregrind/vg_include.h
+++ b/coregrind/vg_include.h
@@ -106,7 +106,7 @@
 
 /* These many bytes below %ESP are considered addressible if we're
    doing the --workaround-gcc296-bugs hack. */
-#define VG_GCC296_BUG_STACK_SLOP 256
+#define VG_GCC296_BUG_STACK_SLOP 1024
 
 /* The maximum number of calls we're prepared to save in a
    backtrace. */
@@ -1152,7 +1152,11 @@ extern void VG_(record_user_err) ( ThreadState* tst,
 
 /* The classification of a faulting address. */
 typedef 
-   enum { Stack, Unknown, Freed, Mallocd, UserG, UserS }
+   enum { Undescribed, /* as-yet unclassified */
+          Stack, 
+          Unknown, /* classification yielded nothing useful */
+          Freed, Mallocd, 
+          UserG, UserS }
    AddrKind;
 
 /* Records info about a faulting address. */
@@ -1168,6 +1172,8 @@ typedef
       ExeContext* lastchange;
       /* Stack */
       ThreadId stack_tid;
+      /* True if is just-below %esp -- could be a gcc bug. */
+      Bool maybe_gcc;
    }
    AddrInfo;
 

diff --git a/vg_errcontext.c b/vg_errcontext.c
index bf0cc5c394c79aff3241538a318f1c1d1939be2e..aabbe430985ab98faa05ae405fcf074bb42c566d 100644 (file)
--- a/vg_errcontext.c
+++ b/vg_errcontext.c
@@ -189,6 +189,7 @@ static void clear_AddrInfo ( AddrInfo* ai )
    ai->rwoffset   = 0;
    ai->lastchange = NULL;
    ai->stack_tid  = VG_INVALID_THREADID;
+   ai->maybe_gcc  = False;
 }
 
 static void clear_ErrContext ( ErrContext* ec )
@@ -227,7 +228,9 @@ Bool vg_eq_ExeContext ( Bool top_2_only,
 static Bool eq_AddrInfo ( Bool cheap_addr_cmp,
                           AddrInfo* ai1, AddrInfo* ai2 )
 {
-   if (ai1->akind != ai2->akind) 
+   if (ai1->akind != Undescribed 
+       && ai2->akind != Undescribed
+       && ai1->akind != ai2->akind) 
       return False;
    if (ai1->akind == Freed || ai1->akind == Mallocd) {
       if (ai1->blksize != ai2->blksize)
@@ -288,8 +291,16 @@ static void pp_AddrInfo ( Addr a, AddrInfo* ai )
                       ai->stack_tid, a);
          break;
       case Unknown:
-         VG_(message)(Vg_UserMsg, 
-                      "   Address 0x%x is not stack'd, malloc'd or free'd", a);
+         if (ai->maybe_gcc) {
+            VG_(message)(Vg_UserMsg, 
+               "   Address 0x%x is just below %%esp.  Possibly a bug in GCC/G++",
+               a);
+            VG_(message)(Vg_UserMsg, 
+               "   v 2.96 or 3.0.X.  To suppress, use: --workaround-gcc296-bugs=yes");
+        } else {
+            VG_(message)(Vg_UserMsg, 
+               "   Address 0x%x is not stack'd, malloc'd or free'd", a);
+         }
          break;
       case Freed: case Mallocd: case UserG: case UserS: {
          UInt delta;
@@ -546,7 +557,10 @@ static void VG_(maybe_add_context) ( ErrContext* ec )
 
    /* Didn't see it.  Copy and add. */
 
-   /* OK, we're really going to collect it. */
+   /* OK, we're really going to collect it.  First, describe any addr
+      info in the error. */
+   if (ec->addrinfo.akind == Undescribed)
+      VG_(describe_addr) ( ec->addr, &ec->addrinfo );
 
    p = VG_(malloc)(VG_AR_ERRCTXT, sizeof(ErrContext));
    *p = *ec;
@@ -604,12 +618,15 @@ void VG_(record_value_error) ( Int size )
 void VG_(record_address_error) ( Addr a, Int size, Bool isWrite )
 {
    ErrContext ec;
+   Bool       just_below_esp;
    if (vg_ignore_errors) return;
 
+   just_below_esp 
+      = VG_(is_just_below_ESP)( VG_(baseBlock)[VGOFF_(m_esp)], a );
+
    /* If this is caused by an access immediately below %ESP, and the
       user asks nicely, we just ignore it. */
-   if (VG_(clo_workaround_gcc296_bugs) 
-       && VG_(is_just_below_ESP)( VG_(baseBlock)[VGOFF_(m_esp)], a ))
+   if (VG_(clo_workaround_gcc296_bugs) && just_below_esp)
       return;
 
    clear_ErrContext( &ec );
@@ -625,7 +642,8 @@ void VG_(record_address_error) ( Addr a, Int size, Bool isWrite )
    ec.m_eip = VG_(baseBlock)[VGOFF_(m_eip)];
    ec.m_esp = VG_(baseBlock)[VGOFF_(m_esp)];
    ec.m_ebp = VG_(baseBlock)[VGOFF_(m_ebp)];
-   VG_(describe_addr) ( a, &ec.addrinfo );
+   ec.addrinfo.akind     = Undescribed;
+   ec.addrinfo.maybe_gcc = just_below_esp;
    VG_(maybe_add_context) ( &ec );
 }
 
@@ -648,7 +666,7 @@ void VG_(record_free_error) ( ThreadState* tst, Addr a )
    ec.m_eip   = tst->m_eip;
    ec.m_esp   = tst->m_esp;
    ec.m_ebp   = tst->m_ebp;
-   VG_(describe_addr) ( a, &ec.addrinfo );
+   ec.addrinfo.akind = Undescribed;
    VG_(maybe_add_context) ( &ec );
 }
 
@@ -666,7 +684,7 @@ void VG_(record_freemismatch_error) ( ThreadState* tst, Addr a )
    ec.m_eip   = tst->m_eip;
    ec.m_esp   = tst->m_esp;
    ec.m_ebp   = tst->m_ebp;
-   VG_(describe_addr) ( a, &ec.addrinfo );
+   ec.addrinfo.akind = Undescribed;
    VG_(maybe_add_context) ( &ec );
 }
 
@@ -685,7 +703,7 @@ void VG_(record_jump_error) ( ThreadState* tst, Addr a )
    ec.m_eip   = tst->m_eip;
    ec.m_esp   = tst->m_esp;
    ec.m_ebp   = tst->m_ebp;
-   VG_(describe_addr) ( a, &ec.addrinfo );
+   ec.addrinfo.akind = Undescribed;
    VG_(maybe_add_context) ( &ec );
 }
 
@@ -704,7 +722,7 @@ void VG_(record_param_err) ( ThreadState* tst, Addr a, Bool isWriteLack,
    ec.m_eip   = tst->m_eip;
    ec.m_esp   = tst->m_esp;
    ec.m_ebp   = tst->m_ebp;
-   VG_(describe_addr) ( a, &ec.addrinfo );
+   ec.addrinfo.akind = Undescribed;
    ec.syscall_param = msg;
    ec.isWriteableLack = isWriteLack;
    VG_(maybe_add_context) ( &ec );
@@ -724,7 +742,7 @@ void VG_(record_user_err) ( ThreadState* tst, Addr a, Bool isWriteLack )
    ec.m_eip   = tst->m_eip;
    ec.m_esp   = tst->m_esp;
    ec.m_ebp   = tst->m_ebp;
-   VG_(describe_addr) ( a, &ec.addrinfo );
+   ec.addrinfo.akind = Undescribed;
    ec.isWriteableLack = isWriteLack;
    VG_(maybe_add_context) ( &ec );
 }

diff --git a/vg_include.h b/vg_include.h
index 61d5517aa53ba11c62cba598078c3f219d5f9cae..b7302549564c89881ff2510c62d86adcc52589ad 100644 (file)
--- a/vg_include.h
+++ b/vg_include.h
@@ -106,7 +106,7 @@
 
 /* These many bytes below %ESP are considered addressible if we're
    doing the --workaround-gcc296-bugs hack. */
-#define VG_GCC296_BUG_STACK_SLOP 256
+#define VG_GCC296_BUG_STACK_SLOP 1024
 
 /* The maximum number of calls we're prepared to save in a
    backtrace. */
@@ -1152,7 +1152,11 @@ extern void VG_(record_user_err) ( ThreadState* tst,
 
 /* The classification of a faulting address. */
 typedef 
-   enum { Stack, Unknown, Freed, Mallocd, UserG, UserS }
+   enum { Undescribed, /* as-yet unclassified */
+          Stack, 
+          Unknown, /* classification yielded nothing useful */
+          Freed, Mallocd, 
+          UserG, UserS }
    AddrKind;
 
 /* Records info about a faulting address. */
@@ -1168,6 +1172,8 @@ typedef
       ExeContext* lastchange;
       /* Stack */
       ThreadId stack_tid;
+      /* True if is just-below %esp -- could be a gcc bug. */
+      Bool maybe_gcc;
    }
    AddrInfo;