Bug 160956 - PATCH: mallinfo implementation
Summary: PATCH: mallinfo implementation
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: memcheck (show other bugs)
Version: 3.3.0
Platform: Unlisted Binaries Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-04-17 22:42 UTC by Eugene Toder
Modified: 2008-05-12 00:41 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Mallinfo patch (1.76 KB, patch)
2008-04-17 22:43 UTC, Eugene Toder
Details
Test case (2.18 KB, text/plain)
2008-04-19 23:26 UTC, Eugene Toder
Details
Patch to pass the test (4.22 KB, patch)
2008-04-19 23:27 UTC, Eugene Toder
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Eugene Toder 2008-04-17 22:42:14 UTC
This patch makes mallinfo() function to return something more useful than 
zeroes. Helps with programs that rely on sensible mallinfo behaviour (e.g. may 
help with documented emacs problem, though I haven't tested emacs).
Comment 1 Eugene Toder 2008-04-17 22:43:48 UTC
Created attachment 24377 [details]
Mallinfo patch
Comment 2 Bart Van Assche 2008-04-19 17:01:25 UTC
Applied (slightly modified) patch on the trunk. Can you please test whether the version on the trunk is OK ? See also http://www.valgrind.org/downloads/repository.html for instructions on how to download and build the trunk.
Comment 3 Eugene Toder 2008-04-19 23:25:26 UTC
I've created a test for mallinfo that checks sanity and basic features.
In fact, my original implementation does not passed it, so here's a patch on top of your commit.
Comment 4 Eugene Toder 2008-04-19 23:26:08 UTC
Created attachment 24416 [details]
Test case
Comment 5 Eugene Toder 2008-04-19 23:27:00 UTC
Created attachment 24417 [details]
Patch to pass the test
Comment 6 Bart Van Assche 2008-04-20 10:45:46 UTC
Thanks for contributing the new patch and regression test. Do you know of any formal standards that document the behavior of mallinfo() ? How can we know that the behavior tested by the regression test matches the behavior of all glibc versions supported by Valgrind ? And how about mallinfo() on AIX and FreeBSD ? AIX is supported since some time by Valgrind, and there is a project ongoing to port Valgrind to FreeBSD.
Comment 7 Eugene Toder 2008-04-20 15:35:02 UTC
In theory mallinfo is defined in SVID2 and XPG4. However I don't have a copy of these and they are old standards. 4th edition of SVID (available from SCO: http://www.sco.com/developers/devspecs/vol1a.pdf) deprecates mallinfo() and doesn't document struct mallinfo. XPG4 evolved into SUS which doesn't have mallinfo.

Moreover, glibc implementation never actually followed the standard. Definition of struct mallinfo here http://docsrv.sco.com:507/en/man/html.S/malloc.S.html (which I believe is what was in SVID2, because HP and SunOS give exactly the same) is quite different from what glibc has. So the only definition for mallinfo on Linux is malloc.h/glibc sources (also note that the link you added to comments - http://www.gnu.org/software/libtool/manual/libc/Statistics-of-Malloc.html - is somewhat outdated; in particular, fields marked as "not used" are currently used).

Most of my regtest checks should work with both standard and glibc implementations, though there are glibc specific bits. I think this makes sense since most programs using mallinfo on Linux will use glibc specific behaviour. According to glibc cvs the first version that should pass my regtest was checked in 1996, so this should not be a problem.

There is no mallinfo on FreeBSD, so no problem here.

I don't have access to any AIX systems, but I suspect that AIX also gave it's own meaning to mallinfo fields. For AIX we can either return zeroes as it was before or someone with access to AIX can provide a version of implementation and regtest that matches AIX behaviour. This means making mallinfo OS specific.
Comment 8 Bart Van Assche 2008-04-21 19:50:23 UTC
A slightly modified version of the last two patches is present in revision 7902. Can you please review the changes and close this issue if you agree with the committed implementation ?
Comment 9 Eugene Toder 2008-04-21 23:53:57 UTC
Looks good to me. Actually, am I missing something or it only differs in comments?
Anyway, as long as it passes my test it's fine with me. Thanks!

btw, I suggest to move test from none to memcheck. none doesn't redirect malloc and friends, so the test actually tests host libc's implementation of mallinfo, not valgrind's implementation (this can be seen in test's output).
Comment 10 Bart Van Assche 2008-04-22 19:15:45 UTC
Next to adding comments, I also added a command-line option -q to the regression test and I added a few lines of code to free all allocated memory, such that memcheck does not complain about memory leaks. The mallinfo regression test has been moved from the directory none/tests to memcheck/tests by this time.
Comment 11 Eugene Toder 2008-04-22 20:26:18 UTC
Ah yes, I noticed improvements to the test, but I was looking for changes to implementation. The -q option is great, actually without it the test may fail if stdio implementation allocates buffers with malloc, but prints are useful for debugging, so it's good to have it an option.
Thanks again.
Comment 12 Julian Seward 2008-05-12 00:35:24 UTC
*** Bug 161251 has been marked as a duplicate of this bug. ***
Comment 13 Julian Seward 2008-05-12 00:41:06 UTC
Mistake; #161251 is NOT a duplicate of this bug.  Ignore Comment #12.