Uninitialised value use in nsDiskCacheMap::FlushRecords

RESOLVED FIXED in mozilla17

Status

()

Core
Networking: Cache
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jseward, Assigned: michal)

Tracking

Trunk
mozilla17
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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)

Updated

5 years ago
Assignee: nobody → michal.novotny
(Assignee)

Comment 1

5 years ago
Created attachment 618831 [details] [diff] [review]
fix

>    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+
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/5fe07a3a8458

Comment 5

5 years ago
https://hg.mozilla.org/mozilla-central/rev/5fe07a3a8458
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.