ERROR in libc/stdlib/mallocr.c???

[J. Johnston]

Corinna Vinschen wrote:
> 
> Hi,
> 
> I think I have found an error in libc/stdlib/mallocr.c.
> 
> Look into line 1384ff:
> 
>     #define SIZE_SZ                (sizeof(INTERNAL_SIZE_T))
>     #ifndef MALLOC_ALIGNMENT
>     #define MALLOC_ALIGN           8
>     #define MALLOC_ALIGNMENT       (SIZE_SZ + SIZE_SZ)
>     #else
>     #define MALLOC_ALIGN           MALLOC_ALIGNMENT
>     #endif
> 
> As you can see, MALLOC_ALIGNMENT is by default defined as 2*SIZE_SZ
> which in turn is defined as sizeof(INTERNAL_SIZE_T) which in turn is
> defined in line 431ff:
> 
>     #ifndef INTERNAL_SIZE_T
>     #define INTERNAL_SIZE_T size_t
>     #endif
> 
> The problem here is that according to the comment in line 168ff
> MALLOC_ALIGNMENT is set to 8 by default:
> 
>   MALLOC_ALIGNMENT          (default: NOT defined)
>      Define this to 16 if you need 16 byte alignment instead of 8 byte alignment
>      which is the normal default.
> 
> Unfortunately this isn't true due to the above statement
> 
>      #define MALLOC_ALIGNMENT       (SIZE_SZ + SIZE_SZ)
> 
> Imagine a system which has sizeof(size_t) = 2 bytes. MALLOC_ALIGNMENT
> is now set to 4!
> 
> As a result of that error, any allocation of small values (<=8) fails
> in mALLOc() at line 2545ff:
> 
>     malloc_extend_top(RCALL nb);
>     remainder_size = long_sub_size_t(chunksize(top), nb);
>     if (chunksize(top) < nb || remainder_size < (long)MINSIZE)
>     {
>       MALLOC_UNLOCK;
>       return 0; /* propagate failure */
>     }
> 
> Even if `malloc_extend_top' succeeded, the following if fails
> since `remainder_size' is < 0.
> 
> Odd enough, MALLOC_ALIGNMENT must be >= 8, otherwise the implementation
> always fails to do the right thing.
> 
> What can we do? I can see three obvious solutions:
> 
> - Force ports to targets with sizeof(size_t) < 4 to define
>   MALLOC_ALIGNMENT as a value >= 8.
> 
> - Force ports to targets with sizeof(size_t) < 4 to set INTERNAL_SIZE_T
>   to a datatype with at least 4 bytes (not good, IMO).
> 
> - The define in line 1387 could be changed to
>         #define MALLOC_ALIGNMENT 8
>   or
>         #define MALLOC_ALIGNMENT MALLOC_ALIGN
> 
> The third is the most simple solution which result in a correct
> behaviour, IMO.
> 

Option 3 is reasonable but should be modified to not affect platforms that have
size_t > 4.

     #define SIZE_SZ                (sizeof(INTERNAL_SIZE_T))
     #ifndef MALLOC_ALIGNMENT
     #define MALLOC_ALIGNMENT       (SIZE_SZ < 4 ? 8 : (SIZE_SZ + SIZE_SZ))
     #endif
     #define MALLOC_ALIGN           MALLOC_ALIGNMENT
    
-- Jeff J.