Re: [Valgrind-developers] valgrind/coregrind

[Julian S. <js...@ac...>]

On Saturday 13 March 2004 11:30, Nicholas Nethercote wrote:
> On Sat, 13 Mar 2004, Jeremy Fitzhardinge wrote:
> > Close bug 71906.  Set default malloc alignment to 8 rather than 4.
>
> Um, problem with that is that non-4-byte aligned mallocs are done with
> VG_(arena_malloc_aligned)(), rather than VG_(arena_malloc)().  So now
> VG_(arena_malloc)() won't be used at all unless one sets --alignment=4.
>
> This doesn't seem like the right thing to do.  Julian, you wrote the
> allocator, do you have an opinion about this?

(historical note:) The Right Solution is to modify vg_malloc2.c so all
payload pointers come out 8-aligned.  I realised I had made a mistake 
to set the baseline alignment at 4 rather than 8 soon after this allocator
went into service, but it was already a complete rewrite of a previous
one and I couldn't be bothered to rewrite it yet again.

See "block layout" at vg_malloc2.c:145.  If the block layout is
changed to the following, and the number of red zone words is 
forced to be even, then presumably the payloads will all be 8-aligned.

     this block total sizeW   (1 word)
     freelist previous ptr    (1 word)
     red zone words (depends on .rz_szW field of Arena)
     (payload words)
     red zone words (depends on .rz_szW field of Arena)
     freelist next  ptr       (1 word)
     this block total sizeW   (1 word)

Clearly the red zone sizeW % 2 == 0 is trivial to enforce.  Question
is whether or not moving the freelist next ptr after the payload is
going to break things or not.  I suspect it will be ok.  Mostly this
will involve possibly changing the family of small functions beginning
at "mk_free_bszW" through to "bszW_to_pszW" -- all the layout knowledge
is encoded in them, I think.

There might be some complication in VG_(arena_malloc_aligned).
That's the only really nasty bit.

I suspect a couple of hours figuring this out might sort it, 
partly because it's fairly well abstracted, partly because there's
a good sanity checker, and partly because there's a standalone
test driver at the bottom of the file.  And given that we are
headed inevitably to 64bit-ville, it's necessary anyway.

J