Re: Bug#487745: epiphany-webkit: crashes on startup
On Tue, Jun 24, 2008 at 09:15:03AM +0100, Sjoerd Simons <sjoerd@luon.net> wrote:
> On Tue, Jun 24, 2008 at 08:44:23AM +0200, Mike Hommey wrote:
> > Or maybe something like this:
> >
> > diff --git a/JavaScriptCore/wtf/FastMalloc.cpp b/JavaScriptCore/wtf/FastMalloc.cpp
> > index 8afc70d..496d1ee 100644
> > --- a/JavaScriptCore/wtf/FastMalloc.cpp
> > +++ b/JavaScriptCore/wtf/FastMalloc.cpp
> > @@ -1820,7 +1820,7 @@ static TCMalloc_Central_FreeListPadded central_cache[kNumClasses];
> >
> > // Page-level allocator
> > static SpinLock pageheap_lock = SPINLOCK_INITIALIZER;
> > -static void* pageheap_memory[(sizeof(TCMalloc_PageHeap) + sizeof(void*) - 1) / sizeof(void*)];
> > +static uint64_t* pageheap_memory[(sizeof(TCMalloc_PageHeap) + sizeof(uint64_t*) - 1) / sizeof(uint64_t*)];
> > static bool phinited = false;
>
> All pointers are the same size and have the same alignment requirements, so
> this change doesn't force the compiler to change make pageheap_memory aligned
> on 8 bytes.
Actually, I was meaning to replace void* with uint64_t, not uint64_t*.
So
static uint64_t pageheap_memory[(sizeof(TCMalloc_PageHeap) + sizeof(uint64_t) - 1) / sizeof(uint64_t)];
> I'm probably missing some trick (not very familiar with C++), but why
> not just do:
>
> static TCMalloc_PageHeap pageheap_memory;
>
> static inline TCMalloc_PageHeap* getPageHeap()
> {
> return &pageheap_memory;
> }
Or simply:
static TCMallog_PageHeap *pageheap = &pageheap_memory;
I don't really know. The reason why they are using a union is strict
aliasing:
http://git.debian.org/?p=pkg-webkit/webkit.git;a=commitdiff;h=ad4791202578b50ac2b19fa92c4e81550e1bdeea
On the other hand, why declare pageheap_memory as an array of void* and
not directly as TCMalloc_PageHeap ?
Interestingly, before being a void* array, it was:
static char pageheap_memory[sizeof(TCMalloc_PageHeap)];
And they changed it to the current thing to solve... alignment issues
on arm.
Before that, tcmalloc was not used, so the code didn't exist,
and the first public version of tcmalloc also uses an array of char.
http://code.google.com/p/google-perftools/source/browse/trunk/src/tcmalloc.cc?r=9
I see no rationale not to use static TCMallog_PageHeap...
Mike
Reply to: