| Message ID | 20160623144042.E649F41C390E5@oldenburg.str.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Jun 23, 2016 at 04:40:42PM +0200, Florian Weimer wrote: > Over all, this decreases the realism of the tests because > it ensures that freshly allocated memory has a well-defined > bit pattern. It also causes malloc to take internal paths > different from regular application usage, and therefore > reduces malloc test coverage. The well-defined bit pattern is more likely to catch any bad tests though, which might make it valuable. However, I agree that it makes sense to remove it for malloc tests. Siddhesh
On 06/23/2016 04:49 PM, Siddhesh Poyarekar wrote: > On Thu, Jun 23, 2016 at 04:40:42PM +0200, Florian Weimer wrote: >> Over all, this decreases the realism of the tests because >> it ensures that freshly allocated memory has a well-defined >> bit pattern. It also causes malloc to take internal paths >> different from regular application usage, and therefore >> reduces malloc test coverage. > > The well-defined bit pattern is more likely to catch any bad tests > though, which might make it valuable. It could also cover up bugs which would otherwise be visible with fresh allocations which contain only zeros. Ideally, we'd run all tests twice, with different settings from the environment (and also with valgrind). Thanks, Florian
On Thu, Jun 23, 2016 at 05:05:33PM +0200, Florian Weimer wrote: > It could also cover up bugs which would otherwise be visible with fresh > allocations which contain only zeros. > > Ideally, we'd run all tests twice, with different settings from the > environment (and also with valgrind). Yeah, if you were to do that kind of coverage at the distribution level (i.e. run the tests twice, once with and then without valgrind) that the change makes sense to make the tests behave as closely as possible to the real world. In short, LGTM. Siddhesh
On Thu, Jun 23, 2016 at 12:00 PM, Siddhesh Poyarekar <siddhesh@sourceware.org> wrote: > On Thu, Jun 23, 2016 at 05:05:33PM +0200, Florian Weimer wrote: >> It could also cover up bugs which would otherwise be visible with fresh >> allocations which contain only zeros. >> >> Ideally, we'd run all tests twice, with different settings from the >> environment (and also with valgrind). > > Yeah, if you were to do that kind of coverage at the distribution > level (i.e. run the tests twice, once with and then without valgrind) > that the change makes sense to make the tests behave as closely as > possible to the real world. Is there a well-documented way to run the tests under valgrind?
On Thu, Jun 23, 2016 at 12:57:17PM -0400, Zack Weinberg wrote:
> Is there a well-documented way to run the tests under valgrind?
I don't think so, but it ought to be sufficient to run valgrind
--trace-children=yes for all of make check. Dead slow, but I don't
see why it shouldn't work.
Maybe this is a good candidate for enhancement in glibc to allow
running individual tests under valgrind, something like
'make valgrind-check'.
Siddhesh
On 06/23/2016 07:16 PM, Siddhesh Poyarekar wrote: > On Thu, Jun 23, 2016 at 12:57:17PM -0400, Zack Weinberg wrote: >> Is there a well-documented way to run the tests under valgrind? > > I don't think so, but it ought to be sufficient to run valgrind > --trace-children=yes for all of make check. Dead slow, but I don't > see why it shouldn't work. > > Maybe this is a good candidate for enhancement in glibc to allow > running individual tests under valgrind, something like > 'make valgrind-check'. Running tests under valgrind is one thing. Interpreting the errors is another. On some architectures, running valgrind on a non-installed libc gives rather poor results. Some versions do not recognize malloc as such, and on some architectures, the suppression files are inactive because the libc paths do not match the hard-coded expectations of valgrind. With certain tests, we have the problem that we have memory leaks when subprocesses call exit, without freeing data, which is marked as a leak in the error log. Thread stack deallocation is generally racy, and stacks may be reported by valgrind as a memory leak. These are just the issues I have seen so far when running valgrind manually. I expect there are more. Thanks, Florian
On 23 Jun 2016 19:25, Florian Weimer wrote: > On 06/23/2016 07:16 PM, Siddhesh Poyarekar wrote: > > On Thu, Jun 23, 2016 at 12:57:17PM -0400, Zack Weinberg wrote: > >> Is there a well-documented way to run the tests under valgrind? > > > > I don't think so, but it ought to be sufficient to run valgrind > > --trace-children=yes for all of make check. Dead slow, but I don't > > see why it shouldn't work. > > > > Maybe this is a good candidate for enhancement in glibc to allow > > running individual tests under valgrind, something like > > 'make valgrind-check'. > > Running tests under valgrind is one thing. Interpreting the errors is > another. > > On some architectures, running valgrind on a non-installed libc gives > rather poor results. Some versions do not recognize malloc as such, and > on some architectures, the suppression files are inactive because the > libc paths do not match the hard-coded expectations of valgrind. not only that, but valgrind frequently needs to be updated whenever a new glibc release is made (to tweak internal false positives). if we tried to use that against git master, i'm afraid it'd always be broken. -mike
On 23 Jun 2016 17:05, Florian Weimer wrote: > On 06/23/2016 04:49 PM, Siddhesh Poyarekar wrote: > > On Thu, Jun 23, 2016 at 04:40:42PM +0200, Florian Weimer wrote: > >> Over all, this decreases the realism of the tests because > >> it ensures that freshly allocated memory has a well-defined > >> bit pattern. It also causes malloc to take internal paths > >> different from regular application usage, and therefore > >> reduces malloc test coverage. > > > > The well-defined bit pattern is more likely to catch any bad tests > > though, which might make it valuable. > > It could also cover up bugs which would otherwise be visible with fresh > allocations which contain only zeros. while certainly true, i think this is much less common of an edge case. code that happens to work because it happened to get zero-ed memory is, in my experience, way less common than code that happens to work because it happened to get garbage initially. -mike
On 06/23/2016 09:31 PM, Mike Frysinger wrote: > On 23 Jun 2016 17:05, Florian Weimer wrote: >> On 06/23/2016 04:49 PM, Siddhesh Poyarekar wrote: >>> On Thu, Jun 23, 2016 at 04:40:42PM +0200, Florian Weimer wrote: >>>> Over all, this decreases the realism of the tests because >>>> it ensures that freshly allocated memory has a well-defined >>>> bit pattern. It also causes malloc to take internal paths >>>> different from regular application usage, and therefore >>>> reduces malloc test coverage. >>> >>> The well-defined bit pattern is more likely to catch any bad tests >>> though, which might make it valuable. >> >> It could also cover up bugs which would otherwise be visible with fresh >> allocations which contain only zeros. > > while certainly true, i think this is much less common of an edge case. > code that happens to work because it happened to get zero-ed memory is, > in my experience, way less common than code that happens to work because > it happened to get garbage initially. Okay, patch withdrawn. Thanks, Florian
diff --git a/test-skeleton.c b/test-skeleton.c index d9bf989..e462eee 100644 --- a/test-skeleton.c +++ b/test-skeleton.c @@ -346,9 +346,6 @@ main (int argc, char *argv[]) unsigned int timeoutfactor = 1; pid_t termpid; - /* Make uses of freed and uninitialized memory known. */ - mallopt (M_PERTURB, 42); - #ifdef STDOUT_UNBUFFERED setbuf (stdout, NULL); #endif