ABR in nsPlainTextSerializer::AppendText; indexing -1 on array

VERIFIED FIXED

Status

()

Core
Layout
--
critical
VERIFIED FIXED
16 years ago
8 years ago

People

(Reporter: John Morrison, Assigned: peterv)

Tracking

Trunk
x86
Windows 2000
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

16 years ago
I was looking for something else, but happened to come up with this instead.

This is part of the checkin for bug 97687.

I'm getting an Array Bounds Read from Purify when running Mozilla. 
Specifically, I can trigger it by making a selection of part of the text in
the URLbar and cutting the selection to the clipboard. Purify info is
below. When I break in the debugger, |mTagStackIndex| is '0x00000000', so
|mTagStack[mTagStackIndex-1]| is Ooops! (Not sure why it isn't flagging it
as an ABW, since it is storing to that address, but...)


[E] ABR: Array bounds read in nsPlainTextSerializer::AppendText(nsIDOMText 
*,int,int,nsAString&) {3 occurrences}
    Reading 4 bytes from 0x09be7144 (4 bytes at 0x09be7144 illegal)
    Address 0x09be7144 is 4 bytes before the beginning of a 2000 byte block at 
0x09be7148
    Address 0x09be7144 points to a C++ new block in heap 0x02c70000
    Thread ID: 0x474
    Error location
    nsPlainTextSerializer::AppendText(nsIDOMText *,int,int,nsAString&) 
[nsPlainTextSerializer.cpp:230]
                                      nsAWritableString& aStr)
    {
      if ((mTagStack[mTagStackIndex-1] == eHTMLTag_noscript) &&
 =>       !(mFlags & nsIDocumentEncoder::OutputNoScriptContent)) {
        return NS_OK;
      }
    
    nsDocumentEncoder::SerializeNodeStart(nsIDOMNode *,int,int,nsAString&) 
[nsDocumentEncoder.cpp:315]
    nsDocumentEncoder::SerializeRangeToString(nsIDOMRange *,nsAString&) 
[nsDocumentEncoder.cpp:861]
    nsDocumentEncoder::EncodeToString(nsAString&) [nsDocumentEncoder.cpp:914]
    nsCopySupport::HTMLCopy(nsISelection *,nsIDocument *,short) 
[nsCopySupport.cpp:119]
    PresShell::DoCopy(void) [nsPresShell.cpp:4324]
    nsPlaintextEditor::Copy(void) [nsPlaintextEditor.cpp:1419]
    nsPlaintextEditor::Cut(void) [nsPlaintextEditor.cpp:1390]
    nsCutCommand::DoCommand(nsAString const&,nsISupports *) 
[nsEditorCommands.cpp:131]
    nsControllerCommandManager::DoCommand(nsAString const&,nsISupports *) 
[nsControllerCommandManager.cpp:199]
Allocation location
    new(UINT)      [msvcrt.DLL]
    nsPlainTextSerializer::nsPlainTextSerializer(void) 
[nsPlainTextSerializer.cpp:126]
          mStartedOutput = PR_FALSE;
        
          // initialize the tag stack to zero:
     =>   mTagStack = new nsHTMLTag[TagStackSize];
          mTagStackIndex = 0;
        
          // initialize the OL stack, where numbers for ordered lists are kept:
    NS_NewPlainTextSerializer(nsIContentSerializer * *) 
[nsPlainTextSerializer.cpp:87]
    CreatePlainTextSerializer [nsContentModule.cpp:308]
    nsComponentManager::CreateInstance(char const*,nsISupports *,nsID 
const&,void * *) [nsComponentManager.cpp:3146]
    nsCreateInstanceByContractID::()(nsID const&,void * *)const 
[nsComponentManager.cpp:164]
    nsDocumentEncoder::EncodeToString(nsAString&) [nsDocumentEncoder.cpp:888]
    nsCopySupport::HTMLCopy(nsISelection *,nsIDocument *,short) 
[nsCopySupport.cpp:119]
    PresShell::DoCopy(void) [nsPresShell.cpp:4324]
    nsPlaintextEditor::Copy(void) [nsPlaintextEditor.cpp:1419]
(Reporter)

Comment 1

16 years ago
Created attachment 51809 [details]
purify output (more readable form).
(Reporter)

Comment 2

16 years ago
need to resolve this before bug 97687 can (safely) go on the branch. (Maybe 
this is bogus, but I don't think so, since I can see the bad indexing in the
debugger without purify).
Blocks: 97687
Severity: normal → critical
Keywords: nsbranch
(Reporter)

Comment 3

16 years ago
(Um, duh. '==' is not '=', so ABR it is. Pretty unlikely that this would give a 
false positive match to the enum, so most harmless, I suppose).
(Assignee)

Comment 4

16 years ago
Created attachment 51811 [details] [diff] [review]
Fix
Comment on attachment 51811 [details] [diff] [review]
Fix

r=sicking
Attachment #51811 - Flags: review+
(Reporter)

Comment 6

16 years ago
(Man, I meant to assign this peterv ... thanks for the fix.)
Assignee: attinasi → peterv

Comment 7

16 years ago
Comment on attachment 51811 [details] [diff] [review]
Fix

sr=waterson
Attachment #51811 - Flags: superreview+
(Assignee)

Comment 8

16 years ago
Checked in. Thanks to all involved.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Let's get this on the branch. 
Keywords: nsbranch → nsbranch+
Um, reopening to make this onto PDT radar. 
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Argh, I should read my bugmail first. This went in on the branch with bug 97687,
closing again. Sorry for the noise...
Bugzilla needs natural language parser to act on my comments, darnit!
Status: REOPENED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED
(Reporter)

Comment 13

16 years ago
verified no more ABR in purify.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.