102613 - UMR: nsReadingIterator<WORD>::*(void)const UMR: Uninitialized memory read in nsScanner::ReadUntil(nsAString&,nsReadEndCondition const&,int)

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect, P4)

Description

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Using Mozilla under Purify, when I started up the browser and went to the bug 
101860 Purify reported the following "Uninitialized Memory Read":

[W] UMR: Uninitialized memory read in nsReadingIterator<WORD>::*(void)const {1 
occurrence}
        Reading 2 bytes from 0x0ecd1738 (2 bytes at 0x0ecd1738 uninitialized)
        Address 0x0ecd1738 is 8192 bytes into a 8194 byte block at 0x0eccf738
        Address 0x0ecd1738 points to a malloc'd block in heap 0x02720000
        Thread ID: 0x4f0
        Error location
            nsReadingIterator<WORD>::*(void)const [nsStringIterator.h:92]
            nsScanner::ReadUntil
(nsReadingIterator<WORD>&,nsReadingIterator<WORD>&,nsReadEndCondition 
const&,int) [nsScanner.cpp:1277]
                    }
                
                    ++current;
             =>     theChar = *current;
                  }
                
                  // If we are here, we didn't find any terminator in the 
string and
            CTextToken::Consume(WORD,nsScanner&,int) [nsHTMLTokens.cpp:553]
                  aScanner.EndReading(end);
                
                  while((NS_OK==result) && (!done)) {
             =>     result=aScanner.ReadUntil(start, end, theEndCondition, 
PR_FALSE);
                    if(NS_OK==result) {
                      result=aScanner.Peek(aChar);
                
            nsHTMLTokenizer::ConsumeText(CToken *&,nsScanner&) 
[nsHTMLTokenizer.cpp:936]
            nsHTMLTokenizer::ConsumeToken(nsScanner&,int&) 
[nsHTMLTokenizer.cpp:502]
            nsParser::Tokenize(int) [nsParser.cpp:2796]
            nsParser::ResumeParse(int,int) [nsParser.cpp:2081]
            nsParser::OnDataAvailable(nsIRequest *,nsISupports *,nsIInputStream 
*,UINT,UINT) [nsParser.cpp:2687]
            nsDocumentOpenInfo::OnDataAvailable(nsIRequest *,nsISupports 
*,nsIInputStream *,UINT,UINT) [nsURILoader.cpp:243]
            nsStreamListenerTee::OnDataAvailable(nsIRequest *,nsISupports 
*,nsIInputStream *,UINT,UINT) [nsStreamListenerTee.cpp:56]
        Allocation location
            malloc         [dbgheap.c:129]
            PR_Malloc      [prmem.c:54]
            nsMemoryImpl::Alloc(UINT) [nsMemoryImpl.cpp:305]
            nsMemory::Alloc(UINT) [nsMemoryImpl.cpp:541]
            nsScanner::Append(char const*,UINT) [nsScanner.cpp:320]
            ParserWriteFunc [nsParser.cpp:2627]
            nsInputStreamTee::WriteSegmentFun(nsIInputStream *,void *,char 
const*,UINT,UINT,UINT *) [nsInputStreamTee.cpp:81]
            nsPipe::nsPipeInputStream::ReadSegments((*)(nsIInputStream *,void 
*,char const*,UINT,UINT,UINT *),void *,UINT,UINT *) [nsPipe2.cpp:411]
            nsInputStreamTee::ReadSegments((*)(nsIInputStream *,void *,char 
const*,UINT,UINT,UINT *),void *,UINT,UINT *) [nsInputStreamTee.cpp:137]
            nsParser::OnDataAvailable(nsIRequest *,nsISupports *,nsIInputStream 
*,UINT,UINT) [nsParser.cpp:2682]

Comment 1

[harishd]

--> 0.9.9

Comment 2

[Heikki Toivonen (remove -bugzilla when emailing directly)]

*** Bug 133432 has been marked as a duplicate of this bug. ***

Comment 3

[[e-mail] (gone - use [e-mail] instead)]

I see this on the trunk right now as well:

 [W] UMR: Uninitialized memory read in nsReadingIterator<WORD>::*(void)const {1
occurrence}
        Reading 2 bytes from 0x10436128 (2 bytes at 0x10436128 uninitialized)
        Address 0x10436128 is 8192 bytes into a 8194 byte block at 0x10434128
        Address 0x10436128 points to a malloc'd block in heap 0x02770000
        Thread ID: 0x518
        Error location
        nsReadingIterator<WORD>::*(void)const [nsStringIterator.h:96]
              CharT
              operator*() const
                {
     =>           return *get();
                }
        
            #if 0
       
nsScanner::ReadUntil(nsReadingIterator<WORD>&,nsReadingIterator<WORD>&,nsReadEndCondition
const&,int) [nsScanner.cpp:1296]
                }
            
                ++current;
         =>     theChar = *current;
              }
            
              // If we are here, we didn't find any terminator in the string and
        CTextToken::Consume(WORD,nsScanner&,int) [nsHTMLTokens.cpp:541]
        nsHTMLTokenizer::ConsumeText(CToken *&,nsScanner&) [nsHTMLTokenizer.cpp:931]
        nsHTMLTokenizer::ConsumeToken(nsScanner&,int&) [nsHTMLTokenizer.cpp:514]
        nsParser::Tokenize(int) [nsParser.cpp:2527]
        nsParser::ResumeParse(int,int,int) [nsParser.cpp:1751]
        nsParser::OnDataAvailable(nsIRequest *,nsISupports *,nsIInputStream
*,UINT,UINT) [nsParser.cpp:2386]
        nsDocumentOpenInfo::OnDataAvailable(nsIRequest *,nsISupports
*,nsIInputStream *,UINT,UINT) [nsURILoader.cpp:244]
        nsHttpChannel::OnDataAvailable(nsIRequest *,nsISupports *,nsIInputStream
*,UINT,UINT) [nsHttpChannel.cpp:3027]
    Allocation location
        malloc         [dbgheap.c:129]
        PR_Malloc      [prmem.c:474]
        nsMemoryImpl::Alloc(UINT) [nsMemoryImpl.cpp:320]
        nsMemory::Alloc(UINT) [nsMemory.cpp:75]
        nsScanner::Append(char const*,UINT) [nsScanner.cpp:335]
        ParserWriteFunc [nsParser.cpp:2324]
        nsPipe::nsPipeInputStream::ReadSegments((*)(nsIInputStream *,void *,char
const*,UINT,UINT,UINT *),void *,UINT,UINT *) [nsPipe2.cpp:419]
        nsParser::OnDataAvailable(nsIRequest *,nsISupports *,nsIInputStream
*,UINT,UINT) [nsParser.cpp:2381]
        nsDocumentOpenInfo::OnDataAvailable(nsIRequest *,nsISupports
*,nsIInputStream *,UINT,UINT) [nsURILoader.cpp:244]
        nsHttpChannel::OnDataAvailable(nsIRequest *,nsISupports *,nsIInputStream
*,UINT,UINT) [nsHttpChannel.cpp:3027]

Comment 4

[timeless]

trunk from last week...
    [W] UMR: Uninitialized memory read in
nsScanner::ReadUntil(nsAString&,nsReadEndCondition const&,int) {2 occurrences}
        Reading 2 bytes from 0x08e68f48 (2 bytes at 0x08e68f48 uninitialized)
        Address 0x08e68f48 is 23320 bytes into a 23324 byte block at 0x08e63430
        Address 0x08e68f48 points to a HeapAlloc'd block in heap 0x00360000
        Thread ID: 0x60c
        Error location
        nsScanner::ReadUntil(nsAString&,nsReadEndCondition const&,int)+0x1b7
[r:\mozilla\htmlparser\src\nsscanner.cpp:1185 ip=0x04c83ab0]
            // Check if all bits are in the required area
            if(!(theChar & aEndCondition.mFilter)) {
              // They were. Do a thorough check.
        
              setcurrent = setstart;
              while (*setcurrent) {
                if (*setcurrent == theChar) {
                  goto found;
                }
                ++setcurrent;
              }
                }
            
                ++current;
         =>     theChar = *current;
              }
            
              // If we are here, we didn't find any terminator in the string and
              // current = mEndPosition
              SetPosition(current);
              AppendUnicodeTo(origin, current, aString);
              return Eof();
            
            found:
              if(addTerminal)
                ++current;
              AppendUnicodeTo(origin, current, aString);
              SetPosition(current);
            
        ConsumeAttributeValueText+0x6f
[r:\mozilla\htmlparser\src\nshtmltokens.cpp:1619 ip=0x04c74b1b]
        CAttributeToken::Consume(WORD,nsScanner&,int)+0x44d
[r:\mozilla\htmlparser\src\nshtmltokens.cpp:1809 ip=0x04c75513]
        nsHTMLTokenizer::ConsumeAttributes(WORD,CToken *,nsScanner&)+0x12b
[r:\mozilla\htmlparser\src\nshtmltokenizer.cpp:634 ip=0x04c48eca]
        nsHTMLTokenizer::ConsumeStartTag(WORD,CToken *&,nsScanner&,int&)+0x2b4
[r:\mozilla\htmlparser\src\nshtmltokenizer.cpp:725 ip=0x04c4a4f6]
        nsHTMLTokenizer::ConsumeTag(WORD,CToken *&,nsScanner&,int&)+0x12f
[r:\mozilla\htmlparser\src\nshtmltokenizer.cpp:599 ip=0x04c48a61]
        nsHTMLTokenizer::ConsumeToken(nsScanner&,int&)+0xe1
[r:\mozilla\htmlparser\src\nshtmltokenizer.cpp:511 ip=0x04c48739]
        nsParser::Tokenize(int)+0x21b
[r:\mozilla\htmlparser\src\nsparser.cpp:2564 ip=0x04c7a8ea]
        nsParser::Tokenize(int)+0x15e
[r:\mozilla\htmlparser\src\nsparser.cpp:2553 ip=0x04c7a82d]
        nsParser::ResumeParse(int,int,int)+0x1fc
[r:\mozilla\htmlparser\src\nsparser.cpp:1760 ip=0x04c7c109]
    Allocation location
        HeapAlloc+0xc        [C:\WINDOWS\System32\KERNEL32.dll ip=0x67e633c8]
        nsScannerBufferList::AllocBuffer(UINT)+0x28
[r:\mozilla\htmlparser\src\nsscannerstring.cpp:74 ip=0x04c41418]
        nsScanner::Append(char const*,UINT)+0xaa
[r:\mozilla\htmlparser\src\nsscanner.cpp:339 ip=0x04c81c82]
        ParserWriteFunc+0x962 [r:\mozilla\htmlparser\src\nsparser.cpp:2364
ip=0x04c7d2a8]
        nsByteArrayInputStream::ReadSegments((*)(nsIInputStream *,void *,char
const*,UINT,UINT,UINT *),void *,UINT,UINT *)+0xcc
[r:\mozilla\xpcom\io\nsbytearrayinputstream.cpp:101 ip=0x0182dae7]
        nsParser::OnDataAvailable(nsIRequest *,nsISupports *,nsIInputStream
*,UINT,UINT)+0x23d [r:\mozilla\htmlparser\src\nsparser.cpp:2421 ip=0x04c7d531]
        nsDocumentOpenInfo::OnDataAvailable(nsIRequest *,nsISupports
*,nsIInputStream *,UINT,UINT)+0x62
[r:\mozilla\uriloader\base\nsuriloader.cpp:343 ip=0x04e32d9d]
        nsHTTPCompressConv::do_OnDataAvailable(nsIRequest *,nsISupports
*,UINT,char *,UINT)+0x1c9
[r:\mozilla\netwerk\streamconv\converters\nshttpcompressconv.cpp:368 ip=0x04019598]
        nsHTTPCompressConv::OnDataAvailable(nsIRequest *,nsISupports
*,nsIInputStream *,UINT,UINT)+0x930
[r:\mozilla\netwerk\streamconv\converters\nshttpcompressconv.cpp:304 ip=0x04019f5a]
        nsStreamListenerTee::OnDataAvailable(nsIRequest *,nsISupports
*,nsIInputStream *,UINT,UINT)+0x285
[r:\mozilla\netwerk\base\src\nsstreamlistenertee.cpp:97 ip=0x03fdae12]

Comment 5

[timeless]

Attachment: only check current if we're going to use it

this double checks current on the first pass. but it should avoid the umr on
the boundary condition.

Comment 6

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Comment on attachment 147386 [details] [diff] [review]
only check current if we're going to use it

r=heikki

I am no longer doing sr's so please ask someone else for that.

Comment 7

[Darin Fisher]

Comment on attachment 147386 [details] [diff] [review]
only check current if we're going to use it

this patch is fine, but it seems like it could be made better since it should
not be necessary to call Peek anymore.	however, care would then need to be
taken to check for EOF properly.

sr=darin

Comment 8

[timeless]

mozilla/parser/htmlparser/src/nsScanner.cpp 	3.129