Closed Bug 149436 Opened 22 years ago Closed 22 years ago

Mozilla crashed on a Tru64 UNIX machine

Categories

(Core :: Layout, defect, P2)

DEC
OSF/1
defect

Tracking

()

VERIFIED FIXED
mozilla1.0.1

People

(Reporter: shanmu, Assigned: karnaze)

References

Details

(Keywords: crash)

Attachments

(1 file)

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.
Attached patch FixSplinter Review
-> NEW

Reporter: Please ask for reviews for your patch (see www.mozilla.org/hacking)
Severity: major → critical
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: crash, review
Summary: Mozilla crashed on a Tru64 UNIX machine → Mozilla crashed on a Tru64 UNIX machine
Can I have a r= and sr= for this patch please.

QA Contact: petersen → moied
-> Karnaze
Assignee: attinasi → karnaze
Priority: -- → P2
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+
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.
The patch is in.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
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
shanmu? - this is your patch
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();


                                                                   
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...
*** Bug 136769 has been marked as a duplicate of this bug. ***
Whiteboard: [FIXED_ON_TRUNK]
Target Milestone: --- → mozilla1.0.1
Keywords: nsbeta1nsbeta1+
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+
added adt1.0.1+
Keywords: adt1.0.1adt1.0.1+
fixed1.0.1
Keywords: fixed1.0.1
Whiteboard: [FIXED_ON_TRUNK]
Keywords: mozilla1.0.1+
*** Bug 155661 has been marked as a duplicate of this bug. ***
*** Bug 159010 has been marked as a duplicate of this bug. ***
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.

Attachment

General

Created:
Updated:
Size: