Discussion:
Valgrind: r16306 - /trunk/massif/ms_main.c
(too old to reply)
s***@valgrind.org
2017-04-19 20:15:51 UTC
Permalink
Raw Message
Author: philippe
Date: Wed Apr 19 21:15:50 2017
New Revision: 16306

Log:
Have a cleaner way to remove the massif preload from LD_PRELOAD.

The previous code was removing the massif preload (when --pages-as-heap=yes)
by replacing the entry with spaces.
This is not very clear, and I suspect this gives problems with the
android linker, which seems to use such a space entry as a real entry
to load (and then fails to start the application).

This patch really removes the entry, by shifting the characters.

Tested on amd64/debian.


Modified:
trunk/massif/ms_main.c

Modified: trunk/massif/ms_main.c
==============================================================================
--- trunk/massif/ms_main.c (original)
+++ trunk/massif/ms_main.c Wed Apr 19 21:15:50 2017
@@ -1947,8 +1947,6 @@
{
Int i;
HChar* LD_PRELOAD_val;
- HChar* s;
- HChar* s2;

/* We will record execontext up to clo_depth + overestimate and
we will store this as ec => we need to increase the backtrace size
@@ -1969,35 +1967,42 @@

// If --pages-as-heap=yes we don't want malloc replacement to occur. So we
// disable vgpreload_massif-$PLATFORM.so by removing it from LD_PRELOAD (or
- // platform-equivalent). We replace it entirely with spaces because then
- // the linker doesn't complain (it does complain if we just change the name
- // to a bogus file). This is a bit of a hack, but LD_PRELOAD is setup well
- // before tool initialisation, so this seems the best way to do it.
+ // platform-equivalent). This is a bit of a hack, but LD_PRELOAD is setup
+ // well before tool initialisation, so this seems the best way to do it.
if (clo_pages_as_heap) {
+ HChar* s1;
+ HChar* s2;
+
clo_heap_admin = 0; // No heap admin on pages.

LD_PRELOAD_val = VG_(getenv)( VG_(LD_PRELOAD_var_name) );
tl_assert(LD_PRELOAD_val);

+ VERB(2, "clo_pages_as_heap orig LD_PRELOAD '%s'\n", LD_PRELOAD_val);
+
// Make sure the vgpreload_core-$PLATFORM entry is there, for sanity.
- s2 = VG_(strstr)(LD_PRELOAD_val, "vgpreload_core");
- tl_assert(s2);
+ s1 = VG_(strstr)(LD_PRELOAD_val, "vgpreload_core");
+ tl_assert(s1);

// Now find the vgpreload_massif-$PLATFORM entry.
- s2 = VG_(strstr)(LD_PRELOAD_val, "vgpreload_massif");
- tl_assert(s2);
+ s1 = VG_(strstr)(LD_PRELOAD_val, "vgpreload_massif");
+ tl_assert(s1);
+ s2 = s1;

- // Blank out everything to the previous ':', which must be there because
+ // Position s1 on the previous ':', which must be there because
// of the preceding vgpreload_core-$PLATFORM entry.
- for (s = s2; *s != ':'; s--) {
- *s = ' ';
- }
+ for (; *s1 != ':'; s1--)
+ ;

- // Blank out everything to the end of the entry, which will be '\0' if
- // LD_PRELOAD was empty before Valgrind started, or ':' otherwise.
- for (s = s2; *s != ':' && *s != '\0'; s++) {
- *s = ' ';
- }
+ // Position s2 on the next ':' or \0
+ for (; *s2 != ':' && *s2 != '\0'; s2++)
+ ;
+
+ // Move all characters from s2 to s1
+ while ((*s1++ = *s2++))
+ ;
+
+ VERB(2, "clo_pages_as_heap cleaned LD_PRELOAD '%s'\n", LD_PRELOAD_val);
}

// Print alloc-fns and ignore-fns, if necessary.

Loading...