Bug#26148: marked as done (mcheck() and mprobe() are broken)
Your message dated Fri, 17 Sep 1999 12:47:25 -0700
with message-id <v04205524b4084b1409e8@[206.163.71.146]>
and subject line Fixed a long time ago.
has caused the attached Bug report to be marked as done.
This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.
(NB: If you are a system administrator and have no idea what I am
talking about this indicates a serious mail system misconfiguration
somewhere. Please contact me immediately.)
Darren Benham
(administrator, Debian Bugs database)
Received: (at submit) by bugs.debian.org; 26 Aug 1998 07:22:38 +0000
Received: (qmail 4523 invoked from network); 26 Aug 1998 07:22:36 -0000
Received: from ppp-gw.fifi.org (HELO tantale.fifi.org) (root@209.133.45.182)
by debian.novare.net with SMTP; 26 Aug 1998 07:22:36 -0000
Received: from ceramic.fifi.org (phil@ceramic.fifi.org [209.133.45.186])
by tantale.fifi.org (8.8.8/8.8.8/Debian/GNU) with ESMTP id AAA28155;
Wed, 26 Aug 1998 00:22:25 -0700
From: Philippe Troin <phil@fifi.org>
Received: (from phil@localhost)
by ceramic.fifi.org (8.8.8/8.8.8/Debian/GNU) id AAA15551;
Wed, 26 Aug 1998 00:22:24 -0700
Date: Wed, 26 Aug 1998 00:22:24 -0700
Message-Id: <199808260722.AAA15551@ceramic.fifi.org>
To: bugs@gnu.org, submit@bugs.debian.org
Subject: mcheck() and mprobe() are broken
Package: libc6
Version: 2.0.7t-1
(The two lines above are for the debian bug tracking system)
>Submitter-Id: net
>Originator: Philippe Troin <phil@fifi.org>
>Organization: None
>Confidential: no
>Synopsis: mcheck() and mprobe() do not work as advertised
>Severity: non-critical
>Priority: low
>Category: libc
>Class: sw-bug
>Release: libc-2.0.7
>Environment: Any
Host type: i586-pc-linux-gnu
System: Linux ceramic 2.1.115 #1 SMP Sun Aug 9 14:58:48 PDT 1998 i686 unknown
Architecture: i686
Addons: crypt linuxthreads localedata
Build CFLAGS: -g -O2
Build CC: gcc -B$(common-objpfx)
Build shared: yes
Build profile: yes
Build omitfp: no
Stdio: libio
>Description:
The excellent mcheck() utility is broken because it doesn't take enough
precautions when manipulating the various __{malloc,free,realloc}__hooks.
Additionnaly mprobe() doesn't work (passes the wrong address to checkhdr()).
>How-To-Repeat:
This perfectly valid C program with mcheck() enabled reports an inexistant
memory problem:
#include <stdlib.h>
#include <string.h>
#include <mcheck.h>
int main() {
void *ptr1, *ptr2;
/**/
mcheck(0);
ptr1=malloc(10);
memset(ptr1, 0, 10);
ptr2=malloc(10);
memset(ptr2, 0, 10);
ptr1=realloc(ptr1, 20);
memset(ptr1, 0, 20);
free(ptr1);
free(ptr2);
return 0;
}
>Fix:
I found the problem to be coming from the malloc/malloc.c file which uses
itself __malloc_hook and __realloc_hook to perform a first-time
initialization. Whenever calling malloc or realloc for the first time in the
program, the __malloc_hook and __realloc_hook are reset to NULL, and as
mcheck() hooks only preserves __malloc_hook before passing control to the real
malloc, __realloc_hook gets cleared.
Also, mprobe() doesn't translate the passed pointer to the mcheck header
before passing it to checkhdr() causing every call to mprobe() to fail.
I fixed the problem with the enclosed patch by:
1) Preserving and restoring all hooks (malloc, free, realloc, memalign) before
the control passes to the underlaying ("real" or other hooks) routines
(functions savehooks and restorehooks).
2) Re-storing the underlaying hook values when returning from underlaying
functions instead of restoring the hooks from the values saved at
initialization. The idea is that the underlaying hooks are free to modify
themselves, which the malloc hooks does (however not doing this proved to
be harmless because the malloc/malloc.c initialization hooks check they've
not been called already).
3) Providing a hook for the (undocumented) memalign, otherwise calling free()
on memalign()-obtained memory will cause an error. This is mostly of
dubious use since memory returned by memalign() won't be aligned correctly
because of the way check works.
4) Changing the texinfo files to reflect that all hooks should be saved before
passing control to the underlaying functions, hooks should be re-saved when
returning from underlaying functions, providing a bigger example of using
hooks and documenting __memalign_hook, documenting how to use gdb for
invoking mcheck() without recompiling or relinking.
5) Fixing the mprobe() function.
Patch:
diff -ru ../glibc-2.0.7t.deborig/malloc/mcheck.c glibc-2.0.7t/malloc/mcheck.c
--- ../glibc-2.0.7t.deborig/malloc/mcheck.c Wed Mar 25 02:34:25 1998
+++ glibc-2.0.7t/malloc/mcheck.c Tue Aug 25 22:39:44 1998
@@ -27,10 +27,17 @@
#include <stdio.h>
#endif
-/* Old hook values. */
-static void (*old_free_hook) __P ((__ptr_t ptr));
-static __ptr_t (*old_malloc_hook) __P ((__malloc_size_t size));
-static __ptr_t (*old_realloc_hook) __P ((__ptr_t ptr, __malloc_size_t size));
+/* Structure to hold hooks */
+struct hook_status
+{
+ void (*free_hook) (__ptr_t);
+ __ptr_t (*malloc_hook) (__malloc_size_t);
+ __ptr_t (*realloc_hook) (__ptr_t, __malloc_size_t);
+ __ptr_t (*memalign_hook) (__malloc_size_t, __malloc_size_t);
+};
+
+/* Old hooks values */
+static struct hook_status old_hooks;
/* Function to call when something awful happens. */
static void (*abortfunc) __P ((enum mcheck_status));
@@ -91,11 +98,35 @@
return status;
}
+static void savehooks __P ((struct hook_status*));
+static void
+savehooks(hooks)
+ struct hook_status *hooks;
+{
+ hooks->malloc_hook = __malloc_hook;
+ hooks->realloc_hook = __realloc_hook;
+ hooks->free_hook = __free_hook;
+ hooks->memalign_hook = __memalign_hook;
+}
+
+static void restorehooks __P ((const struct hook_status*));
+static void
+restorehooks(hooks)
+ const struct hook_status *hooks;
+{
+ __malloc_hook = hooks->malloc_hook;
+ __realloc_hook = hooks->realloc_hook;
+ __free_hook = hooks->free_hook;
+ __memalign_hook = hooks->memalign_hook;
+}
+
static void freehook __P ((__ptr_t));
static void
freehook (ptr)
__ptr_t ptr;
{
+ struct hook_status hooks;
+
if (ptr)
{
struct hdr *hdr = ((struct hdr *) ptr) - 1;
@@ -104,9 +135,11 @@
flood (ptr, FREEFLOOD, hdr->size);
ptr = (__ptr_t) hdr;
}
- __free_hook = old_free_hook;
+ savehooks(&hooks);
+ restorehooks(&old_hooks);
free (ptr);
- __free_hook = freehook;
+ savehooks(&old_hooks);
+ restorehooks(&hooks);
}
static __ptr_t mallochook __P ((__malloc_size_t));
@@ -114,11 +147,40 @@
mallochook (size)
__malloc_size_t size;
{
+ struct hook_status hooks;
struct hdr *hdr;
- __malloc_hook = old_malloc_hook;
+ savehooks(&hooks);
+ restorehooks(&old_hooks);
hdr = (struct hdr *) malloc (sizeof (struct hdr) + size + 1);
- __malloc_hook = mallochook;
+ savehooks(&old_hooks);
+ restorehooks(&hooks);
+
+ if (hdr == NULL)
+ return NULL;
+
+ hdr->size = size;
+ hdr->magic = MAGICWORD;
+ ((char *) &hdr[1])[size] = MAGICBYTE;
+ flood ((__ptr_t) (hdr + 1), MALLOCFLOOD, size);
+ return (__ptr_t) (hdr + 1);
+}
+
+static __ptr_t memalignhook __P ((__malloc_size_t, __malloc_size_t));
+static __ptr_t
+memalignhook (size, align)
+ __malloc_size_t size;
+ __malloc_size_t align;
+{
+ struct hook_status hooks;
+ struct hdr *hdr;
+
+ savehooks(&hooks);
+ restorehooks(&old_hooks);
+ hdr = (struct hdr *) memalign (sizeof (struct hdr) + size + 1, align);
+ savehooks(&old_hooks);
+ restorehooks(&hooks);
+
if (hdr == NULL)
return NULL;
@@ -135,6 +197,7 @@
__ptr_t ptr;
__malloc_size_t size;
{
+ struct hook_status hooks;
struct hdr *hdr;
__malloc_size_t osize;
@@ -152,13 +215,11 @@
osize = 0;
hdr = NULL;
}
- __free_hook = old_free_hook;
- __malloc_hook = old_malloc_hook;
- __realloc_hook = old_realloc_hook;
+ savehooks(&hooks);
+ restorehooks(&old_hooks);
hdr = (struct hdr *) realloc ((__ptr_t) hdr, sizeof (struct hdr) + size + 1);
- __free_hook = freehook;
- __malloc_hook = mallochook;
- __realloc_hook = reallochook;
+ savehooks(&old_hooks);
+ restorehooks(&hooks);
if (hdr == NULL)
return NULL;
@@ -214,12 +275,11 @@
/* These hooks may not be safely inserted if malloc is already in use. */
if (!__malloc_initialized <= 0 && !mcheck_used)
{
- old_free_hook = __free_hook;
- __free_hook = freehook;
- old_malloc_hook = __malloc_hook;
+ savehooks(&old_hooks);
__malloc_hook = mallochook;
- old_realloc_hook = __realloc_hook;
__realloc_hook = reallochook;
+ __free_hook = freehook;
+ __memalign_hook = memalignhook;
mcheck_used = 1;
}
@@ -229,5 +289,5 @@
enum mcheck_status
mprobe (__ptr_t ptr)
{
- return mcheck_used ? checkhdr (ptr) : MCHECK_DISABLED;
+ return mcheck_used ? checkhdr (((struct hdr*)ptr)-1) : MCHECK_DISABLED;
}
diff -ru ../glibc-2.0.7t.deborig/manual/memory.texi glibc-2.0.7t/manual/memory.texi
--- ../glibc-2.0.7t.deborig/manual/memory.texi Fri Jan 30 09:05:21 1998
+++ glibc-2.0.7t/manual/memory.texi Tue Aug 25 22:32:02 1998
@@ -572,7 +572,23 @@
The easiest way to arrange to call @code{mcheck} early enough is to use
the option @samp{-lmcheck} when you link your program; then you don't
-need to modify your program source at all.
+need to modify your program source at all. Alternately you might use
+a debugger to insert a call to @code{mcheck} whenever the program is
+started, for example these gdb commands will automatically call @code{mcheck}
+whenever the program starts:
+
+@smallexample
+(gdb) break main
+Breakpoint 1, main (argc=2, argv=0xbffff964) at whatever.c:10
+(gdb) command 1
+Type commands for when breakpoint 1 is hit, one per line.
+End with a line saying just "end".
+>call mcheck(0)
+>continue
+>end
+(gdb) ...
+@end smallexample
+
@end deftypefun
@deftypefun {enum mcheck_status} mprobe (void *@var{pointer})
@@ -658,34 +674,86 @@
@end smallexample
@end defvar
+@comment malloc.h
+@comment GNU
+@defvar __memalign_hook
+The value of this variable is a pointer to function that @code{memalign}
+uses whenever it is called. You should define this function to look
+like @code{memalign}; that is, like:
+
+@smallexample
+void *@var{function} (size_t @var{size}, size_t @var{alignment})
+@end smallexample
+@end defvar
+
You must make sure that the function you install as a hook for one of
these functions does not call that function recursively without restoring
the old value of the hook first! Otherwise, your program will get stuck
-in an infinite recursion.
-
-Here is an example showing how to use @code{__malloc_hook} properly. It
-installs a function that prints out information every time @code{malloc}
-is called.
+in an infinite recursion. Before calling the function recursively, one
+should make sure to restore all the hooks to their previous value. When
+coming back from the recursive call, all the hooks should be resaved
+since a hook might modify itself.
+
+Here is an example showing how to use @code{__malloc_hook} and
+@code{__free_hook} properly. It installs a function that prints out
+information every time @code{malloc} or @code{free} is called. We just
+assume here that @code{realloc} and @code{memalign} are not used in our
+program.
@smallexample
+/* Global variables used to hold underlaying hook values */
static void *(*old_malloc_hook) (size_t);
+static void (*old_free_hook) (void*);
+
+/* Prototypes for our hooks */
+static void *my_malloc_hook (size_t);
+static void my_free_hook(void*);
+
static void *
my_malloc_hook (size_t size)
@{
void *result;
+ /* Restore all old hooks */
__malloc_hook = old_malloc_hook;
+ __free_hook = old_free_hook;
+ /* Call recursively */
result = malloc (size);
+ /* Save underlaying hooks */
+ old_malloc_hook = __malloc_hook;
+ old_free_hook = __free_hook;
/* @r{@code{printf} might call @code{malloc}, so protect it too.} */
printf ("malloc (%u) returns %p\n", (unsigned int) size, result);
+ /* Restore our own hooks */
__malloc_hook = my_malloc_hook;
+ __free_hook = my_free_hook;
return result;
@}
+static void *
+my_free_hook (void *ptr)
+@{
+ /* Restore all old hooks */
+ __malloc_hook = old_malloc_hook;
+ __free_hook = old_free_hook;
+ /* Call recursively */
+ free (ptr);
+ /* Save underlaying hooks */
+ old_malloc_hook = __malloc_hook;
+ old_free_hook = __free_hook;
+ /* @r{@code{printf} might call @code{free}, so protect it too.} */
+ printf ("freed pointer %p\n", ptr);
+ /* Restore our own hooks */
+ __malloc_hook = my_malloc_hook;
+ __free_hook = my_free_hook;
+@}
+
main ()
@{
...
old_malloc_hook = __malloc_hook;
+ old_free_hook = __free_hook;
__malloc_hook = my_malloc_hook;
+ __free_hook = my_free_hook;
...
@}
@end smallexample
@@ -805,6 +873,9 @@
@item void (*__free_hook) (void *@var{ptr})
A pointer to a function that @code{free} uses whenever it is called.
+
+@item void (*__memalign_hook) (size_t @var{size}, size_t @var{alignment})
+A pointer to a function that @code{memalign} uses whenever it is called.
@item struct mallinfo mallinfo (void)
Return information about the current dynamic memory usage.
Phil.
Reply to: