767406 - Uninitialised value use in mozilla::scache::StartupCache::WriteToDisk()

Status: RESOLVED FIXED

Description

[Julian Seward [:jseward]]

TEST_PATH=dom/browser-element/mochitest/test_browserElement_oop_Alert.html

gives the complaint below.  This also happens on B2G, every time I
start up a content process (I think .. not sure).

Conditional jump or move depends on uninitialised value(s)
   at 0x53C5025: mozilla::scache::StartupCache::WriteToDisk() (StartupCache.cpp:418)
   by 0x53C5462: mozilla::scache::StartupCache::~StartupCache() (StartupCache.cpp:136)
   by 0x53C5A1A: mozilla::scache::StartupCache::InitSingleton() (StartupCache.cpp:112)
   by 0x53C5A5C: mozilla::scache::StartupCache::GetSingleton() (StartupCache.cpp:93)
   by 0x606A150: NS_InitXPCOM2_P (nsXPComInit.cpp:493)
   by 0x52A2C26: XRE_InitEmbedding2 (nsEmbedFunctions.cpp:157)
   by 0x5F85B1C: mozilla::ipc::ScopedXREEmbed::Start() (ScopedXREEmbed.cpp:83)
   by 0x5F567DE: mozilla::dom::ContentProcess::Init() (ContentProcess.cpp:22)
   by 0x52A2FDA: XRE_InitChildProcess (nsEmbedFunctions.cpp:474)
   by 0x401A8C: main (MozillaRuntimeMain.cpp:48)
 Uninitialised value was created by a heap allocation
   at 0x402AC1B: malloc (vg_replace_malloc.c:268)
   by 0x4060EFB: moz_xmalloc (mozalloc.cpp:54)
   by 0x53C59DE: mozilla::scache::StartupCache::InitSingleton() (mozalloc.h:200)
   by 0x53C5A5C: mozilla::scache::StartupCache::GetSingleton() (StartupCache.cpp:93)
   by 0x606A150: NS_InitXPCOM2_P (nsXPComInit.cpp:493)
   by 0x52A2C26: XRE_InitEmbedding2 (nsEmbedFunctions.cpp:157)
   by 0x5F85B1C: mozilla::ipc::ScopedXREEmbed::Start() (ScopedXREEmbed.cpp:83)
   by 0x5F567DE: mozilla::dom::ContentProcess::Init() (ContentProcess.cpp:22)
   by 0x52A2FDA: XRE_InitChildProcess (nsEmbedFunctions.cpp:474)
   by 0x401A8C: main (MozillaRuntimeMain.cpp:48)

Comment 1

[Nathan Froyd [:froydnj]]

FWIW, it looks like ~StartupCache always expects StartupCache::Init to succeed.  That's not a good assumption.

Comment 2

[Julian Seward [:jseward]]

This is still present in current m-c, and observable on x86_64-linux,
using TEST_PATH=content/base/test/test_ipc_messagemanager_blob.html

Comment 3

[Julian Seward [:jseward]]

Any suggestions who can take this?  It's still alive and well in m-c and
I'd like to get rid of it.

Comment 4

[Nathan Froyd [:froydnj]]

Attachment: patch

This is one way to handle the problem: flag that the relevant tables have been initialized before checking them in WriteToDisk.

The other possibility is to initialize the tables in the constructor, rather than in Init.  I am not sure if that would be compatible with The Gecko Way.  Maybe it's OK because our hashtables are now infallible by default and we'd just throw during the constructor if the hashtable.Init() call failed?

Comment 5

[Michael Wu [:mwu]]

Comment on attachment 669965 [details] [diff] [review]
patch

Can we check mTable.IsInitialized() ?

Comment 6

[Nathan Froyd [:froydnj]]

Attachment: patch

(In reply to Michael Wu [:mwu] from comment #5)
> Can we check mTable.IsInitialized() ?

We certainly can; that's a much better idea.  How about this patch?

Comment 7

[Michael Wu [:mwu]]

Comment on attachment 670105 [details] [diff] [review]
patch

Much better. :)

Comment 8

[Nathan Froyd [:froydnj]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ac7e592b097c

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/ac7e592b097c