X-Authentication-Warning: delorie.com: mailnull set sender to djgpp-workers-bounces using -f
Date: Mon, 11 Feb 2002 16:52:58 -0600
From: Eric Rudd <rudd AT cyberoptics DOT com>
Subject: Re: Alignment problem
To: Charles Sandmann <sandmann AT clio DOT rice DOT edu>
Cc: eliz AT is DOT elta DOT co DOT il, djgpp-workers AT delorie DOT com
Message-id: <3C684B4A.3AC31AA2@cyberoptics.com>
Organization: CyberOptics
MIME-version: 1.0
X-Mailer: Mozilla 4.72 [en] (Win95; U)
Content-type: text/plain; charset=us-ascii
Content-transfer-encoding: 7bit
X-Accept-Language: en,pdf
References: <10202112024 DOT AA29687 AT clio DOT rice DOT edu>
Reply-To: djgpp-workers AT delorie DOT com

Charles Sandmann wrote:

>> Since malloc rounds the size to a multiple of 8 bytes, the rest should be
>> fine as long as we get contiguous memory.  Whenever we get noncontiguous
>> chunk, we can again align its beginning and continue.
>
> Agreed.  (This is all in one place - probably 2-3 lines of code).

Here is a patch that seemed to work (though I've only given it a cursory
test).  Please take a look at it and see whether I have made a mistake:


*** src/libc/ansi/stdlib/malloc.old     Fri Aug 20 18:46:58 1999
--- src/libc/ansi/stdlib/malloc.c       Mon Feb 11 16:05:02 2002
***************
*** 20,25 ****
--- 20,26 ----

  #define NUMSMALL      0
  #define ALIGN         8
+ #define ALIGN1          (ALIGN-1)
  #define SMALL         (NUMSMALL*ALIGN)

  static BLOCK *slop = 0;
***************
*** 192,203 ****
    }

    chunk_size = size+16; /* two ends plus two placeholders */
!   rv = (BLOCK *)sbrk(chunk_size);
    if (rv == (BLOCK *)(-1))
      return 0;
  #if DEBUG
    printf("sbrk(%d) -> %08x, expected %08x\n", chunk_size, rv,
expected_sbrk);
  #endif
    if (rv == expected_sbrk)
    {
      expected_sbrk = (BLOCK *)((char *)rv + chunk_size);
--- 193,208 ----
    }

    chunk_size = size+16; /* two ends plus two placeholders */
!
!   /* Ask sbrk() for ALIGN1 extra bytes, to allow for possible alignment. */

!
!   rv = (BLOCK *)sbrk(chunk_size+ALIGN1);
    if (rv == (BLOCK *)(-1))
      return 0;
  #if DEBUG
    printf("sbrk(%d) -> %08x, expected %08x\n", chunk_size, rv,
expected_sbrk);
  #endif
+   rv = (BLOCK *) (((int)rv + ALIGN1) & ~ALIGN1);  /* Align the pointer. */
    if (rv == expected_sbrk)
    {
      expected_sbrk = (BLOCK *)((char *)rv + chunk_size);

> Another idea:
>
> We could add a sbrk(0) call before calling sbrk() with the expected size -
> and if it's aligned the sbrk() value should be also.  If sbrk(0) not
> aligned, we sbrk() the small number of bytes to align - which will always
> work (unless there's a equality bug right at the boundary in sbrk...)

It seemed to me that simply keeping sbrk() at least 7 bytes ahead of the
expected end of the region was simpler and safer.  I don't know whether there
are people doing multi-threaded stuff out there, but if they are, I fear that
sbrk() could get called with some bad (size % 8 != 0) value between the two
times it is called in the above scheme.  Or is malloc() inherently
non-re-entrant?

> By the way, I think the reason we are off by 4 is stubinfo is 0x54 bytes
> and it's the very first thing we sbrk() :-P

Aha!  However, I think that forcing malloc() to work with non-aligned sbrk()
values is a better solution than requiring alignment from stubinfo, since it
makes the code less intertwined.

-Eric