Reuben!

Per Chet's last comment line

  There's no place where that value is part of a data segment.

I think that's a misreading of V's (admittedly ambiguous) claim about
the segment.  I think it means the brk segment.  And one way that could
happen is that the allocated block has for whatever reason been allocated
by the system (glibc?) malloc, which puts it in the brk segment, but has
been passed to the Valgrind-provided free function, hence causing the
reported error.  (Note -- unproven theory, at best).

So it might be some kind of malloc/free interception problem.  Have there
been any changes to bash lately that might account for that?

Julian!

Note that this problem goes back to at least bash 4.2, i.e. February 2011, as per the Debian bug report.

By default, bash seems to use its own malloc (and certainly in the case where I do a default "./configure && make" from git sources), so I guess it's easily possible.

And indeed configuring as

./configure --without-bash-malloc

works nicely.

(My reference to bash 4.2 in the Debian bug report is bogus, sorry, although that doesn't really matter now.)

(In reply to rrt from comment #2)
> By default, bash seems to use its own malloc (and certainly in the case
> where I do a default "./configure && make" from git sources), so I guess
> it's easily possible.

As a side note: in that case, what you report is symptomatic of bash
using its own malloc to allocate a block but the system free to release
it.  Could that be the case?

A bit of gdb does suggest that this is the case: i.e. bash's sh_malloc is called to allocate (and is not intercepted) but glibc's free is called to free.

I've updated the bash bug report thread, will confirm the outcome here.

This could be also confirmed by running Valgrind with --trace-malloc, for example:
   $ valgrind --trace-malloc=yes bash -c 'exit 0'

Free'd address should have appeared previously in one of malloc/calloc/realloc/...

I think the problem is that bash not only has its own malloc/free implementation (valgrind should intercept that just fine). But also has malloc wrappers for some malloc functions that call their internal counterparts directly (so valgrind won't know that is a matching allocation/deallocation function).

See xmalloc.h:

#if defined(USING_BASH_MALLOC) && !defined (DISABLE_MALLOC_WRAPPERS)
extern PTR_T sh_xmalloc __P((size_t, const char *, int));
extern PTR_T sh_xrealloc __P((void *, size_t, const char *, int));
extern void sh_xfree __P((void *, const char *, int));

#define xmalloc(x)      sh_xmalloc((x), __FILE__, __LINE__)
#define xrealloc(x, n)  sh_xrealloc((x), (n), __FILE__, __LINE__)
#define xfree(x)        sh_xfree((x), __FILE__, __LINE__)

#ifdef free
#undef free
#endif
#define free(x)         sh_xfree((x), __FILE__, __LINE__)
#endif  /* USING_BASH_MALLOC */

Yep, that seems to be it. If you explicitly add the following to config.h (I couldn't find the configure option that does that):

#define DISABLE_MALLOC_WRAPPERS 1

Then a bash build from source seems to work fine under valgrind (and finds a couple of small memory leaks).

Thanks very much Mark.

What's the solution? A suppression? (It would seem to go against Valgrind's philosophy to say "build bash without the (debugging) wrappers"!) But as highlighted in the Debian thread, it would be good to fix this.

(The other obvious this would be simply to build --without-bash-malloc; the bash maintainers say this would have performance implications.)

Or can valgrind be taught about the wrappers?

(In reply to rrt from comment #10)
> What's the solution? A suppression? (It would seem to go against Valgrind's
> philosophy to say "build bash without the (debugging) wrappers"!) But as
> highlighted in the Debian thread, it would be good to fix this.
> 
> (The other obvious this would be simply to build --without-bash-malloc; the
> bash maintainers say this would have performance implications.)
> 
> Or can valgrind be taught about the wrappers?

Some distros (Fedora in particular) already build with --without-bash-malloc. So it seems well tested. I would encourage the bash maintainer to contact the glibc hackers and show them some benchmarks where the generic system malloc performs worse than the bash one. They are really interested in making sure the system malloc performs well for everybody.

If you want to keep the bash specific malloc implementation, but want to restore the malloc interposition that breaks valgrind (and anything else that depends on malloc interposition working correctly) then building with -DDISABLE_MALLOC_WRAPPERS=1 seems like the easiest way forward. You might want to ask the bash maintainers whether they feel that is a supported option. It seems to only be used to some malloc debug/tracing messages.

There are ways to teach valgrind about allocators that don't obey malloc interposition, but they are slightly involved. See http://www.valgrind.org/docs/manual/mc-manual.html#mc-manual.clientreqs

If you do want to make the sh_xmalloc wrappers work for valgrind with normal malloc interposition (like the normal xmalloc calls) then you could make them just call malloc/free directly when running under valgrind (it would obviously still be broken for other malloc interposers):

diff -ur bash-4.4/xmalloc.c bash-4.4.valgrind/xmalloc.c
--- bash-4.4/xmalloc.c	2016-08-26 15:08:14.000000000 +0200
+++ bash-4.4.valgrind/xmalloc.c	2017-04-14 15:49:26.832197332 +0200
@@ -35,6 +35,8 @@
 #  include "ansi_stdlib.h"
 #endif /* HAVE_STDLIB_H */
 
+#include <valgrind/valgrind.h>
+
 #include "error.h"
 
 #include "bashintl.h"
@@ -180,7 +182,10 @@
 #endif
 
   FINDBRK();
-  temp = sh_malloc (bytes, file, line);
+  if (RUNNING_ON_VALGRIND)
+    temp = malloc (bytes);
+  else
+    temp = sh_malloc (bytes, file, line);
 
   if (temp == 0)
     sh_allocerr ("xmalloc", bytes, file, line);
@@ -203,7 +208,10 @@
 #endif
 
   FINDBRK();
-  temp = pointer ? sh_realloc (pointer, bytes, file, line) : sh_malloc (bytes, file, line);
+  if (RUNNING_ON_VALGRIND)
+    temp = pointer ? realloc (pointer, bytes) : malloc (bytes);
+  else
+    temp = pointer ? sh_realloc (pointer, bytes, file, line) : sh_malloc (bytes, file, line);
 
   if (temp == 0)
     sh_allocerr ("xrealloc", bytes, file, line);
@@ -218,6 +226,11 @@
      int line;
 {
   if (string)
-    sh_free (string, file, line);
+    {
+      if (RUNNING_ON_VALGRIND)
+	free (string);
+      else
+	sh_free (string, file, line);
+    }
 }
 #endif

Thanks very much for the detailed guidance, Mark.

Closing. I think "WONTFIX" is strictly accurate, as Valgrind's diagnosis is indeed incorrect.