Fix 341539 VG_(describe_addr) should not describe address as belonging to client

        segment if it is past the heap end

Describes specifically an address that is in the brk data segment
or in the 'reduced' (after brk reduction) section of the brk data segment.

Based on a patch from Ivo Raisr.

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

diff --git a/NEWS b/NEWS
index f3704fe8894c8cad38760a76cb7c8e1d61fa9ead..4597a70b969cd04688dd3fb57be5ea5cf947e68e 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -18,8 +18,8 @@ Release 3.11.0 is under development, not yet released.
 * ==================== OTHER CHANGES ====================
 
 * Address description logic (used by memcheck and helgrind)
-  now describes anonymous or file mmap-ed segments and
-  shared memory segments.
+  now describes addressed in anonymous segments, file mmap-ed
+  segments, shared memory segments and the brk data segment.
 
 * Option --error-markers=<begin>,<end> can be used to mark
   the begin/end of errors in textual output mode, to facilitate
@@ -94,6 +94,8 @@ where XXXXXX is the bug number as listed below.
 340856  disInstr(arm64): unhandled instruction 0x1E634C45 (fcsel)
 340922  arm64: unhandled getgroups/setgroups syscalls
 341238  Recognize GCC5/DWARFv5 DW_LANG constants (Go, C11, C++11, C++14)
+341539  VG_(describe_addr) should not describe address as belonging to client
+        segment if it is past the heap end
 341789  aarch64: shmat fails with valgrind on ARMv8
 342063  wrong format specifier for test mcblocklistsearch in gdbserver_tests
 n-i-bz  Provide implementations of certain compiler builtins to support

diff --git a/coregrind/m_addrinfo.c b/coregrind/m_addrinfo.c
index 533b620aad6bd6097015ad5544d582a227c9081e..290f27922422e10c16598d9fb5e94dd15313a9d8 100644 (file)
--- a/coregrind/m_addrinfo.c
+++ b/coregrind/m_addrinfo.c
@@ -30,6 +30,7 @@
 */
 
 #include "pub_core_basics.h"
+#include "pub_core_clientstate.h"
 #include "pub_core_libcassert.h"
 #include "pub_core_libcbase.h"
 #include "pub_core_libcprint.h"
@@ -263,6 +264,27 @@ void VG_(describe_addr) ( Addr a, /*OUT*/AddrInfo* ai )
    /* Try to find a segment belonging to the client. */
    {
       const NSegment *seg = VG_(am_find_nsegment) (a);
+
+      /* Special case to detect the brk data segment. */
+      if (seg != NULL
+          && seg->kind == SkAnonC
+          && VG_(brk_limit) >= seg->start
+          && VG_(brk_limit) <= seg->end+1) {
+         /* Address a is in a Anon Client segment which contains
+            VG_(brk_limit). So, this segment is the brk data segment
+            as initimg-linux.c:setup_client_dataseg maps an anonymous
+            segment followed by a reservation, with one reservation
+            page that will never be used by syswrap-generic.c:do_brk,
+            when increasing VG_(brk_limit).
+            So, the brk data segment will never be merged with the
+            next segment, and so an address in that area will
+            either be in the brk data segment, or in the unmapped
+            part of the brk data segment reservation. */
+         ai->tag = Addr_BrkSegment;
+         ai->Addr.BrkSegment.brk_limit = VG_(brk_limit);
+         return;
+      }
+
       if (seg != NULL 
           && (seg->kind == SkAnonC 
               || seg->kind == SkFileC
@@ -328,6 +350,9 @@ void VG_(clear_addrinfo) ( AddrInfo* ai)
          VG_(free)(ai->Addr.SectKind.objname);
          break;
 
+      case Addr_BrkSegment:
+         break;
+
       case Addr_SegmentKind:
          VG_(free)(ai->Addr.SegmentKind.filename);
          break;
@@ -586,6 +611,26 @@ static void pp_addrinfo_WRK ( Addr a, const AddrInfo* ai, Bool mc,
          }
          break;
 
+      case Addr_BrkSegment:
+         if (a < ai->Addr.BrkSegment.brk_limit)
+            VG_(emit)( "%sAddress 0x%llx is in the brk data segment"
+                       " 0x%llx-0x%llx%s\n",
+                       xpre,
+                       (ULong)a,
+                       (ULong)VG_(brk_base),
+                       (ULong)ai->Addr.BrkSegment.brk_limit - 1,
+                       xpost );
+         else
+            VG_(emit)( "%sAddress 0x%llx is %lu bytes after "
+                       "the brk data segment limit"
+                       " 0x%llx%s\n",
+                       xpre,
+                       (ULong)a,
+                       a - ai->Addr.BrkSegment.brk_limit,
+                       (ULong)ai->Addr.BrkSegment.brk_limit,
+                       xpost );
+         break;
+
       case Addr_SegmentKind:
          VG_(emit)( "%sAddress 0x%llx is in "
                     "a %s%s%s %s%s%pS segment%s\n",

diff --git a/include/pub_tool_addrinfo.h b/include/pub_tool_addrinfo.h
index b8fc63463912b529ae16e93f15e52c264ad51fdc..3b1c45ff7588ce26877f64e9993691535c051c8e 100644 (file)
--- a/include/pub_tool_addrinfo.h
+++ b/include/pub_tool_addrinfo.h
@@ -72,6 +72,7 @@ typedef
       Addr_DataSym,     // in a global data sym
       Addr_Variable,    // variable described by the debug info
       Addr_SectKind,    // Section from a mmap-ed object file
+      Addr_BrkSegment,  // address in brk data segment
       Addr_SegmentKind  // Client segment (mapped memory)
    }
    AddrTag;
@@ -174,6 +175,15 @@ struct _AddrInfo {
          VgSectKind kind;
       } SectKind;
 
+      // Described address is or was in the brk data segment.
+      // brk_limit is the limit that was in force
+      // at the time address was described. 
+      // If address is >= brk_limit, it means address is in a zone
+      // of the data segment that was shrinked.
+      struct {
+         Addr brk_limit; // limit in force when address was described.
+      } BrkSegment;
+
       struct {
          SegKind segkind;   // SkAnonC, SkFileC or SkShmC.
          HChar   *filename; // NULL if segkind != SkFileC

diff --git a/memcheck/tests/dw4.c b/memcheck/tests/dw4.c
index 38c2421d3fec2c62eb74bc6df57fbd10f11f6553..574f97c7191a650f6c6f88b64e8c509e8a49e6c1 100644 (file)
--- a/memcheck/tests/dw4.c
+++ b/memcheck/tests/dw4.c
@@ -19,11 +19,11 @@
 #include "memcheck/memcheck.h"
 
 /* Cause memcheck to complain about the address "a" and so to print
-   its best guess as to what "a" actually is.  a must be
-   addressible. */
-
+   its best guess as to what "a" actually is.*/
 void croak ( void* aV )
 {
+  if(VALGRIND_CHECK_MEM_IS_ADDRESSABLE(aV,1) != 0)
+     return;
   char* a = (char*)aV;
   char* undefp = malloc(1);
   char saved = *a;
@@ -82,5 +82,15 @@ int main ( void )
   croak( q);
   unlink(filename);
 
+  /* Describe memory in or past the heap end. */
+  void *addr = sbrk(0);
+  croak(addr); // in the first brk page, after brk_limit
+  sbrk(4 * 1024); // increase brk segment
+  croak(addr); // Now, must be inside.
+  addr = (void *) ((char*)addr + 2 * 1024);
+  croak(addr); // Must still be inside.
+  sbrk(-3*1024);
+  croak(addr); // Must now be after.
+  
   return 0;
 }

diff --git a/memcheck/tests/dw4.stderr.exp b/memcheck/tests/dw4.stderr.exp
index 605d9bdc0c9fd0ed758c3a83ae7c23e91326a77e..e396a43b8ab3a240a8cf9e5f6ea0fcf6fe419d8c 100644 (file)
--- a/memcheck/tests/dw4.stderr.exp
+++ b/memcheck/tests/dw4.stderr.exp
@@ -27,3 +27,23 @@ Uninitialised byte(s) found during client check request
    by 0x........: main (dw4.c:82)
  Address 0x........ is in a rw- mapped file valgrind-dw4-test.PID segment
 
+Uninitialised byte(s) found during client check request
+   at 0x........: croak (dw4.c:32)
+   by 0x........: main (dw4.c:87)
+ Address 0x........ is 0 bytes after the brk data segment limit 0x........
+
+Uninitialised byte(s) found during client check request
+   at 0x........: croak (dw4.c:32)
+   by 0x........: main (dw4.c:89)
+ Address 0x........ is in the brk data segment 0x........-0x........
+
+Uninitialised byte(s) found during client check request
+   at 0x........: croak (dw4.c:32)
+   by 0x........: main (dw4.c:91)
+ Address 0x........ is in the brk data segment 0x........-0x........
+
+Unaddressable byte(s) found during client check request
+   at 0x........: croak (dw4.c:25)
+   by 0x........: main (dw4.c:93)
+ Address 0x........ is 1024 bytes after the brk data segment limit 0x........
+