[Replicant] [libsamsung-ipc] [PATCH 5/9] Autotools: add support for Valgrind for checks
Denis 'GNUtoo' Carikli
GNUtoo at cyberdimension.org
Mon May 23 16:56:13 UTC 2022
This enables contributors to run Valgrind when running 'make check'
for instance before sending patches.
The --enable-valgrind option was used instead of using
--enable-checking=valgrind like GCC uses as the code for that is much
easier to get right.
As Valgrind is very slow, I had to increase the timeouts in the
ipc-modem tests. For now I settled on 60 seconds to have some margin
for machines or setups that are potentially slower than the one I use.
For the guix.scm, with Guix 1.3.0, Valgrind needs the ld.so .symtab to
work, so as Valgrind doesn't know where to find them, so we had to use
--extra-debuginfo-path for that.
This is also why we pass '-v' to valgrind as it prints message
explaining the issue when it doesn't find the required symbols.
And we also needed a trick mentioned in the Guix mailing list[1] to
find the right path for the debug information as the glibc being used
by packages isn't exported publicly.
[1]https://lists.gnu.org/archive/html/help-guix/2022-03/msg00036.html
The proper fix will be merged in the next Guix release (merging it now
is not possible since it would require to rebuild everything, and that
is only possible with new releases).
A temporary workaround with 4d1a88312cbebb10d47905d6d023953ac85bcd04
(gnu: valgrind: Allow ld.so symbols to be found.) was also merged in
the current Guix so it is possible to get it with guix pull, but here
it's probably better to stick with a workaround in the guix.scm for
now as this also enables to use older guix revisions for testing.
Thanks to the people on #guix on Liberachat for helping me to solve
this Valgrind issue.
Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo at cyberdimension.org>
---
configure.ac | 22 +++++++++++-
samsung-ipc/tests/Makefile.am | 3 ++
samsung-ipc/tests/libsamsung-ipc-test.py | 9 ++++-
scripts/PKGBUILD | 7 ++--
scripts/guix.scm | 22 +++++++++---
tools/Makefile.am | 3 ++
tools/tests/ipc-modem.py | 44 +++++++++++++++++++-----
tools/tests/nv_data-imei.py | 10 +++++-
tools/tests/nv_data-md5.py | 10 +++++-
9 files changed, 112 insertions(+), 18 deletions(-)
diff --git a/configure.ac b/configure.ac
index b5b694b..f55064c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -18,6 +18,7 @@ PKG_PROG_PKG_CONFIG
PKG_CHECK_MODULES(OPENSSL, openssl >= $OPENSSL_REQUIRED)
AC_SUBST(OPENSSL_CFLAGS)
AC_SUBST(OPENSSL_LIBS)
+AC_SUBST(VALGRIND)
#------------------------------------------------------------------------------
# check for debugging
@@ -28,7 +29,23 @@ AC_ARG_ENABLE(debug,
AM_CONDITIONAL( [WANT_DEBUG], [test x"$debug" = x"yes"])
#------------------------------------------------------------------------------
+# TODO: GCC has --enable-checking= for extensive runtime checks and one of the
+# available values is valgrind, so it would be a good idea to convert to
+# it for consistency across free software projects.
+AC_ARG_ENABLE(valgrind-tests,
+ [ --enable-valgrind-tests Build with -ggdb3, -O0 and use valgrind from PATH during tests (default=disabled)],
+ [valgrind_tests=$enableval],
+ [valgrind_tests="no"])
+AM_CONDITIONAL( [WANT_VALGRIND_CHECKING], [test x"$valgrind_tests" = x"yes"])
+
+AS_IF([test x"$valgrind_tests" = x"yes"],
+ [AC_CHECK_PROG([VALGRIND], [valgrind], [valgrind])
+ AS_IF(
+ [test x"$VALGRIND" = x""],
+ [AC_MSG_ERROR(
+ [valgrind tests are enabled but valgrind was not found in PATH ($PATH)])])])
+#------------------------------------------------------------------------------
AC_CONFIG_FILES([
Makefile
samsung-ipc.pc
@@ -40,7 +57,8 @@ AC_CONFIG_FILES([
#------------------------------------------------------------------------------
-AS_IF([test x"debug" = x"yes"], [: ${CFLAGS="-ggdb -O0"}], [])
+AS_IF([test x"$valgrind_tests" = x"yes"], [: ${CFLAGS="-ggdb3 -O0"}],
+ [test x"debug" = x"yes"], [: ${CFLAGS="-ggdb -O0"}], [])
AC_PROG_CC
AM_PROG_CC_C_O
@@ -66,6 +84,8 @@ echo " debug build.............: $debug"
echo
echo " prefix..................: $prefix"
echo
+echo " valgrind tests..........: $valgrind_tests"
+echo
echo "------------------------------------------------------------------------"
echo
echo "Now type 'make' to compile and 'make install' to install this package."
diff --git a/samsung-ipc/tests/Makefile.am b/samsung-ipc/tests/Makefile.am
index cc63d0a..1a7a56b 100644
--- a/samsung-ipc/tests/Makefile.am
+++ b/samsung-ipc/tests/Makefile.am
@@ -24,6 +24,9 @@ libsamsung_ipc_test_LDFLAGS =
# TODO: Find a way to make test more modular and represent each run of
# libsamsung-ipc-test in TEST while having it implemented in a single
# python file
+if WANT_VALGRIND_CHECKING
+AM_TESTS_ENVIRONMENT = VALGRIND='$(VALGRIND)'; export VALGRIND;
+endif
PY_LOG_COMPILER = $(PYTHON)
TEST_EXTENSIONS = .py
TESTS = libsamsung-ipc-test.py
diff --git a/samsung-ipc/tests/libsamsung-ipc-test.py b/samsung-ipc/tests/libsamsung-ipc-test.py
index 05b64ee..527c908 100755
--- a/samsung-ipc/tests/libsamsung-ipc-test.py
+++ b/samsung-ipc/tests/libsamsung-ipc-test.py
@@ -35,7 +35,14 @@ class libsamsung_ipc_test(object):
+ os.sep \
+ 'libsamsung-ipc-test'
- self.run = sh.Command(command_path)
+ if 'VALGRIND' in os.environ:
+ self.run = sh.Command(os.environ['VALGRIND'])
+ self.run = self.run.bake('-v',
+ '--log-file=valgrind.%p.log',
+ '--leak-check=full',
+ command_path)
+ else:
+ self.run = sh.Command(command_path)
def run_all_tests(self):
output = str(self.run('list-tests')).split(os.linesep)
diff --git a/scripts/PKGBUILD b/scripts/PKGBUILD
index 6f70c19..7deae5e 100644
--- a/scripts/PKGBUILD
+++ b/scripts/PKGBUILD
@@ -20,7 +20,8 @@ makedepends=('autoconf'
'pkg-config'
'python'
'python-sh'
- 'sed')
+ 'sed'
+ 'valgrind')
depends=('openssl')
pkgver() {
@@ -39,7 +40,9 @@ pkgver() {
build() {
../../autogen.sh
- ../../configure --prefix=/usr --enable-debug
+ ../../configure \
+ --prefix=/usr \
+ --enable-debug --enable-valgrind-tests
make
}
diff --git a/scripts/guix.scm b/scripts/guix.scm
index b723b7a..b8e5bef 100644
--- a/scripts/guix.scm
+++ b/scripts/guix.scm
@@ -53,13 +53,15 @@
(guix packages)
(gnu packages android)
(gnu packages autotools)
+ (gnu packages commencement)
(gnu packages disk)
(gnu packages linux)
(gnu packages llvm)
(gnu packages pkg-config)
(gnu packages python)
(gnu packages python-xyz)
- (gnu packages tls))
+ (gnu packages tls)
+ (gnu packages valgrind))
(define %common-strict-cflags
(string-append
@@ -103,10 +105,12 @@
`(("autoreconf" ,autoconf)
("aclocal" ,automake)
("ddrescue", ddrescue)
+ ("libc:debug", (@@ (gnu packages commencement) glibc-final) "debug")
("libtool" ,libtool)
("pkgconfig" ,pkg-config)
("python" ,python)
- ("python-sh" ,python-sh)))
+ ("python-sh" ,python-sh)
+ ("valgrind" ,valgrind)))
(inputs
`(("openssl" ,openssl)))
(arguments
@@ -116,9 +120,19 @@
(lambda _
(substitute* (find-files "." ".*\\.py$")
(("/usr/bin/env python") (which "python3")))
- #t)))
+ #t))
+ (add-after 'patch-python 'fix-valgrind
+ (lambda _
+ (substitute* (find-files "." ".*\\.py$")
+ (("'--leak-check=full',")
+ (string-append
+ "'--leak-check=full', '--extra-debuginfo-path="
+ (assoc-ref %build-inputs "libc:debug")
+ "/lib/debug',")))
+ #t)))
#:configure-flags
- (list "--enable-debug")))
+ (list "--enable-debug"
+ "--enable-valgrind-tests")))
(synopsis "libsamsung-ipc is a free software implementation of the Samsung IPC modem protocol")
(description
"libsamsung-ipc is a free software implementation of the Samsung IPC modem protocol,
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 7f51630..c1e8cc3 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -13,6 +13,9 @@ bin_PROGRAMS = \
# TODO: Find a way to make test more modular and represent each run of the
# nv_data-imei in TEST while having it implemented in a single python file
+if WANT_VALGRIND_CHECKING
+AM_TESTS_ENVIRONMENT = VALGRIND='$(VALGRIND)'; export VALGRIND;
+endif
PY_LOG_COMPILER = $(PYTHON)
TEST_EXTENSIONS = .py
TESTS = tests/ipc-modem.py \
diff --git a/tools/tests/ipc-modem.py b/tools/tests/ipc-modem.py
index af996ac..25331ae 100755
--- a/tools/tests/ipc-modem.py
+++ b/tools/tests/ipc-modem.py
@@ -49,8 +49,18 @@ class IpcModem(object):
+ os.sep \
+ 'ipc-modem'
- ipc_modem = sh.Command(command_path)
- self.ipc_modem = ipc_modem.bake('--dry-run')
+ if 'VALGRIND' in os.environ:
+ self.timeout = 60
+ ipc_modem = sh.Command(os.environ['VALGRIND'])
+ self.ipc_modem = ipc_modem.bake('-v',
+ '--log-file=valgrind.%p.log',
+ '--leak-check=full',
+ command_path,
+ '--dry-run')
+ else:
+ self.timeout = 3
+ ipc_modem = sh.Command(command_path)
+ self.ipc_modem = ipc_modem.bake('--dry-run')
def test_help(self):
try:
@@ -60,16 +70,28 @@ class IpcModem(object):
else:
raise Exception()
- def test_boot(self, timeout=2):
+ def test_boot(self, timeout=None):
+ if timeout==None:
+ timeout = self.timeout
+
self.ipc_modem('boot', _timeout=timeout)
- def test_power_on(self, timeout=2):
+ def test_power_on(self, timeout=None):
+ if timeout==None:
+ timeout = self.timeout
+
self.ipc_modem('power-on', _timeout=timeout)
- def test_power_off(self, timeout=2):
+ def test_power_off(self, timeout=None):
+ if timeout==None:
+ timeout = self.timeout
+
self.ipc_modem('power-off', _timeout=timeout)
- def test_start(self, timeout=3):
+ def test_start(self, timeout=None):
+ if timeout==None:
+ timeout = self.timeout
+
try:
self.ipc_modem('start', _timeout=timeout)
except sh.TimeoutException:
@@ -77,7 +99,10 @@ class IpcModem(object):
else:
raise Exception()
- def test_call_with_number(self, timeout=3):
+ def test_call_with_number(self, timeout=None):
+ if timeout==None:
+ timeout = self.timeout
+
expected_output = "[I] Got call number!"
output = ""
try:
@@ -90,7 +115,10 @@ class IpcModem(object):
if output != expected_output:
raise Exception()
- def test_call_without_number(self, timeout=3):
+ def test_call_without_number(self, timeout=None):
+ if timeout==None:
+ timeout = self.timeout
+
expected_output = "[E] Missing call number"
output = get_output(self.ipc_modem('power-on',
'--call=',
diff --git a/tools/tests/nv_data-imei.py b/tools/tests/nv_data-imei.py
index cec4e7f..79dddc8 100755
--- a/tools/tests/nv_data-imei.py
+++ b/tools/tests/nv_data-imei.py
@@ -58,7 +58,15 @@ class NvDataImei(object):
+ os.sep \
+ 'nv_data-imei'
- self.nv_data_imei = sh.Command(command_path)
+ if 'VALGRIND' in os.environ:
+ self.nv_data_imei = sh.Command(os.environ['VALGRIND'])
+ self.nv_data_imei = self.nv_data_imei.bake(
+ '-v',
+ '--log-file=valgrind.%p.log',
+ '--leak-check=full',
+ command_path)
+ else:
+ self.nv_data_imei = sh.Command(command_path)
def test_help(self):
try:
diff --git a/tools/tests/nv_data-md5.py b/tools/tests/nv_data-md5.py
index 315daee..91c37f6 100755
--- a/tools/tests/nv_data-md5.py
+++ b/tools/tests/nv_data-md5.py
@@ -44,7 +44,15 @@ class NvDataMD5(object):
+ os.sep \
+ 'nv_data-md5'
- self.nv_data_md5 = sh.Command(command_path)
+ if 'VALGRIND' in os.environ:
+ self.nv_data_md5 = sh.Command(os.environ['VALGRIND'])
+ self.nv_data_md5 = self.nv_data_md5.bake(
+ '-v',
+ '--log-file=valgrind.%p.log',
+ '--leak-check=full',
+ command_path)
+ else:
+ self.nv_data_md5 = sh.Command(command_path)
def test_help(self):
try:
self.nv_data_md5()
--
2.36.0
More information about the Replicant
mailing list