Re: Problem with valgrind-tests: relies on bash not causing error

[Reuben Thomas]

On 5 August 2017 at 19:46, Paul Eggert  wrote:

> Thanks, I installed the attached somewhat-more-fancier patch; does it work
> for you?
>

​That works fine; thanks very much.

-- 
https://rrt.sc3d.org 

Re: Problem with valgrind-tests: relies on bash not causing error

[Paul Eggert]

Thanks, I installed the attached somewhat-more-fancier patch; does it work for 
you?
>From 59c181fa400ead71e9a28e3db9abc107f8fc9ea3 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 5 Aug 2017 11:45:10 -0700
Subject: [PATCH] valgrind-tests: use ls, and cache

* m4/valgrind-tests.m4: Test ls, not bash.
Problem reported by Reuben Thomas.
Also, cache the result so that it can be overridden.
---
 ChangeLog|  7 +++
 m4/valgrind-tests.m4 | 30 +++---
 2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index caab63e..74e9831 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2017-08-05  Paul Eggert  
+
+	valgrind-tests: use ls, and cache
+	* m4/valgrind-tests.m4: Test ls, not bash.
+	Problem reported by Reuben Thomas.
+	Also, cache the result so that it can be overridden.
+
 2017-08-04  Paul Eggert  
 
 	manywarnings: port to 64-bit GCC builds of Emacs
diff --git a/m4/valgrind-tests.m4 b/m4/valgrind-tests.m4
index 00189d8..7e4bf60 100644
--- a/m4/valgrind-tests.m4
+++ b/m4/valgrind-tests.m4
@@ -1,4 +1,4 @@
-# valgrind-tests.m4 serial 3
+# valgrind-tests.m4 serial 4
 dnl Copyright (C) 2008-2017 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -11,27 +11,27 @@ dnl From Simon Josefsson
 # Check if valgrind is available, and set VALGRIND to it if available.
 AC_DEFUN([gl_VALGRIND_TESTS],
 [
-  AC_ARG_ENABLE(valgrind-tests,
+  AC_ARG_ENABLE([valgrind-tests],
 AS_HELP_STRING([--disable-valgrind-tests],
[don't try to run self tests under valgrind]),
 [opt_valgrind_tests=$enableval], [opt_valgrind_tests=yes])
 
   # Run self-tests under valgrind?
   if test "$opt_valgrind_tests" = "yes" && test "$cross_compiling" = no; then
-AC_CHECK_PROGS(VALGRIND, valgrind)
-  fi
+AC_CHECK_PROGS([VALGRIND], [valgrind])
 
-  OPTS="-q --error-exitcode=1 --leak-check=full"
+if test "$VALGRIND"; then
+  AC_CACHE_CHECK([for valgrind options for tests],
+[gl_cv_opt_valgrind_tests],
+[gl_cv_opt_valgrind_tests="-q --error-exitcode=1 --leak-check=full"
+ $VALGRIND $gl_valgrind_opts ls > /dev/null 2>&1 ||
+   gl_cv_opt_valgrind_tests=no])
 
-  if test -n "$VALGRIND" \
- && $VALGRIND $OPTS $SHELL -c 'exit 0' > /dev/null 2>&1; then
-opt_valgrind_tests=yes
-VALGRIND="$VALGRIND $OPTS"
-  else
-opt_valgrind_tests=no
-VALGRIND=
+  if test "$gl_cv_opt_valgrind_tests" != no; then
+VALGRIND="$VALGRIND $gl_cv_opt_valgrind_tests"
+  else
+VALGRIND=
+  fi
+fi
   fi
-
-  AC_MSG_CHECKING([whether self tests are run under valgrind])
-  AC_MSG_RESULT($opt_valgrind_tests)
 ])
-- 
2.7.4

Re: Problem with valgrind-tests: relies on bash not causing error

[Reuben Thomas]

On 12 April 2017 at 13:36, Reuben Thomas  wrote:

> The test for whether to use valgrind runs:
>
> /bin/bash -c 'exit 0'
>
> This looks pretty harmless; unfortunately, bash itself causes problems:
>
> $ valgrind -q --error-exitcode=1 --leak-check=full /bin/bash -c 'exit 0'
> ==32197== Invalid free() / delete / delete[] / realloc()
> ==32197==at 0x4C2ED5B: free (in /usr/lib/valgrind/vgpreload_
> memcheck-amd64-linux.so)
> ==32197==by 0x45E1D0: ??? (in /bin/bash)
> ==32197==by 0x45E37F: run_unwind_frame (in /bin/bash)
> ==32197==by 0x47B664: parse_and_execute (in /bin/bash)
> ==32197==by 0x4209D6: ??? (in /bin/bash)
> ==32197==by 0x41F893: main (in /bin/bash)
> ==32197==  Address 0x423b828 is in the brk data segment 0x4228000-0x423bfff
> ==32197==
>
> Here I was using the provided Ubuntu 16.04 build of bash:
>
> $ bash --version
> GNU bash, version 4.3.46(1)-release (x86_64-pc-linux-gnu)
> Copyright (C) 2013 Free Software Foundation, Inc.
>
> Maybe use some other known-available utility that doesn't play weird
> tricks with memory? /bin/ls works for me!
>

​Attached, a patch to fix this. (I believe this change is still needed.)

-- 
https://rrt.sc3d.org 


valgrind-tests.m4
Description: application/m4

Re: Problem with valgrind-tests: relies on bash not causing error

[Bernhard Voelker]

On 04/14/2017 10:56 PM, Marc Nieper-Wißkirchen wrote:
> While your proposed solution would work for anyone who is aware of this 
> problem, it won't work for an unaware user who would simply run ./configure, 
> wondering why valgrind isn't detected. As it seems
> rather involved to resolve the incompatibility between bash and valgrind, I 
> would like to ask to have gnulib's valgrind-tests changed upstream. As this 
> change amounts simply in replacing $(SHELL) by
> any other well-known utility, the changes to be done are trivial.

/bin/true seems fine, doesn't it?

Have a nice day,
Berny

Re: Problem with valgrind-tests: relies on bash not causing error

[Marc Nieper-Wißkirchen]

Hi Bruno,

Hi Reuben,

>/The test for whether to use valgrind runs:/
>//
>//bin/bash -c 'exit 0'/
>//
>/This looks pretty harmless; unfortunately, bash itself causes problems:/

I'd suggest that you change m4/valgrind-tests.m4 to use the AC_CACHE_CHECK
macro instead of "lone" AC_MSG_CHECKING and AC_MSG_RESULT invocations.
Then you have an cache variable that you set as an environment variable
in order to override the result of this test, e.g.
  $ env gl_cv_use_valgrind=yes ./configure

Remember, nowadays that the autoconf cache is disabled by default, the
major benefit of AC_CACHE_CHECK is that it provides the user a way to
selectively override configure test results.

Bruno

Today I have run into the same problem (namely that valgrind and bash
are not going along very well).

While your proposed solution would work for anyone who is aware of this
problem, it won't work for an unaware user who would simply run
./configure, wondering why valgrind isn't detected. As it seems rather
involved to resolve the incompatibility between bash and valgrind, I
would like to ask to have gnulib's valgrind-tests changed upstream. As
this change amounts simply in replacing $(SHELL) by any other well-known
utility, the changes to be done are trivial.

Having an updated version upstream makes it much easier for everyone  to
pick up the necessary changes to make valgrind-tests work.

Marc

Re: Problem with valgrind-tests: relies on bash not causing error

[Reuben Thomas]

On 12 April 2017 at 15:23, Bruno Haible  wrote:

> Hi Reuben,
>
> > This is a separate issue, though: the real problem here is that a way is
> > needed to run libtool wrappers with libtool
>
> In GNU gettext, I never succeeded in making valgrind work with the libtool
> wrappers; instead I noted
>   # You should build with --disable-shared when using valgrind.
>
> See [1], lines 170..199.
>
> Bruno
>
> [1] http://git.savannah.gnu.org/gitweb/?p=gettext.git;a=blob;
> f=gettext-tools/tests/Makefile.am
>

This is not feasible for ​Enchant, which is based around dynamically-loaded
libraries.

​However, I just used a one line shell script, called run-test:

#!/bin/sh
libtool --mode=execute $VALGRIND "$@"
​
​and then in Makefile.am:

​LOG_COMPILER = $(srcdir)/run-test

It is working for me, and I'm happily fixing the bugs found by Valgrind.

-- 
http://rrt.sc3d.org

Re: Problem with valgrind-tests: relies on bash not causing error

[Bruno Haible]

Hi Reuben,

> This is a separate issue, though: the real problem here is that a way is
> needed to run libtool wrappers with libtool

In GNU gettext, I never succeeded in making valgrind work with the libtool
wrappers; instead I noted
  # You should build with --disable-shared when using valgrind.

See [1], lines 170..199.

Bruno

[1] 
http://git.savannah.gnu.org/gitweb/?p=gettext.git;a=blob;f=gettext-tools/tests/Makefile.am

Re: Problem with valgrind-tests: relies on bash not causing error

[Bruno Haible]

Hi Reuben,

> The test for whether to use valgrind runs:
> 
> /bin/bash -c 'exit 0'
> 
> This looks pretty harmless; unfortunately, bash itself causes problems:

I'd suggest that you change m4/valgrind-tests.m4 to use the AC_CACHE_CHECK
macro instead of "lone" AC_MSG_CHECKING and AC_MSG_RESULT invocations.
Then you have an cache variable that you set as an environment variable
in order to override the result of this test, e.g.
  $ env gl_cv_use_valgrind=yes ./configure

Remember, nowadays that the autoconf cache is disabled by default, the
major benefit of AC_CACHE_CHECK is that it provides the user a way to
selectively override configure test results.

Bruno

Re: Problem with valgrind-tests: relies on bash not causing error

[Reuben Thomas]

​Having applied the workaround of using "ls" in the configure test, my
tests still don't run because the LOG_COMPILER trick suggested in the
gnulib manual doesn't take account of libtool wrappers. Net result:
valgrind is being called on bash (as it runs the test binary's wrapper
script).

This is a separate issue, though: the real problem here is that a way is
needed to run libtool wrappers with libtool, so that they are executed as:

libtool --mode=execute valgrind $LIBTOOL_WRAPPER_SCRIPT

The problem is that it's not obvious where the responsibility should lie. I
don't expect automake to go examining all my test scripts to see if they
look like libtool wrappers; on the other hand, I'm not sure what libtool
can do, since by the time its wrapper has been run under valgrind, it's too
late.

For now I'll simply write manual scripts to call my tests and pass the
value of $VALGRIND in, but I would appreciate suggestions here. I find
valgrind invaluable, so making it easy to run such tests (so that it's just
a question of adding the "valgrind-tests" module to one's project and
making a couple of Makefile.am settings) would be great.

I guess one further option is to fix the bug in bash, and then call
valgrind with --trace-children=yes. Maybe that would be cleanest and
simplest (albeit we'll need a workaround until fixed bash is widely
available).

-- 
http://rrt.sc3d.org