Closed Bug 746920 Opened 8 years ago Closed 7 years ago

Uninitialised value use in nsDiskCacheMap::FlushRecords

Categories

(Core :: Networking: Cache, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: jseward, Assigned: michal)

Details

Attachments

(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]
fix

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]
fix

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

Looks good to me.
Attachment #618831 - Flags: review?(hurley) → review+
https://hg.mozilla.org/mozilla-central/rev/5fe07a3a8458
Status: NEW → RESOLVED
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.