Re: jemalloc() assumes DSS is aligned

On Wednesday, June 13, 2012 12:29:26 pm Jason Evans wrote:
> On Jun 13, 2012, at 8:31 AM, John Baldwin wrote:
> > I tracked down a weird bug at work on the older jemalloc in FreeBSD 8/9 that a 
> > co-worker tripped over.  Specifically, if you build the program below and link 
> > it with gold, the program will have an _end symbol that is on an odd address 
> > (std::nothrow results in some single-byte symbol being added to the end of the 
> > BSS).  This causes the first arena allocated by jemalloc to use an odd 
> > address, and the rbt_nil structures for that arena's embedded trees (like 
> > runs_avail) to be allocated on odd addresses.  This interferes with the RB 
> > trees using the low bit to distinguish red vs black.  Specifically, the 
> > program ends up setting the right node of rbt_nil to an incorrect pointer 
> > value (the low bit gets cleared) resulting in an eventual segfault.  Looking 
> > at phkmalloc, it always applied round_page() to the results from sbrk().  I 
> > believe that for jemalloc only the very first allocation from the DSS needs to 
> > check for misalignment, and the patch below does fix the segfault on FreeBSD 
> > 8.  I have a stab at porting the change to jemalloc 3.0.0 in HEAD, but I'm not 
> > sure if it is quite correct.  Also, I only made the DSS align on the quantum 
> > boundary rather than a page boundary.  BTW, I filed a bug with the binutils 
> > folks as I initially thought this was a gold bug.  However, POSIX doesn't make 
> > any guarantees about the return value of sbrk(), so I think gold is not 
> > broken.
> 
> Hi John,
> 
> Your fix for FreeBSD 7/8/9 looks correct to me.  I don't currently have any development machines running anything but 10-CURRENT, so I'd be 
grateful if you could commit the fix, assuming it isn't much trouble for you.  (I'll set up additional development installations if needed.)

Sure, I'm fine with doing that.

> I don't think this is an issue for HEAD's chunk_alloc_dss(), because there is logic to always insert enough padding to allocate on chunk alignment 
boundaries, and also base_alloc() no longer makes any attempt to use a partial dss 'chunk'.

Ok, this was my main concern was to ensure it was fixed going forward.

> Thanks,
> Jason
> 
> P.S. Sorry about putting off responding to your original email for too long.

No problem, I figured the original got lost. :-P

-- 
John Baldwin