On 09/27/14 03:53, Bernd Edlinger wrote:
Comment before this change. Someone not familiar with this code is
going to have no idea why these two lines exist.
Ok, I added a comment now, do you like it?
Yes.
Please try to include a testcase. If you're having trouble reproducing
on the trunk, you could use MALLOC_PERTURB per c#8 in the bug report.
If there's a way to set environment variables in our testing framework
that may be a reasonable way to test (if you need to do that, limit
testing to linux targets as we'll have a dependency on glibc features).
For whatever reason, the first -include must end with a pragma
as in the PR, and MALLOC_PERTURB_ must be set to something.
Then we get an ICE, otherwise we get an error message without line number.
I tried to make this a valid test case, but that might be less trivial than
it looks at first sight.
I tried to set MALLOC_PERTURB_=123 globally, like this:
MALLOC_PERTURB_=123 make -k check
but then this happened:
Sigh. Yea, I guess if we're hitting the allocator insanely hard,
scrubbing memory might turn out to slow things down in a significant
way. Or it may simply be the case that we're using free'd memory in
some way and with the MALLOC_PERTURB changes we're in an infinite loop
in the dumping code or something similar.
Well, I added a test case, but it does not reliably fail without the
patch, because setting
MALLOC_PERTURB_ causes too much trouble at this time.
I would propose to set MALLOC_PERTURB_ globally at a later time.
Sorry, just to be clear, I wasn't suggesting to set it globally, but
just for the duration of this test as a potentially easier way to
trigger the failure.
However, it may make sense to do that at some point. I also think that
Jakub bootstraps and runs the regression suite with valgrind late in the
release cycle, which would catch this problem if it raises its head again.
Boot-Strapped & Regression-Tested on x86_64-linux-gnu.
Ok for trunk?
Yes, this is OK for the trunk.
jeff
