[Webkit-unassigned] [Bug 19946] Possible misalignment in RenderArena when compiled for debug

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=19946


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #55726|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #3 from Darin Adler <darin at apple.com>  2010-05-11 17:09:43 PST ---
(From update of attachment 55726)
> +#ifdef WTF_ARMV5_NEEDS_ALIGN_8
> +#define ARENA_ALIGN_MASK 7
> +#else
>  #define ARENA_ALIGN_MASK 3
> +#endif

I'd like to see us do in a way that's a bit more automatic. One way is:

    #define ARENA_ALIGN_MASK (sizeof(AllocAlignmentInteger) - 1)

All that ARM-specific stuff seems undesirable. In fact, other 64-bit platforms would probably benefit from 64-bit-aligned objects in the arenas.

> +#define ARENA_ALIGNED_HEADER_SIZE ((sizeof(RenderArenaDebugHeader) + ARENA_ALIGN_MASK) & ~ARENA_ALIGN_MASK)

We should use the ARENA_ALIGN macro here. We can change it so it doesn't take a "pool" argument and then use it like this:

   ARENA_ALIGN(sizeof(RenderArenaDebugHeader))

Does this need to be a macro? This is C++ so I'd think we could just use "static const" instead.

> -    RenderArenaDebugHeader* header = static_cast<RenderArenaDebugHeader*>(ptr) - 1;
> +    void* block = static_cast<unsigned char*>(ptr) - ARENA_ALIGNED_HEADER_SIZE;

Using "unsigned char*" here instead of "char*" is a bit strange.

Change seems OK, but I think it would be better to do this without so much ARM-specific. review- for now

Please let me know if I misunderstood the basis of the need for this on ARM. Is it needed on 32-bit ARM?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.