| Message ID | 1368780720-14852-1-git-send-email-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
Stefan Hajnoczi <stefanha@redhat.com> writes: > glibc wipes malloc(3) memory when the MALLOC_PERTURB_ environment > variable is set. The value of the environment variable determines the > bit pattern used to wipe memory. For more information, see > http://udrepper.livejournal.com/11429.html. > > Set MALLOC_PERTURB_ for gtester and qemu-iotests. Note we always set > the environment variable to 1 so the test is deterministic. Setting a > random variable might expose more bugs but would be harder to reproduce. > > Both make check and qemu-iotests pass with MALLOC_PERTURB_ enabled. > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > Lucas noticed KVM autotest failures when enabling MALLOC_PERTURB_. By enabling > it for in-tree test suites we can detect memory management errors earlier. > > tests/Makefile | 4 +++- > tests/qemu-iotests/check | 2 +- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/tests/Makefile b/tests/Makefile > index a307d5a..25f6d28 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -171,6 +171,7 @@ GCOV_OPTIONS = -n $(if $(V),-f,) > $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y) > $(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,) > $(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \ > + MALLOC_PERTURB_=1 \ > gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y),"GTESTER $@") > $(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y); do \ > echo Gcov report for $$f:;\ If you want punishment, why not go for extra punishment? MALLOC_PERTURB_=$(($RANDOM % 255 + 1)) [...]
On Fri, May 17, 2013 at 11:54:12AM +0200, Markus Armbruster wrote: > Stefan Hajnoczi <stefanha@redhat.com> writes: > > > glibc wipes malloc(3) memory when the MALLOC_PERTURB_ environment > > variable is set. The value of the environment variable determines the > > bit pattern used to wipe memory. For more information, see > > http://udrepper.livejournal.com/11429.html. > > > > Set MALLOC_PERTURB_ for gtester and qemu-iotests. Note we always set > > the environment variable to 1 so the test is deterministic. Setting a > > random variable might expose more bugs but would be harder to reproduce. > > > > Both make check and qemu-iotests pass with MALLOC_PERTURB_ enabled. > > > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > > --- > > Lucas noticed KVM autotest failures when enabling MALLOC_PERTURB_. By enabling > > it for in-tree test suites we can detect memory management errors earlier. > > > > tests/Makefile | 4 +++- > > tests/qemu-iotests/check | 2 +- > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/tests/Makefile b/tests/Makefile > > index a307d5a..25f6d28 100644 > > --- a/tests/Makefile > > +++ b/tests/Makefile > > @@ -171,6 +171,7 @@ GCOV_OPTIONS = -n $(if $(V),-f,) > > $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y) > > $(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,) > > $(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \ > > + MALLOC_PERTURB_=1 \ > > gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y),"GTESTER $@") > > $(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y); do \ > > echo Gcov report for $$f:;\ > > If you want punishment, why not go for extra punishment? > > MALLOC_PERTURB_=$(($RANDOM % 255 + 1)) That could lead to non-reproducable failures though. I think it is better to use a fixed value so that you're more likely to be able to reproduce the issue every time you run the tests. Rather than setting MALLOC_PERTURB_=1 unconditionally in the Makefile though, it ought to honour any existing MALLOC_PERTURB_ env variable the user has set. That could let automated test harness run repeatedly with random MALLOC_PERTURB_, while still giving a deterministic value for developers by default. Daniel
"Daniel P. Berrange" <berrange@redhat.com> writes: > On Fri, May 17, 2013 at 11:54:12AM +0200, Markus Armbruster wrote: >> Stefan Hajnoczi <stefanha@redhat.com> writes: >> >> > glibc wipes malloc(3) memory when the MALLOC_PERTURB_ environment >> > variable is set. The value of the environment variable determines the >> > bit pattern used to wipe memory. For more information, see >> > http://udrepper.livejournal.com/11429.html. >> > >> > Set MALLOC_PERTURB_ for gtester and qemu-iotests. Note we always set >> > the environment variable to 1 so the test is deterministic. Setting a >> > random variable might expose more bugs but would be harder to reproduce. >> > >> > Both make check and qemu-iotests pass with MALLOC_PERTURB_ enabled. >> > >> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> >> > --- >> > Lucas noticed KVM autotest failures when enabling MALLOC_PERTURB_. By enabling >> > it for in-tree test suites we can detect memory management errors earlier. >> > >> > tests/Makefile | 4 +++- >> > tests/qemu-iotests/check | 2 +- >> > 2 files changed, 4 insertions(+), 2 deletions(-) >> > >> > diff --git a/tests/Makefile b/tests/Makefile >> > index a307d5a..25f6d28 100644 >> > --- a/tests/Makefile >> > +++ b/tests/Makefile >> > @@ -171,6 +171,7 @@ GCOV_OPTIONS = -n $(if $(V),-f,) >> > $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y) >> > $(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,) >> > $(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \ >> > + MALLOC_PERTURB_=1 \ >> > gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y),"GTESTER $@") >> > $(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y); do \ >> > echo Gcov report for $$f:;\ >> >> If you want punishment, why not go for extra punishment? >> >> MALLOC_PERTURB_=$(($RANDOM % 255 + 1)) > > That could lead to non-reproducable failures though. I think it is better > to use a fixed value so that you're more likely to be able to reproduce > the issue every time you run the tests. A fixed value misses failures. A random value is just as reproducible as a fixed one, simply use the same value. Requires the value to be logged, of course. > Rather than setting MALLOC_PERTURB_=1 unconditionally in the Makefile > though, it ought to honour any existing MALLOC_PERTURB_ env variable > the user has set. That could let automated test harness run repeatedly > with random MALLOC_PERTURB_, while still giving a deterministic value > for developers by default. Yes, we should respect the user's MALLOC_PERTURB_.
On Fri, May 17, 2013 at 11:54 AM, Markus Armbruster <armbru@redhat.com> wrote: > Stefan Hajnoczi <stefanha@redhat.com> writes: > >> glibc wipes malloc(3) memory when the MALLOC_PERTURB_ environment >> variable is set. The value of the environment variable determines the >> bit pattern used to wipe memory. For more information, see >> http://udrepper.livejournal.com/11429.html. >> >> Set MALLOC_PERTURB_ for gtester and qemu-iotests. Note we always set >> the environment variable to 1 so the test is deterministic. Setting a >> random variable might expose more bugs but would be harder to reproduce. >> >> Both make check and qemu-iotests pass with MALLOC_PERTURB_ enabled. >> >> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> >> --- >> Lucas noticed KVM autotest failures when enabling MALLOC_PERTURB_. By enabling >> it for in-tree test suites we can detect memory management errors earlier. >> >> tests/Makefile | 4 +++- >> tests/qemu-iotests/check | 2 +- >> 2 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/tests/Makefile b/tests/Makefile >> index a307d5a..25f6d28 100644 >> --- a/tests/Makefile >> +++ b/tests/Makefile >> @@ -171,6 +171,7 @@ GCOV_OPTIONS = -n $(if $(V),-f,) >> $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y) >> $(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,) >> $(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \ >> + MALLOC_PERTURB_=1 \ >> gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y),"GTESTER $@") >> $(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y); do \ >> echo Gcov report for $$f:;\ > > If you want punishment, why not go for extra punishment? > > MALLOC_PERTURB_=$(($RANDOM % 255 + 1)) Covered in the commit description: "Note we always set the environment variable to 1 so the test is deterministic. Setting a random variable might expose more bugs but would be harder to reproduce." I didn't want to clutter output with "MALLOC_PERTURB_=123" on every run. AFAIK we have no log where this can be silently stashed. I guess we could write it to a file and add that to .gitignore. Stefan
On Fri, May 17, 2013 at 12:07 PM, Daniel P. Berrange <berrange@redhat.com> wrote: > Rather than setting MALLOC_PERTURB_=1 unconditionally in the Makefile > though, it ought to honour any existing MALLOC_PERTURB_ env variable > the user has set. That could let automated test harness run repeatedly > with random MALLOC_PERTURB_, while still giving a deterministic value > for developers by default. Good point. Will fix in v2. Stefan
On 17/05/13 07:07 AM, Daniel P. Berrange wrote: >> If you want punishment, why not go for extra punishment? >> >> MALLOC_PERTURB_=$(($RANDOM % 255 + 1)) > > That could lead to non-reproducable failures though. I think it is better > to use a fixed value so that you're more likely to be able to reproduce > the issue every time you run the tests. > > Rather than setting MALLOC_PERTURB_=1 unconditionally in the Makefile > though, it ought to honour any existing MALLOC_PERTURB_ env variable > the user has set. That could let automated test harness run repeatedly > with random MALLOC_PERTURB_, while still giving a deterministic value > for developers by default. Indeed. I've never thought about it, thanks for sharing the insight.
On 17/05/13 08:15 AM, Stefan Hajnoczi wrote: > On Fri, May 17, 2013 at 11:54 AM, Markus Armbruster <armbru@redhat.com> wrote: >> Stefan Hajnoczi <stefanha@redhat.com> writes: >> >>> glibc wipes malloc(3) memory when the MALLOC_PERTURB_ environment >>> variable is set. The value of the environment variable determines the >>> bit pattern used to wipe memory. For more information, see >>> http://udrepper.livejournal.com/11429.html. >>> >>> Set MALLOC_PERTURB_ for gtester and qemu-iotests. Note we always set >>> the environment variable to 1 so the test is deterministic. Setting a >>> random variable might expose more bugs but would be harder to reproduce. >>> >>> Both make check and qemu-iotests pass with MALLOC_PERTURB_ enabled. >>> >>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> >>> --- >>> Lucas noticed KVM autotest failures when enabling MALLOC_PERTURB_. By enabling >>> it for in-tree test suites we can detect memory management errors earlier. >>> >>> tests/Makefile | 4 +++- >>> tests/qemu-iotests/check | 2 +- >>> 2 files changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/tests/Makefile b/tests/Makefile >>> index a307d5a..25f6d28 100644 >>> --- a/tests/Makefile >>> +++ b/tests/Makefile >>> @@ -171,6 +171,7 @@ GCOV_OPTIONS = -n $(if $(V),-f,) >>> $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y) >>> $(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,) >>> $(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \ >>> + MALLOC_PERTURB_=1 \ >>> gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y),"GTESTER $@") >>> $(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y); do \ >>> echo Gcov report for $$f:;\ >> >> If you want punishment, why not go for extra punishment? >> >> MALLOC_PERTURB_=$(($RANDOM % 255 + 1)) > > Covered in the commit description: > "Note we always set > the environment variable to 1 so the test is deterministic. Setting a > random variable might expose more bugs but would be harder to reproduce." > > I didn't want to clutter output with "MALLOC_PERTURB_=123" on every > run. AFAIK we have no log where this can be silently stashed. I > guess we could write it to a file and add that to .gitignore. Yes, you could consider to have a debug log or something similar.
On 17/05/13 07:58 AM, Markus Armbruster wrote: > "Daniel P. Berrange" <berrange@redhat.com> writes: > >> On Fri, May 17, 2013 at 11:54:12AM +0200, Markus Armbruster wrote: >>> Stefan Hajnoczi <stefanha@redhat.com> writes: >>> >>>> glibc wipes malloc(3) memory when the MALLOC_PERTURB_ environment >>>> variable is set. The value of the environment variable determines the >>>> bit pattern used to wipe memory. For more information, see >>>> http://udrepper.livejournal.com/11429.html. >>>> >>>> Set MALLOC_PERTURB_ for gtester and qemu-iotests. Note we always set >>>> the environment variable to 1 so the test is deterministic. Setting a >>>> random variable might expose more bugs but would be harder to reproduce. >>>> >>>> Both make check and qemu-iotests pass with MALLOC_PERTURB_ enabled. >>>> >>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> >>>> --- >>>> Lucas noticed KVM autotest failures when enabling MALLOC_PERTURB_. By enabling >>>> it for in-tree test suites we can detect memory management errors earlier. >>>> >>>> tests/Makefile | 4 +++- >>>> tests/qemu-iotests/check | 2 +- >>>> 2 files changed, 4 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/tests/Makefile b/tests/Makefile >>>> index a307d5a..25f6d28 100644 >>>> --- a/tests/Makefile >>>> +++ b/tests/Makefile >>>> @@ -171,6 +171,7 @@ GCOV_OPTIONS = -n $(if $(V),-f,) >>>> $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y) >>>> $(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,) >>>> $(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \ >>>> + MALLOC_PERTURB_=1 \ >>>> gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y),"GTESTER $@") >>>> $(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y); do \ >>>> echo Gcov report for $$f:;\ >>> >>> If you want punishment, why not go for extra punishment? >>> >>> MALLOC_PERTURB_=$(($RANDOM % 255 + 1)) >> >> That could lead to non-reproducable failures though. I think it is better >> to use a fixed value so that you're more likely to be able to reproduce >> the issue every time you run the tests. > > A fixed value misses failures. > > A random value is just as reproducible as a fixed one, simply use the > same value. Requires the value to be logged, of course. That's a pretty great idea, Markus. I'm going to implement it in virt-test, thanks!
On Fri, May 17, 2013 at 11:07:38AM +0100, Daniel P. Berrange wrote: > On Fri, May 17, 2013 at 11:54:12AM +0200, Markus Armbruster wrote: > > If you want punishment, why not go for extra punishment? > > > > MALLOC_PERTURB_=$(($RANDOM % 255 + 1)) > > That could lead to non-reproducable failures though. I think it is better > to use a fixed value so that you're more likely to be able to reproduce > the issue every time you run the tests. We've tested libguestfs for years with: random_val="$(awk 'BEGIN{srand(); print 1+int(255*rand())}' < /dev/null)" export MALLOC_PERTURB_=$random_val and have never seen a non-reproducable failure caused by this. It has, however, caught some simple memory failures, although valgrind is a lot better if you can afford to run it. Rich.
diff --git a/tests/Makefile b/tests/Makefile index a307d5a..25f6d28 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -171,6 +171,7 @@ GCOV_OPTIONS = -n $(if $(V),-f,) $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y) $(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,) $(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \ + MALLOC_PERTURB_=1 \ gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y),"GTESTER $@") $(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y); do \ echo Gcov report for $$f:;\ @@ -180,7 +181,8 @@ $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y) .PHONY: $(patsubst %, check-%, $(check-unit-y)) $(patsubst %, check-%, $(check-unit-y)): check-%: % $(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,) - $(call quiet-command,gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER $*") + $(call quiet-command,MALLOC_PERTURB_=1 \ + gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER $*") $(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y); do \ echo Gcov report for $$f:;\ $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \ diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check index 432732c..1527cda 100755 --- a/tests/qemu-iotests/check +++ b/tests/qemu-iotests/check @@ -214,7 +214,7 @@ do start=`_wallclock` $timestamp && echo -n " ["`date "+%T"`"]" [ ! -x $seq ] && chmod u+x $seq # ensure we can run it - ./$seq >$tmp.out 2>&1 + MALLOC_PERTURB_=1 ./$seq >$tmp.out 2>&1 sts=$? $timestamp && _timestamp stop=`_wallclock`
glibc wipes malloc(3) memory when the MALLOC_PERTURB_ environment variable is set. The value of the environment variable determines the bit pattern used to wipe memory. For more information, see http://udrepper.livejournal.com/11429.html. Set MALLOC_PERTURB_ for gtester and qemu-iotests. Note we always set the environment variable to 1 so the test is deterministic. Setting a random variable might expose more bugs but would be harder to reproduce. Both make check and qemu-iotests pass with MALLOC_PERTURB_ enabled. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- Lucas noticed KVM autotest failures when enabling MALLOC_PERTURB_. By enabling it for in-tree test suites we can detect memory management errors earlier. tests/Makefile | 4 +++- tests/qemu-iotests/check | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-)