|
From: <sv...@va...> - 2011-12-22 13:30:40
|
Author: philippe
Date: 2011-12-22 13:25:58 +0000 (Thu, 22 Dec 2011)
New Revision: 12314
Log:
* none/tests/linux/mremap3.vgtest : new test
mremap3.c based on testcase provided by Jan Engelhardt
* coregrind/m_syswrap/syswrap-generic.c
- The two 'no-thrash checks' that were introduced to fix bug #129866
were (probably) broken when adress space manager was reworked.
The new VG_(am_get_advisory_client_simple) returns NULL for a free
segment, but the check was based on checking not NULL and then
that the state is free.
=> replaces these two local checks by a call to the new
am Bool VG_(am_covered_by_single_free_segment) function.
* coregrind/pub_core_aspacemgr.h
coregrind/m_aspacemgr/aspacemgr-linux.c
- new function Bool VG_(am_covered_by_single_free_segment)
Added:
trunk/none/tests/linux/mremap3.c
trunk/none/tests/linux/mremap3.stderr.exp
trunk/none/tests/linux/mremap3.stdout.exp
trunk/none/tests/linux/mremap3.vgtest
Modified:
trunk/NEWS
trunk/coregrind/m_aspacemgr/aspacemgr-linux.c
trunk/coregrind/m_syswrap/syswrap-generic.c
trunk/coregrind/pub_core_aspacemgr.h
trunk/none/tests/linux/Makefile.am
Modified: trunk/NEWS
===================================================================
--- trunk/NEWS 2011-12-19 11:03:07 UTC (rev 12313)
+++ trunk/NEWS 2011-12-22 13:25:58 UTC (rev 12314)
@@ -23,6 +23,7 @@
https://bugs.kde.org/show_bug.cgi?id=XXXXXX
where XXXXXX is the bug number as listed below.
+276993 fix mremap 'no thrash checks'
283413 Fix wrong sanity check
286270 vgpreload is not friendly to 64->32 bit execs, gives ld.so warnings
286374 Running cachegrind with --branch-sim=yes on 64-bit PowerPC program fails
Modified: trunk/coregrind/m_aspacemgr/aspacemgr-linux.c
===================================================================
--- trunk/coregrind/m_aspacemgr/aspacemgr-linux.c 2011-12-19 11:03:07 UTC (rev 12313)
+++ trunk/coregrind/m_aspacemgr/aspacemgr-linux.c 2011-12-22 13:25:58 UTC (rev 12314)
@@ -1984,7 +1984,29 @@
return VG_(am_get_advisory)( &mreq, True/*client*/, ok );
}
+/* Similar to VG_(am_find_nsegment) but only returns free segments. */
+static NSegment const * VG_(am_find_free_nsegment) ( Addr a )
+{
+ Int i = find_nsegment_idx(a);
+ aspacem_assert(i >= 0 && i < nsegments_used);
+ aspacem_assert(nsegments[i].start <= a);
+ aspacem_assert(a <= nsegments[i].end);
+ if (nsegments[i].kind == SkFree)
+ return &nsegments[i];
+ else
+ return NULL;
+}
+Bool VG_(am_covered_by_single_free_segment)
+ ( Addr start, SizeT len)
+{
+ NSegment const* segLo = VG_(am_find_free_nsegment)( start );
+ NSegment const* segHi = VG_(am_find_free_nsegment)( start + len - 1 );
+
+ return segLo != NULL && segHi != NULL && segLo == segHi;
+}
+
+
/* Notifies aspacem that the client completed an mmap successfully.
The segment array is updated accordingly. If the returned Bool is
True, the caller should immediately discard translations from the
Modified: trunk/coregrind/m_syswrap/syswrap-generic.c
===================================================================
--- trunk/coregrind/m_syswrap/syswrap-generic.c 2011-12-19 11:03:07 UTC (rev 12313)
+++ trunk/coregrind/m_syswrap/syswrap-generic.c 2011-12-22 13:25:58 UTC (rev 12314)
@@ -403,16 +403,8 @@
non-fixed, which is not what we want */
advised = VG_(am_get_advisory_client_simple)( needA, needL, &ok );
if (ok) {
- /* VG_(am_get_advisory_client_simple) (first arg == 0, meaning
- this-or-nothing) is too lenient, and may allow us to trash
- the next segment along. So make very sure that the proposed
- new area really is free. This is perhaps overly
- conservative, but it fixes #129866. */
- NSegment const* segLo = VG_(am_find_nsegment)( needA );
- NSegment const* segHi = VG_(am_find_nsegment)( needA + needL - 1 );
- if (segLo == NULL || segHi == NULL
- || segLo != segHi || segLo->kind != SkFree)
- ok = False;
+ /* Fixes bug #129866. */
+ ok = VG_(am_covered_by_single_free_segment) ( needA, needL );
}
if (ok && advised == needA) {
ok = VG_(am_extend_map_client)( &d, (NSegment*)old_seg, needL );
@@ -466,15 +458,8 @@
non-fixed, which is not what we want */
advised = VG_(am_get_advisory_client_simple)( needA, needL, &ok );
if (ok) {
- /* VG_(am_get_advisory_client_simple) (first arg == 0, meaning
- this-or-nothing) is too lenient, and may allow us to trash
- the next segment along. So make very sure that the proposed
- new area really is free. */
- NSegment const* segLo = VG_(am_find_nsegment)( needA );
- NSegment const* segHi = VG_(am_find_nsegment)( needA + needL - 1 );
- if (segLo == NULL || segHi == NULL
- || segLo != segHi || segLo->kind != SkFree)
- ok = False;
+ /* Fixes bug #129866. */
+ ok = VG_(am_covered_by_single_free_segment) ( needA, needL );
}
if (!ok || advised != needA)
goto eNOMEM;
Modified: trunk/coregrind/pub_core_aspacemgr.h
===================================================================
--- trunk/coregrind/pub_core_aspacemgr.h 2011-12-19 11:03:07 UTC (rev 12313)
+++ trunk/coregrind/pub_core_aspacemgr.h 2011-12-22 13:25:58 UTC (rev 12314)
@@ -151,6 +151,17 @@
extern Addr VG_(am_get_advisory_client_simple)
( Addr start, SizeT len, /*OUT*/Bool* ok );
+/* Returns True if [start, start + len - 1] is covered by a single
+ free segment, otherwise returns False.
+ This allows to check the following case:
+ VG_(am_get_advisory_client_simple) (first arg == 0, meaning
+ this-or-nothing) is too lenient, and may allow us to trash
+ the next segment along. So make very sure that the proposed
+ new area really is free. This is perhaps overly
+ conservative, but it fixes #129866. */
+extern Bool VG_(am_covered_by_single_free_segment)
+ ( Addr start, SizeT len);
+
/* Notifies aspacem that the client completed an mmap successfully.
The segment array is updated accordingly. If the returned Bool is
True, the caller should immediately discard translations from the
Modified: trunk/none/tests/linux/Makefile.am
===================================================================
--- trunk/none/tests/linux/Makefile.am 2011-12-19 11:03:07 UTC (rev 12313)
+++ trunk/none/tests/linux/Makefile.am 2011-12-22 13:25:58 UTC (rev 12314)
@@ -7,12 +7,14 @@
blockfault.stderr.exp blockfault.vgtest \
mremap.stderr.exp mremap.stderr.exp-glibc27 mremap.stdout.exp \
mremap.vgtest \
- mremap2.stderr.exp mremap2.stdout.exp mremap2.vgtest
+ mremap2.stderr.exp mremap2.stdout.exp mremap2.vgtest \
+ mremap3.stderr.exp mremap3.stdout.exp mremap3.vgtest
check_PROGRAMS = \
blockfault \
mremap \
- mremap2
+ mremap2 \
+ mremap3
AM_CFLAGS += $(AM_FLAG_M3264_PRI)
Added: trunk/none/tests/linux/mremap3.c
===================================================================
--- trunk/none/tests/linux/mremap3.c (rev 0)
+++ trunk/none/tests/linux/mremap3.c 2011-12-22 13:25:58 UTC (rev 12314)
@@ -0,0 +1,40 @@
+#define _GNU_SOURCE 1
+#include <sys/mman.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+int main(void)
+{
+ /* first find free segment of 40K, then unmap it */
+ void *initial_area = mmap((void *)0x10000000, 40960, PROT_READ|PROT_WRITE,
+ MAP_ANONYMOUS|MAP_PRIVATE,0,0);
+
+ if (initial_area == MAP_FAILED)
+ perror ("initial area");
+ printf("initial_area= %p\n", initial_area);
+ if (munmap(initial_area, 40960) != 0)
+ perror ("munmap initial_area");
+
+ /* remap the same segment, but with 4K size */
+ void *area = mmap(initial_area, 4096, PROT_READ|PROT_WRITE,
+ MAP_ANONYMOUS|MAP_PRIVATE,0,0);
+ if (area == MAP_FAILED)
+ perror ("area");
+ if (area != initial_area)
+ printf("FAILED : was expecting to get back the initial_area\n");
+ printf("area= %p\n", area);
+ strcpy(area, "Hello World");
+
+ /* extend it to 40K */
+ void *a2 = mremap(area, 4096, 40960, 0);
+ if (a2 == MAP_FAILED) {
+ perror("mremap");
+ }
+ if (a2 != initial_area)
+ printf("FAILED : was expecting to get back the same area increased\n");
+ printf("increased area= %p\n", a2);
+ printf("%s\n", (char *)a2);
+ return 0;
+}
Added: trunk/none/tests/linux/mremap3.stderr.exp
===================================================================
--- trunk/none/tests/linux/mremap3.stderr.exp (rev 0)
+++ trunk/none/tests/linux/mremap3.stderr.exp 2011-12-22 13:25:58 UTC (rev 12314)
@@ -0,0 +1,2 @@
+
+
Added: trunk/none/tests/linux/mremap3.stdout.exp
===================================================================
--- trunk/none/tests/linux/mremap3.stdout.exp (rev 0)
+++ trunk/none/tests/linux/mremap3.stdout.exp 2011-12-22 13:25:58 UTC (rev 12314)
@@ -0,0 +1,4 @@
+initial_area= 0x........
+area= 0x........
+increased area= 0x........
+Hello World
Added: trunk/none/tests/linux/mremap3.vgtest
===================================================================
--- trunk/none/tests/linux/mremap3.vgtest (rev 0)
+++ trunk/none/tests/linux/mremap3.vgtest 2011-12-22 13:25:58 UTC (rev 12314)
@@ -0,0 +1,2 @@
+prog: mremap3
+stdout_filter: ../../../tests/filter_addresses
|