[pacman-dev] [PATCH 1/5] valgrind.supp: add fakeroot/fakechroot errors

older
[pacman-dev] [PATCH] Give groups...

Andrew Gregory

6 Jan 2014 6 Jan '14

1:19 p.m.

Signed-off-by: Andrew Gregory --- Patch submitted upstream for the __lxstat error. valgrind.supp | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/valgrind.supp b/valgrind.supp index 55effa9..ac54b74 100644 --- a/valgrind.supp +++ b/valgrind.supp @@ -109,3 +109,43 @@ ... fun:regexec } +{ + fakeroot-msgsnd + Memcheck:Param + msgsnd(msgp->mtext) + fun:__msgsnd_nocancel +} +{ + fakechroot-add-to-environ + Memcheck:Leak + fun:malloc + fun:__add_to_environ +} +{ + fakechroot-static-fakechroot-init-malloc + Memcheck:Leak + fun:malloc + ... + fun:fakechroot_init +} +{ + fakechroot-static-fakechroot-init-calloc + Memcheck:Leak + fun:calloc + ... + fun:fakechroot_init +} +{ + fakechroot-static-bindtextdomain + Memcheck:Leak + match-leak-kinds: reachable + fun:malloc + obj:/usr/lib/libfakeroot/fakechroot/libfakechroot.so + obj:/usr/lib/libfakeroot/fakechroot/libfakechroot.so + fun:bindtextdomain +} +{ + fakechroot-uninitialized-var-lxstat + Memcheck:Cond + fun:__lxstat +} -- 1.8.5.2

Show replies by thread

Andrew Gregory

6 Jan 6 Jan

1:19 p.m.

New subject: [pacman-dev] [PATCH 2/5] pmtest: send valgrind output to var/log/valgrind

This will allow us to detect whether valgrind found any errors while still preserving pacman's return code for tests. Signed-off-by: Andrew Gregory --- test/pacman/pmtest.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py index 2c50f2a..e5094a1 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -220,6 +220,7 @@ def run(self, pacman): cmd.extend(["libtool", "execute", "valgrind", "-q", "--tool=memcheck", "--leak-check=full", "--show-reachable=yes", + "--log-file=%s" % os.path.join(self.root, "var/log/valgrind"), "--suppressions=%s" % suppfile]) cmd.extend([pacman["bin"], "--config", os.path.join(self.root, util.PACCONF), @@ -231,7 +232,7 @@ def run(self, pacman): if pacman["debug"]: cmd.append("--debug=%s" % pacman["debug"]) cmd.extend(shlex.split(self.args)) - if not (pacman["gdb"] or pacman["valgrind"] or pacman["nolog"]): + if not (pacman["gdb"] or pacman["nolog"]): output = open(os.path.join(self.root, util.LOGFILE), 'w') else: output = None -- 1.8.5.2

Andrew Gregory

1:19 p.m.

New subject: [pacman-dev] [PATCH 3/5] pmtest: use --gen-suppression=all with valgrind

Let valgrind do the work of writing any suppression rules needed by the test suite. Signed-off-by: Andrew Gregory --- test/pacman/pmtest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py index e5094a1..1c3ea7c 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -220,6 +220,7 @@ def run(self, pacman): cmd.extend(["libtool", "execute", "valgrind", "-q", "--tool=memcheck", "--leak-check=full", "--show-reachable=yes", + "--gen-suppressions=all", "--log-file=%s" % os.path.join(self.root, "var/log/valgrind"), "--suppressions=%s" % suppfile]) cmd.extend([pacman["bin"], -- 1.8.5.2

Andrew Gregory

1:19 p.m.

New subject: [pacman-dev] [PATCH 4/5] pmtest: use valgrind --child-silent-after-fork

Silences false warnings after alpm forks to run install scripts. Signed-off-by: Andrew Gregory --- test/pacman/pmtest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py index 1c3ea7c..e780a6d 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -221,6 +221,7 @@ def run(self, pacman): "--tool=memcheck", "--leak-check=full", "--show-reachable=yes", "--gen-suppressions=all", + "--child-silent-after-fork=yes", "--log-file=%s" % os.path.join(self.root, "var/log/valgrind"), "--suppressions=%s" % suppfile]) cmd.extend([pacman["bin"], -- 1.8.5.2

Andrew Gregory

1:19 p.m.

New subject: [pacman-dev] [PATCH 5/5] pmtest: add memory check to tests with valgrind

Signed-off-by: Andrew Gregory --- test/pacman/pmrule.py | 3 +++ test/pacman/pmtest.py | 1 + 2 files changed, 4 insertions(+) diff --git a/test/pacman/pmrule.py b/test/pacman/pmrule.py index ba94ab8..96ecdec 100644 --- a/test/pacman/pmrule.py +++ b/test/pacman/pmrule.py @@ -112,6 +112,9 @@ def check(self, test): if case == "EXIST": if not os.path.isfile(filename): success = 0 + elif case == "EMPTY": + if not os.path.getsize(filename) == 0: + success = 0 elif case == "MODIFIED": for f in test.files: if f.name == key: diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py index e780a6d..7079b78 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -224,6 +224,7 @@ def run(self, pacman): "--child-silent-after-fork=yes", "--log-file=%s" % os.path.join(self.root, "var/log/valgrind"), "--suppressions=%s" % suppfile]) + self.addrule("FILE_EMPTY=var/log/valgrind") cmd.extend([pacman["bin"], "--config", os.path.join(self.root, util.PACCONF), "--root", self.root, -- 1.8.5.2

Dan McGee

26 Jan 26 Jan

1:06 p.m.

New subject: [pacman-dev] [PATCH 3/5] pmtest: use --gen-suppression=all with valgrind

On Mon, Jan 6, 2014 at 2:19 PM, Andrew Gregory wrote:

...

Let valgrind do the work of writing any suppression rules needed by the test suite.

Signed-off-by: Andrew Gregory ---

This makes sense. It just makes it a heck of a lot easier to get the possible suppression into the file, but it doesn't actually write it out for you. Signed-off-by: Dan McGee test/pacman/pmtest.py | 1 +

...

1 file changed, 1 insertion(+)

diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py index e5094a1..1c3ea7c 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -220,6 +220,7 @@ def run(self, pacman): cmd.extend(["libtool", "execute", "valgrind", "-q", "--tool=memcheck", "--leak-check=full", "--show-reachable=yes", + "--gen-suppressions=all", "--log-file=%s" % os.path.join(self.root, "var/log/valgrind"), "--suppressions=%s" % suppfile]) cmd.extend([pacman["bin"], -- 1.8.5.2

Dan McGee

1:08 p.m.

New subject: [pacman-dev] [PATCH 4/5] pmtest: use valgrind --child-silent-after-fork

On Mon, Jan 6, 2014 at 2:19 PM, Andrew Gregory wrote:

...

Silences false warnings after alpm forks to run install scripts.

Signed-off-by: Andrew Gregory ---

We didn't actually run install scripts in the test suite before if I remember right, so there was likely no need for this option before. Seems reasonable to add, especially given these forks are short-lived anyway and they can just clean themselves up.

...

test/pacman/pmtest.py | 1 + 1 file changed, 1 insertion(+)

diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py index 1c3ea7c..e780a6d 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -221,6 +221,7 @@ def run(self, pacman): "--tool=memcheck", "--leak-check=full", "--show-reachable=yes", "--gen-suppressions=all", + "--child-silent-after-fork=yes", "--log-file=%s" % os.path.join(self.root, "var/log/valgrind"), "--suppressions=%s" % suppfile]) cmd.extend([pacman["bin"], -- 1.8.5.2

Dan McGee

1:10 p.m.

New subject: [pacman-dev] [PATCH 5/5] pmtest: add memory check to tests with valgrind

On Mon, Jan 6, 2014 at 2:19 PM, Andrew Gregory wrote:

...

Signed-off-by: Andrew Gregory ---

Seems like this is two patches in one- one to add FILE_EMPTY, and one to use it. How does FILE_EMPTY behave in these cases? Namely the first one feels like a gray area that the code does not make super obvious. * File doesn't exist * File exists, is empty * File exists, is not empty * File is a directory Also, don't we document the available rules somewhere?

...

test/pacman/pmrule.py | 3 +++ test/pacman/pmtest.py | 1 + 2 files changed, 4 insertions(+)

diff --git a/test/pacman/pmrule.py b/test/pacman/pmrule.py index ba94ab8..96ecdec 100644 --- a/test/pacman/pmrule.py +++ b/test/pacman/pmrule.py @@ -112,6 +112,9 @@ def check(self, test): if case == "EXIST": if not os.path.isfile(filename): success = 0 + elif case == "EMPTY": + if not os.path.getsize(filename) == 0: + success = 0 elif case == "MODIFIED": for f in test.files: if f.name == key: diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py index e780a6d..7079b78 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -224,6 +224,7 @@ def run(self, pacman): "--child-silent-after-fork=yes", "--log-file=%s" % os.path.join(self.root, "var/log/valgrind"), "--suppressions=%s" % suppfile]) + self.addrule("FILE_EMPTY=var/log/valgrind") cmd.extend([pacman["bin"], "--config", os.path.join(self.root, util.PACCONF), "--root", self.root, -- 1.8.5.2

Andrew Gregory

3 Feb 3 Feb

10:05 a.m.

New subject: [pacman-dev] [PATCHv2 1/2] pmrule: add FILE_EMPTY rule

Succeeds if the specified path is a file and is empty. Signed-off-by: Andrew Gregory --- test/pacman/README | 1 + test/pacman/pmrule.py | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/test/pacman/README b/test/pacman/README index c7aeb10..2516a8a 100644 --- a/test/pacman/README +++ b/test/pacman/README @@ -299,6 +299,7 @@ its DEPENDS field. . FILE rules FILE_EXIST=path/to/file + FILE_EMPTY=path/to/file FILE_MODIFIED=path/to/file FILE_MODE=path/to/file|octal FILE_TYPE=path/to/file|type (possible types: dir, file, link) diff --git a/test/pacman/pmrule.py b/test/pacman/pmrule.py index ba94ab8..a91741b 100644 --- a/test/pacman/pmrule.py +++ b/test/pacman/pmrule.py @@ -112,6 +112,10 @@ def check(self, test): if case == "EXIST": if not os.path.isfile(filename): success = 0 + elif case == "EMPTY": + if not (os.path.isfile(filename) + and os.path.getsize(filename) == 0): + success = 0 elif case == "MODIFIED": for f in test.files: if f.name == key: -- 1.8.5.3

Andrew Gregory

10:05 a.m.

New subject: [pacman-dev] [PATCHv2 2/2] pmtest: add memory check to tests with valgrind

Signed-off-by: Andrew Gregory --- test/pacman/README | 5 +++++ test/pacman/pmtest.py | 1 + 2 files changed, 6 insertions(+) diff --git a/test/pacman/README b/test/pacman/README index 2516a8a..8d8354a 100644 --- a/test/pacman/README +++ b/test/pacman/README @@ -62,6 +62,11 @@ This example will run all tests from the "tests" directory. Use the "help" option to get the full list of parameters: ./pactest.py --help +When run with the `--valgrind' option, an additional rule will be added to all +tests to check for memory leaks. To use `--valgrind' when running the full +test suite, run: + make PY_LOG_FLAGS=--valgrind check + The following pieces of software are required to run the pactest suite: fakeroot (required to run as non-root user) diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py index e780a6d..7079b78 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -224,6 +224,7 @@ def run(self, pacman): "--child-silent-after-fork=yes", "--log-file=%s" % os.path.join(self.root, "var/log/valgrind"), "--suppressions=%s" % suppfile]) + self.addrule("FILE_EMPTY=var/log/valgrind") cmd.extend([pacman["bin"], "--config", os.path.join(self.root, util.PACCONF), "--root", self.root, -- 1.8.5.3

Allan McRae

8:57 p.m.

New subject: [pacman-dev] [PATCHv2 2/2] pmtest: add memory check to tests with valgrind

On 04/02/14 03:05, Andrew Gregory wrote:

...

Signed-off-by: Andrew Gregory --- test/pacman/README | 5 +++++ test/pacman/pmtest.py | 1 + 2 files changed, 6 insertions(+)

diff --git a/test/pacman/README b/test/pacman/README index 2516a8a..8d8354a 100644 --- a/test/pacman/README +++ b/test/pacman/README @@ -62,6 +62,11 @@ This example will run all tests from the "tests" directory. Use the "help" option to get the full list of parameters: ./pactest.py --help

+When run with the `--valgrind' option, an additional rule will be added to all +tests to check for memory leaks. To use `--valgrind' when running the full +test suite, run: + make PY_LOG_FLAGS=--valgrind check + The following pieces of software are required to run the pactest suite:

Awesome! Is this what you are seeing when running the tests with valgrind? # FAIL: 14 FAIL: test/pacman/tests/ignore002 FAIL: test/pacman/tests/ignore003 FAIL: test/pacman/tests/ignore004 FAIL: test/pacman/tests/provision010 FAIL: test/pacman/tests/provision012 FAIL: test/pacman/tests/provision020 FAIL: test/pacman/tests/provision022 FAIL: test/pacman/tests/sync-nodepversion02 FAIL: test/pacman/tests/sync-nodepversion04 FAIL: test/pacman/tests/sync1008 FAIL: test/pacman/tests/sync300 FAIL: test/pacman/tests/upgrade071 FAIL: test/pacman/tests/upgrade075 FAIL: test/pacman/tests/upgrade077 I note this is down from 64 after your export freeing functions patch! Allan

3731

Age (days ago)

3760

Last active (days ago)

List overview

Download

10 comments

3 participants

participants (3)

Allan McRae
Andrew Gregory
Dan McGee

[pacman-dev] [PATCH 1/5] valgrind.supp: add fakeroot/fakechroot errors

tags

participants (3)