Re-enable DrMemory test for ICOImageDecoderTests.parseAndDecodeByteByByte.

third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp

 /*
  * Copyright (c) 2008, 2009, Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 118 matching lines @@
 size_t ICOImageDecoder::decodeFrameCount()
 {
     decodeSize();
 
     // If decodeSize() fails, return the existing number of frames.  This way
     // if we get halfway through the image before decoding fails, we won't
     // suddenly start reporting that the image has zero frames.
     if (failed())
         return m_frameBufferCache.size();
 
+    // Check if there is enough data available to parse directory entries.
+    if (m_decodedOffset < sizeOfDirectory + m_dirEntries.size() * sizeOfDirectory)

[Peter Kasting, 2016/06/22 23:35:28]

Is the problem here as follows?:

* Someone calls decodeFrameCount() after we've received enough data for the
initial directory, but not for the subsequent directory entries
* So |m_dirEntries| has been resized, but the individual entries are not
initialized
* This means their offsets and sizes are zero, and thus the loop below claims
we've received all of them
* As we start receiving more data, the frame count then recognizes that we don't
have all the necessary data, and decreases

If so, I don't think this is the right fix.  There are a lot of functions in
this file that expect that if m_dirEntries contains an entry, that entry's
details are set.

I think the best way to fix this is:

* Change processDirectory() to take an outparam, numDirectoryEntries, and set it
to the desired number of directory entries.  Remove the m_dirEntries.resize()
call from that function.
* Change processDirectoryEntries() to take this value as an argument, check
against it in the top conditional (instead of using m_dirEntries.size()), and if
the conditional succeeds, do the m_dirEntries.resize() call there.
* Change decodeDirectory() to something like this:

    size_t numDirectoryEntries = m_dirEntries.size();
    if ((m_decodedOffset < sizeOfDirectory) &&
!processDirectory(&numDirectoryEntries))
        ...

    return (m_decodedOffset >= (sizeOfDirectory + (numDirectoryEntries *
sizeOfDirEntry))) || processDirectoryEntries(numDirectoryEntries);

This ensures that m_dirEntries stays empty until we can properly set its
members.  This should avoid the need to add a null constructor for
IconDirectoryEntry as well, as we'll never allocate one of those unless we are
going to immediately set all its members or else setFailed().

[aleksandar.stojiljkovic, 2016/06/23 10:11:29]

Yes, that's the problem.
Used your approach with the change that i introduced member variable to hold
parsed m_dirEntriesCount before resizing m_dirEntries.
This way, we avoid multiple calls to readUint16 in processDirectory when
m_data->size( is between sizeOfDirectory (6) and (sizeOfDirectory +
m_dirEntriesCount * sizeOfDirEntry).
Thanks.

+        return 0;
+
     // Length of sequence of completely received frames.
     for (size_t i = 0; i < m_dirEntries.size(); ++i) {
         const IconDirectoryEntry& dirEntry = m_dirEntries[i];
         if ((dirEntry.m_imageOffset + dirEntry.m_byteSize) > m_data->size())
             return i;
     }
     return m_dirEntries.size();
 }
 
 void ICOImageDecoder::setDataForPNGDecoderAtIndex(size_t index)
@@ skipping 176 matching lines @@
     ASSERT_WITH_SECURITY_IMPLICATION(index < m_dirEntries.size());
     const uint32_t imageOffset = m_dirEntries[index].m_imageOffset;
     if ((imageOffset > m_data->size()) || ((m_data->size() - imageOffset) < 4))
         return Unknown;
     char buffer[4];
     const char* data = m_fastReader.getConsecutiveData(imageOffset, 4, buffer);
     return strncmp(data, "\x89PNG", 4) ? BMP : PNG;
 }
 
 } // namespace blink