Closed Bug 95614 Opened 19 years ago Closed 19 years ago

parser crashes related to uninitialized iterators - Trunk [@ nsSharedBufferList::Position::Distance]

Categories

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

x86
Linux
defect
Not set
blocker

Tracking

()

VERIFIED FIXED

People

(Reporter: dbaron, Assigned: harishd)

References

Details

(Keywords: crash, smoketest, topcrash, Whiteboard: [fix in hand])

Crash Data

Attachments

(3 files)

Tor and I both saw parser crashes related to an uninitialized iterator in
CTextToken::Consume within a few minutes of starting to use today's build.  I'll
attach some snips of gdb output showing the problem.

Considering the frequency of the crash, this *may* need to be a smoketest
blocker, so marking as such at least until we have a better idea of how often it
happens.
I suspect bug 91437 as the cause.
it's a pretty bad one - smoketest blocker sounds fine. Had 6 of those crashes in
less than an hour.
btw.. i ran with the "old" patch in bug 91437 for 3 days without problems
(attachment id=45504)
Does it happen only on LINUX?
I'm very close to a fix. Will attach a patch soon.
Whiteboard: [fix in hand]
Talked to vidur and realized that NS_ENSURE_SUCCESS will also assert ( a lot! )
and therefore would be more annoying than being helpful. Rewriting that part.
The old patch (attachment id=45504) uses the NS_ENSURE_SUCCESS macro which will
assert. A failure result code from nsScanner::Peek() does not indicate an
exceptional condition, just that we've reached the end of the current buffer.
Hence, an assertion is incorrect and the macro shouldn't be used.

Also, the early return in nsScanner::ReadUntil in the old patch will avoid the
call to Eof() at the end of the method. The original notion was that the scanner
could be used in push (the general case in the browser) or pull (sometimes used
in standalone tests) modes. I'm not sure if the latter mode is still functional,
but the call to Eof() was a crucial component of making it work correctly.
I think there was another place that was very similar
(ConsumeMarkupDeclaration?) that should probably have the same fix...
>I think there was another place that was very similar
>(ConsumeMarkupDeclaration?) that should probably have the same fix...

I don't think this code ever gets executed. Even if it does the scanner changes
in patch v1.2 should be able avoid the problem.

*** Bug 95625 has been marked as a duplicate of this bug. ***
r=heikki, and sending to harishd
Assignee: bratell → harishd
Whiteboard: [fix in hand] → [fix in hand][Need sr= ]
sr=waterson. you are so money, baby.
Status: NEW → ASSIGNED
Whiteboard: [fix in hand][Need sr= ] → [fix in hand]
Marking FIXED.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
*** Bug 95672 has been marked as a duplicate of this bug. ***
Thanks for cleaning up my mess! 

I should have tested more and not just accepted other people's well intended 
advice.
*** Bug 95667 has been marked as a duplicate of this bug. ***
*** Bug 95701 has been marked as a duplicate of this bug. ***
*** Bug 95708 has been marked as a duplicate of this bug. ***
Keywords: crash, topcrash
Summary: parser crashes related to uninitialized iterators → parser crashes related to uninitialized iterators - Trunk [@ nsSharedBufferList::Position::Distance]
Adding crash, topcrash keywords and Trunk [@
nsSharedBufferList::Position::Distance] to summary for future reference.
QA Contact: bsharma → moied
dbaron, could you please verify this bug..?
Status: RESOLVED → VERIFIED
Crash Signature: [@ nsSharedBufferList::Position::Distance]
You need to log in before you can comment on or make changes to this bug.