If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[valgrind] StartupCache passes uninitialized data to ZipWriter

NEW
Unassigned

Status

()

Core
XPCOM
4 years ago
a year ago

People

(Reporter: rillian, Unassigned)

Tracking

Trunk
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

Trying to reproduce bug 882589 (crash at cubeslam.com) I hit a valgrind error in the startup cache on x86_64 linux:

==22910== Thread 33:
==22910== Conditional jump or move depends on uninitialised value(s)
==22910==    at 0x9265E06: fill_window (deflate.c:1444)
==22910==    by 0x9266609: deflate_fast (deflate.c:1642)
==22910==    by 0x9265284: MOZ_Z_deflate (deflate.c:903)
==22910==    by 0x898DF13: nsDeflateConverter::OnDataAvailable(nsIRequest*, nsISupports*, nsIInputStream*, unsigned long, unsigned int) (nsDeflateConverter.cpp:129)
==22910==    by 0x898F1D6: nsZipDataStream::ProcessData(nsIRequest*, nsISupports*, char*, unsigned long, unsigned int) (nsZipDataStream.cpp:147)
==22910==    by 0x898F410: nsZipDataStream::ReadStream(nsIInputStream*) (nsZipDataStream.cpp:175)
==22910==    by 0x8994110: nsZipWriter::AddEntryStream(nsACString_internal const&, long, int, nsIInputStream*, bool, unsigned int) (nsZipWriter.cpp:502)
==22910==    by 0x8993D57: nsZipWriter::AddEntryStream(nsACString_internal const&, long, int, nsIInputStream*, bool) (nsZipWriter.cpp:446)
==22910==    by 0x73A0DE5: mozilla::scache::CacheCloseHelper(nsACString_internal const&, nsAutoPtr<mozilla::scache::CacheEntry>&, void*) (StartupCache.cpp:425)
==22910==    by 0x73A33D9: nsBaseHashtable<nsCStringHashKey, nsAutoPtr<mozilla::scache::CacheEntry>, mozilla::scache::CacheEntry*>::s_EnumStub(PLDHashTable*, PLDHashEntryHdr*, unsigned int, void*) (nsBaseHashtable.h:454)
==22910==    by 0x904270D: PL_DHashTableEnumerate (pldhash.cpp:714)
==22910==    by 0x73A2D90: nsBaseHashtable<nsCStringHashKey, nsAutoPtr<mozilla::scache::CacheEntry>, mozilla::scache::CacheEntry*>::Enumerate(PLDHashOperator (*)(nsACString_internal const&, nsAutoPtr<mozilla::scache::CacheEntry>&, void*), void*) (nsBaseHashtable.h:223)

Comment 1

4 years ago
Wasn't there a known valgrind issue at MOZ_Z_deflate?

If not, it's unlikely that this is a bug in the startup cache, but rather the client who stored the data. Can you maybe add a valgrind assert at http://mxr.mozilla.org/mozilla-central/source/startupcache/StartupCache.cpp#348 that the buffer is all initialized data?
Flags: needinfo?(giles)
(Reporter)

Comment 2

4 years ago
Created attachment 763814 [details] [diff] [review]
assert StarupCache::PutBuffer is initialized

Looks like your're right. I added the attached assert and the uninitialized warning from StartupCache still occurs...without any report from PutBuffer..

I can't find a bug for the MOZ_Z_deflate issue to mark this a duplicate.
Flags: needinfo?(giles)

Comment 3

4 years ago
gmail search pointed me back at bug 629209, see comment 2 and following.
(Reporter)

Comment 4

4 years ago
I see, thanks. So zlib say they fixed it, we might have been seeing it still occur, but no one followed up. I'll try --track-origins=yes and see if it says anything more.

Comment 5

a year ago
I encountered this same "Conditional jump or move depends on uninitialised value(s)" message when testing Tor Browser 6.5a1 under valgrind on an x86_64 Linux system (the browser code I used is ESR 45.2.0 plus some Tor Project patches). In addition, I sometimes see the following valgrind message:
==1062== Thread 29 StartupCache:
==1062== Use of uninitialised value of size 8
==1062==    at 0xCD6DDDD: crc32_little (crc32.c:264)
==1062==    by 0xCD6D98C: MOZ_Z_crc32 (crc32.c:222)
==1062==    by 0x9011FAA: nsZipDataStream::ProcessData(nsIRequest*, nsISupports*, char*, unsigned long, unsigned int) (nsZipDataStream.cpp:135)
==1062==    by 0x901231C: nsZipDataStream::ReadStream(nsIInputStream*) (nsZipDataStream.cpp:171)
==1062==    by 0x9016ABB: nsZipWriter::AddEntryStream(nsACString_internal const&, long, int, nsIInputStream*, bool, unsigned int) (nsZipWriter.cpp:497)
==1062==    by 0x901670F: nsZipWriter::AddEntryStream(nsACString_internal const&, long, int, nsIInputStream*, bool) (nsZipWriter.cpp:444)
==1062==    by 0xC0B8D89: mozilla::scache::CacheCloseHelper(nsACString_internal const&, mozilla::scache::CacheEntry const*, mozilla::scache::CacheWriteHolder const*) (StartupCache.cpp:425)
==1062==    by 0xC0B902C: mozilla::scache::StartupCache::WriteToDisk() (StartupCache.cpp:480)
==1062==    by 0xC0B92DD: mozilla::scache::StartupCache::ThreadedWrite(void*) (StartupCache.cpp:553)
==1062==    by 0x6300C64: _pt_root (ptthread.c:216)
==1062==    by 0x4E3F181: start_thread (pthread_create.c:312)
==1062==    by 0x5BFE47C: clone (clone.S:111)
You need to log in before you can comment on or make changes to this bug.