This is the mail archive of the
libc-alpha@sources.redhat.com
mailing list for the glibc project.
Re: dl-sysdep Problem
- To: libc-alpha at sources dot redhat dot com
- Subject: Re: dl-sysdep Problem
- From: John Reiser <jreiser at BitWagon dot com>
- Date: Sun, 01 Jul 2001 12:47:10 -0700
- CC: aj at suse dot de
- Organization: -
Andreas Jaeger <aj@suse.de> wrote:
> the brk (0) looks quite strange to me.
There is a global variable __curbrk (with weak alias ___brk_addr)
which gets set to its first value by calling __brk(0).
See sysdeps/unix/sysv/linux/i386/brk.c line 46.
However, in __sbrk() sysdeps/generic/sbrk.c line 41:
if (__curbrk == NULL || __libc_multiple_libcs)
if (__brk (0) < 0) /* Initialize the break. */
return (void *) -1;
So the __brk(0) in sysdeps/unix/sysv/linux/dl-sysdep.c line 32
also appears to be redundant: the test for __curbrk==NULL will
be true during the first call to __sbrk(), thus causing __brk(0)
to be called then. frob_brk() could become empty.
Tracking down whether this is true in all architectures
would take some effort.
Andreas Jaeger wrote:
>
> John writes in PR libc/2312:
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> There are two problems with this code from
> ----- glibc-2.2.3/sysdeps/unix/sysv/linux/dl-sysdep.c:
> if (__sbrk (0) == &_end)
> /* The dynamic linker was run as a program, and so the initial break
> starts just after our bss, at &_end. The malloc in dl-minimal.c
> will consume the rest of this page, so tell the kernel to move the
> break up that far. When the user program examines its break, it
> will see this new value and not clobber our data. */
> __sbrk (_dl_pagesize - ((&_end - (void *) 0) & _dl_pagesize));
> -----
> The code does not do what the comment says; there is a "- 1" missing
> from the usual formulation. In detail: Note that the form of the
> argument is A - (B & A) , which is equivalent to A &~ B .
> In this case, A is _dl_pagesize , which is a power of 2.
> Thus the value of the argument to __sbrk is either 0 or _dl_pagesize ,
> (either the &~ clears the '1' bit, or the &~ leaves it untouched),
> and in fact is 0 unless &_end designates an even page.
> So after the return from __sbrk, the brk will be somewhere on
> an odd-numbered page, and will not be on a page boundary unless
> &_end is already on a page boundary.
>
> However, the action is not needed anyway, because [if it is removed, then]
> it is subsumed by correct code which executes later:
> ----- sysdeps/generic/dl-sysdep.c:199
> if (__sbrk (0) == &_end)
> /* The dynamic linker was run as a program, and so the initial break
> starts just after our bss, at &_end. The malloc in dl-minimal.c
> will consume the rest of this page, so tell the kernel to move the
> break up that far. When the user program examines its break, it
> will see this new value and not clobber our data. */
> __sbrk (_dl_pagesize - ((&_end - (void *) 0) & (_dl_pagesize - 1)));
> -----
> Note the " - 1" near the righthand end of the line, which differs
> from sysdeps/unix/sysv/linux/dl-sysdep.c .
> The incorrect code in linux/dl-sysdep.c is invoked from
> ----- sysdeps/generic/dl-sysdep.c:181
> #ifdef DL_SYSDEP_INIT
> DL_SYSDEP_INIT;
> #endif
>
> #ifdef DL_PLATFORM_INIT
> DL_PLATFORM_INIT;
> #endif
> -----
> which is just a few lines earlier than the correct code which recovers
> from the mistake.
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> I therefore suggest the appended patch but ask to double check this -
> the brk (0) looks quite strange to me.
>
> Ok to commit?
>
> Andreas
>
> 2001-07-01 Andreas Jaeger <aj@suse.de>
>
> * sysdeps/unix/sysv/linux/dl-sysdep.c (frob_brk): Remove duplicate
> sbrk.
> Closes PR libc/2312, suggested by John Reiser <jreiser@BitWagon.com>.
>
> ============================================================
> Index: sysdeps/unix/sysv/linux/dl-sysdep.c
> --- sysdeps/unix/sysv/linux/dl-sysdep.c 1997/07/26 02:33:02 1.3
> +++ sysdeps/unix/sysv/linux/dl-sysdep.c 2001/07/01 12:24:34
> @@ -1,5 +1,5 @@
> /* Dynamic linker system dependencies for Linux.
> - Copyright (C) 1995, 1997 Free Software Foundation, Inc.
> + Copyright (C) 1995, 1997, 2001 Free Software Foundation, Inc.
> This file is part of the GNU C Library.
>
> The GNU C Library is free software; you can redistribute it and/or
> @@ -27,16 +27,7 @@
> static inline void
> frob_brk (void)
> {
> - extern size_t _dl_pagesize;
> - extern void _end;
> __brk (0); /* Initialize the break. */
> - if (__sbrk (0) == &_end)
> - /* The dynamic linker was run as a program, and so the initial break
> - starts just after our bss, at &_end. The malloc in dl-minimal.c
> - will consume the rest of this page, so tell the kernel to move the
> - break up that far. When the user program examines its break, it
> - will see this new value and not clobber our data. */
> - __sbrk (_dl_pagesize - ((&_end - (void *) 0) & _dl_pagesize));
> }
>
> #include <sysdeps/generic/dl-sysdep.c>
>
> --
> Andreas Jaeger
> SuSE Labs aj@suse.de
> private aj@arthur.inka.de
> http://www.suse.de/~aj
--
John Reiser, jreiser@BitWagon.com