Closed Bug 746920 Opened 8 years ago Closed 7 years ago

Uninitialised value use in nsDiskCacheMap::FlushRecords


(Core :: Networking: Cache, defect)

Not set





(Reporter: jseward, Assigned: michal)



(1 file)

I've never seen this one before; perhaps it's new?

Unfortunately I don't have good STR.  I had to run the entire first
1/10th of mochitests on Valgrind in order to repro it, using the
command line:

for ((qq = 1; qq <= 1; qq++)); do (DISPLAY=:3.0 make -C ff-opt mochitest-plain EXTRA_TEST_ARGS="--this-chunk=$qq --total-chunks=10 --close-when-done --debugger=/home/sewardj/VgTRUNK/tchain/Inst/bin/valgrind --debugger-args='--smc-check=all-non-file --suppressions=/home/sewardj/MOZ/SUPPS/mochitest-mc.supp --error-limit=no --stats=no --trace-children=yes --child-silent-after-fork=yes "--trace-children-skip=/usr/bin/hg,/bin/rm,*/bin/certutil,*/bin/pk12util,*/bin/ssltunnel,*/bin/uname,*/bin/which,*/bin/ps,*/bin/grep" --tool=memcheck --track-origins=no'") 2>&1 | tee spew-mc-1--chunk-$qq-of-10-XXX-2 ; done

Note the use of --this-chunk and --total-chunks

The resulting complaint happens only at the end of the run (which is about 30
CPU minutes running Memcheck on a fast machine):

Conditional jump or move depends on uninitialised value(s)
   at 0x62FB818: nsDiskCacheMap::FlushRecords(bool) (nsDiskCacheMap.cpp:268)
   by 0x62FC297: nsDiskCacheMap::Close(bool) (nsDiskCacheMap.cpp:197)
   by 0x62F3F05: nsDiskCacheDevice::Shutdown_Private(bool) (nsDiskCacheDevice.cpp:485)
   by 0x62F3F3D: nsDiskCacheDevice::Shutdown() (nsDiskCacheDevice.cpp:462)
   by 0x62EFD89: nsCacheService::OnProfileShutdown(bool) (nsCacheService.cpp:2193)
   by 0x62F039D: nsCacheProfilePrefObserver::Observe(nsISupports*, char const*, unsigned short const*) (nsCacheService.cpp:420)
   by 0x6FCB639: nsObserverList::NotifyObservers(nsISupports*, char const*, unsigned short const*) (nsObserverList.cpp:130)
   by 0x6FCB921: nsObserverService::NotifyObservers(nsISupports*, char const*, unsigned short const*) (nsObserverService.cpp:182)
   by 0x6282756: nsXREDirProvider::DoShutdown() (nsXREDirProvider.cpp:852)
   by 0x627B2D3: ScopedXPCOMStartup::~ScopedXPCOMStartup() (nsAppRunner.cpp:1125)
   by 0x6280AC7: XREMain::XRE_main(int, char**, nsXREAppData const*) (nsAppRunner.cpp:3867)
   by 0x6280DC9: XRE_main (nsAppRunner.cpp:3921)

 Uninitialised value was created by a heap allocation
   at 0x4029C92: realloc (vg_replace_malloc.c:632)
   by 0x62FBB6F: nsDiskCacheMap::ShrinkRecords() (nsDiskCacheMap.cpp:383)
   by 0x62FBBF5: nsDiskCacheMap::Trim() (nsDiskCacheMap.cpp:225)
   by 0x62F414C: nsDiskCacheDevice::EvictEntries(char const*) (nsDiskCacheDevice.cpp:1003)
   by 0x62EEC3A: nsCacheService::EvictEntriesForClient(char const*, int) (nsCacheService.cpp:1394)
   by 0x7002A07: NS_InvokeByIndex_P (xptcinvoke_x86_64_unix.cpp:195)
   by 0x6B5166A: XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) (XPCWrappedNative.cpp:3114)
   by 0x6B579CF: XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) (XPCWrappedNativeJSOps.cpp:1549)
   by 0x73226D6: js::InvokeKernel(JSContext*, js::CallArgs, js::MaybeConstruct) (jscntxtinlines.h:314)
   by 0x731BA9B: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:2757)
   by 0x73220FE: js::RunScript(JSContext*, JSScript*, js::StackFrame*) (jsinterp.cpp:475)
   by 0x732279C: js::InvokeKernel(JSContext*, js::CallArgs, js::MaybeConstruct) (jsinterp.cpp:535)
Assignee: nobody → michal.novotny
Attached patch fixSplinter Review
>    if (newRecordsPerBucket < kMinRecordCount)
>        newRecordsPerBucket = kMinRecordCount;

newRecordsPerBucket should be compared against minimum per bucket count instead of minimum total count. The wrong minimum count causes growing the records in same cases and the newly allocated memory is not initialized.
Attachment #618831 - Flags: review?(bsmith)
Comment on attachment 618831 [details] [diff] [review]

Review of attachment 618831 [details] [diff] [review]:

Nick, I am not familiar with this code. Could you review the patch? If not, maybe Jason would be a better reviewer.
Attachment #618831 - Flags: review?(bsmith) → review?(hurley)
Comment on attachment 618831 [details] [diff] [review]

Review of attachment 618831 [details] [diff] [review]:

Looks good to me.
Attachment #618831 - Flags: review?(hurley) → review+
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.