Closed
Bug 149436
Opened 22 years ago
Closed 22 years ago
Mozilla crashed on a Tru64 UNIX machine
Categories
(Core :: Layout, defect, P2)
Tracking
()
VERIFIED
FIXED
mozilla1.0.1
People
(Reporter: shanmu, Assigned: karnaze)
References
Details
(Keywords: crash)
Attachments
(1 file)
708 bytes,
patch
|
karnaze
:
review+
kinmoz
:
superreview+
chofmann
:
approval+
|
Details | Diff | Splinter Review |
Mozilla 1.0 crahsed while trying to access www.tamilbeat.com I loaded this page and clicked on the second item on the right. Mozilla crashed. It didn't crash if when I clicked the first item (or any oter item). Upon debugging I found that the following. In ./layout/html/table/src/nsTableFrame.cpp the line 5918 reads like this for (PRInt32 rowX = info.rowIndex; rowX <= cellEndRowIndex; rowX++) { nsTableRowFrame* rowFrame = (rowX == info.rowIndex) ? info.topRow :rowFrame->GetNextRow(); .... } Here rowFrame's scope is inside the for loop. Tru64 UNIX uses the memory that is freed and there is no gaurantee that the address will still hold the same value after a while. So once mozilla steps out of this loop the memory will be used. So when the next time this value is accessed it get the garbage value and crashes. To avoid this it would be safe to declare rowFrame outside of the loop. nsTableRowFrame* rowFrame; for (PRInt32 rowX = info.rowIndex; rowX <= cellEndRowIndex; rowX++) { rowFrame = (rowX == info.rowIndex) ? info.topRow : rowFrame->GetNextRow(); .... } I made these changes and it worked fine and didn't crash. I found the same exact code in another place in the same file. I will attach the fix soon.
Reporter | ||
Comment 1•22 years ago
|
||
Comment 2•22 years ago
|
||
-> NEW Reporter: Please ask for reviews for your patch (see www.mozilla.org/hacking)
Reporter | ||
Comment 3•22 years ago
|
||
Can I have a r= and sr= for this patch please.
Updated•22 years ago
|
QA Contact: petersen → moied
Updated•22 years ago
|
Priority: -- → P2
Assignee | ||
Comment 5•22 years ago
|
||
Comment on attachment 86501 [details] [diff] [review] Fix r=karnaze
Attachment #86501 -
Flags: review+
Comment on attachment 86501 [details] [diff] [review] Fix sr=kin@netscape.com
Attachment #86501 -
Flags: superreview+
Reporter | ||
Comment 7•22 years ago
|
||
I don't have permissions to commit my changes in to the repository. may I ask some one in this list to commit these changes.
Assignee | ||
Comment 8•22 years ago
|
||
The patch is in.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 9•22 years ago
|
||
This commit have added two "may be used uninitialized" warnings: +layout/html/table/src/nsTableFrame.cpp:5917 + `class nsTableRowFrame * rowFrame' might be used uninitialized in this function + +layout/html/table/src/nsTableFrame.cpp:5950 + `class nsTableRowFrame * rowFrame' might be used uninitialized in this function See also bug 59652 and bug 59675
Assignee | ||
Comment 10•22 years ago
|
||
shanmu? - this is your patch
Reporter | ||
Comment 11•22 years ago
|
||
These two warnings always showed up. I just had a glimpse of the build log from one of the clobber build (before the fix was checked in), where I see this. Warning 156: "nsTableFrame.cpp", line 5918 # The variable 'rowFrame' may not be initialized on all possible paths from start of the function. bleRowFrame* rowFrame = (rowX == info.rowIndex) ? info.topRow : rowFrame->GetNextRow(); ^^^^^^^^ Warning 156: "nsTableFrame.cpp", line 5950 # The variable 'rowFrame' may not be initialized on all possible paths from start of the function. bleRowFrame* rowFrame = (rowX == info.rowIndex) ? info.topRow : rowFrame->GetNextRow();
Comment 12•22 years ago
|
||
Yes, this has always shown up. If you look at the logic... yes it is "ok" because the first time thru the loop you don't access rowframe, only on subsequent times thru the loop. I had suggested a change to remove these warnings because in this loop this check is unnecessary (and wasteful). But never checked it in...
Comment 13•22 years ago
|
||
*** Bug 136769 has been marked as a duplicate of this bug. ***
Assignee | ||
Updated•22 years ago
|
Whiteboard: [FIXED_ON_TRUNK]
Target Milestone: --- → mozilla1.0.1
Assignee | ||
Updated•22 years ago
|
Updated•22 years ago
|
Comment 14•22 years ago
|
||
Comment on attachment 86501 [details] [diff] [review] Fix a=chofmann for the 1.01 branch. add fixed1.01 after checking in. thanks
Attachment #86501 -
Flags: approval+
Updated•22 years ago
|
Keywords: mozilla1.0.1 → mozilla1.0.1+
Updated•22 years ago
|
Keywords: mozilla1.0.1+
Comment 17•22 years ago
|
||
*** Bug 155661 has been marked as a duplicate of this bug. ***
Comment 18•22 years ago
|
||
*** Bug 159010 has been marked as a duplicate of this bug. ***
Comment 19•22 years ago
|
||
Verified with branch build 20020731 on winxp, adding KW:verified1.0.1
Status: RESOLVED → VERIFIED
Keywords: verified1.0.1
You need to log in
before you can comment on or make changes to this bug.
Description
•