550805 - valgrind "Invalid read of size 8" [@ idalloc]

Status: RESOLVED FIXED

(Core :: Memory Allocator, defect)

Description

[Mats Palmgren (inactive)]

STR: run valgrind on firefox debug build, x86-64 Linux:

Invalid read of size 8
   at 0x40A8CC: idalloc (jemalloc.c:4243)
   by 0x40A9B4: free (jemalloc.c:6017)
   by 0x5B200AB: moz_free (mozalloc.cpp:69)
   by 0x165365EA: gfxTextRun::~gfxTextRun() (mozalloc.h:234)
   by 0x16539C85: nsAutoPtr<gfxTextRun>::~nsAutoPtr() (nsAutoPtr.h:104)
   by 0x1654D0C9: TextRunWordCache::MakeTextRun(unsigned short const*, unsigned int, gfxFontGroup*, gfxTextRunFactory::Parameters const*, unsigned int) (gfxTextRunWordCache.cpp:686)
   by 0x1654D1D2: gfxTextRunWordCache::MakeTextRun(unsigned short const*, unsigned int, gfxFontGroup*, gfxTextRunFactory::Parameters const*, unsigned int) (gfxTextRunWordCache.cpp:992)
   by 0x1654A7B4: gfxTextRunCache::MakeTextRun(unsigned short const*, unsigned int, gfxFontGroup*, gfxContext*, unsigned int, unsigned int) (gfxTextRunCache.cpp:93)
   by 0x160E54AB: nsThebesFontMetrics::AutoTextRun::AutoTextRun(nsThebesFontMetrics*, nsIRenderingContext*, unsigned short const*, int) (nsThebesFontMetrics.h:171)
   by 0x160E425F: nsThebesFontMetrics::GetWidth(unsigned short const*, unsigned int, int&, int*, nsThebesRenderingContext*) (nsThebesFontMetrics.cpp:334)
   by 0x160E025E: nsThebesRenderingContext::GetWidthInternal(unsigned short const*, unsigned int, int&, int*) (nsThebesRenderingContext.cpp:1318)
   by 0x160E0E7A: nsThebesRenderingContext::GetWidth(unsigned short const*, unsigned int, int&, int*) (nsThebesRenderingContext.cpp:943)
   by 0x17CB81F5: nsLayoutUtils::GetStringWidth(nsIFrame const*, nsIRenderingContext*, unsigned short const*, int) (nsLayoutUtils.cpp:2654)
   by 0x17E810C5: nsTextBoxFrame::GetTextSize(nsPresContext*, nsIRenderingContext&, nsString const&, nsSize&, int&) (nsTextBoxFrame.cpp:985)
   by 0x17E81157: nsTextBoxFrame::CalcTextSize(nsBoxLayoutState&) (nsTextBoxFrame.cpp:999)
   by 0x17E812B3: nsTextBoxFrame::GetPrefSize(nsBoxLayoutState&) (nsTextBoxFrame.cpp:1043)
   by 0x17E6F686: nsSprocketLayout::GetPrefSize(nsIFrame*, nsBoxLayoutState&) (nsSprocketLayout.cpp:1366)
   by 0x17E6D885: nsBoxFrame::GetPrefSize(nsBoxLayoutState&) (nsBoxFrame.cpp:808)
   by 0x17E8CC10: nsMenuFrame::GetPrefSize(nsBoxLayoutState&) (nsMenuFrame.cpp:1279)
   by 0x17E6F686: nsSprocketLayout::GetPrefSize(nsIFrame*, nsBoxLayoutState&) (nsSprocketLayout.cpp:1366)
   by 0x17E6D885: nsBoxFrame::GetPrefSize(nsBoxLayoutState&) (nsBoxFrame.cpp:808)
   by 0x17E6F686: nsSprocketLayout::GetPrefSize(nsIFrame*, nsBoxLayoutState&) (nsSprocketLayout.cpp:1366)
   by 0x17E6D885: nsBoxFrame::GetPrefSize(nsBoxLayoutState&) (nsBoxFrame.cpp:808)
   by 0x17E6F686: nsSprocketLayout::GetPrefSize(nsIFrame*, nsBoxLayoutState&) (nsSprocketLayout.cpp:1366)
   by 0x17E6D885: nsBoxFrame::GetPrefSize(nsBoxLayoutState&) (nsBoxFrame.cpp:808)
   by 0x17E6F686: nsSprocketLayout::GetPrefSize(nsIFrame*, nsBoxLayoutState&) (nsSprocketLayout.cpp:1366)
   by 0x17E6D885: nsBoxFrame::GetPrefSize(nsBoxLayoutState&) (nsBoxFrame.cpp:808)
   by 0x17E7041D: nsSprocketLayout::PopulateBoxSizes(nsIFrame*, nsBoxLayoutState&, nsBoxSize*&, int&, int&, int&) (nsSprocketLayout.cpp:783)
   by 0x17E70A11: nsSprocketLayout::Layout(nsIFrame*, nsBoxLayoutState&) (nsSprocketLayout.cpp:247)
   by 0x17E6D329: nsBoxFrame::DoLayout(nsBoxLayoutState&) (nsBoxFrame.cpp:938)
   by 0x17E6B858: nsIFrame::Layout(nsBoxLayoutState&) (nsBox.cpp:548)
   by 0x17E72119: nsStackLayout::Layout(nsIFrame*, nsBoxLayoutState&) (nsStackLayout.cpp:342)
   by 0x17E6D329: nsBoxFrame::DoLayout(nsBoxLayoutState&) (nsBoxFrame.cpp:938)
   by 0x17E6B858: nsIFrame::Layout(nsBoxLayoutState&) (nsBox.cpp:548)
   by 0x17E6DC0B: nsBoxFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) (nsBoxFrame.cpp:748)
   by 0x17E69BB0: nsRootBoxFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) (nsRootBoxFrame.cpp:236)
 Address 0x1e300000 is 48 bytes inside a block of size 1,024 free'd
   at 0x4C236BE: operator delete[](void*) (vg_replace_malloc.c:364)
   by 0x53577FC: JSFunctionBoxQueue::~JSFunctionBoxQueue() (jsparse.h:813)
   by 0x534E1A0: JSCompiler::markFunArgs(JSFunctionBox*, unsigned int) (jsparse.cpp:2061)
   by 0x534E1D5: JSCompiler::analyzeFunctions(JSFunctionBox*, unsigned int&) (jsparse.cpp:1856)
   by 0x5356D58: JSCompiler::compileScript(JSContext*, JSObject*, JSStackFrame*, JSPrincipals*, unsigned int, unsigned short const*, unsigned long, _IO_FILE*, char const*, unsigned int, JSString*, unsigned int) (jsparse.cpp:962)
   by 0x52B0811: JS_CompileUCScriptForPrincipals (jsapi.cpp:4492)
   by 0x52B0A5E: JS_CompileScriptForPrincipals (jsapi.cpp:4447)
   by 0x146918C1: mozJSComponentLoader::GlobalForLocation(nsILocalFile*, JSObject**, char**, long*) (mozJSComponentLoader.cpp:1296)
   by 0x14692DF9: mozJSComponentLoader::LoadModule(nsILocalFile*, nsIModule**) (mozJSComponentLoader.cpp:703)
   by 0x58C2198: nsComponentManagerImpl::AutoRegisterComponent(nsILocalFile*, nsTArray<DeferredModule>&, int) (nsComponentManager.cpp:3116)
   by 0x58C243B: nsComponentManagerImpl::LoadLeftoverComponents(nsCOMArray<nsILocalFile>&, nsTArray<DeferredModule>&, int) (nsComponentManager.cpp:3171)
   by 0x58C48B5: nsComponentManagerImpl::AutoRegister(nsIFile*) (nsComponentManager.cpp:3432)
   by 0x587B47C: NS_InitXPCOM3_P (nsXPComInit.cpp:687)
   by 0x5064B6C: ScopedXPCOMStartup::Initialize() (nsAppRunner.cpp:1112)
   by 0x50685DC: XRE_main (nsAppRunner.cpp:3338)
   by 0x4020FF: main (nsBrowserApp.cpp:158)

Comment 1

[Mats Palmgren (inactive)]

The problem appears to be that we're using the system allocator:
http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/src/gfxFont.cpp#2300
and then jemalloc deallocator:
http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/src/gfxFont.cpp#2374

Comment 2

[Mats Palmgren (inactive)]

Attachment: Like so?

Overriding the "std::nothrow" variants fixes it for me.

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 431005 [details] [diff] [review]
Like so?

Thanks!  I didn't know we were using nothrow.

In the future, I think we should avoid using nothrow and instead use either default or "fallible", but we can switch these functions back after the TODO dynamic analysis.

Comment 4

[Julian Seward [:jseward]]

+1 to land asap!

Comment 5

[Mats Palmgren (inactive)]

http://hg.mozilla.org/mozilla-central/rev/4060f2d15340