Closed Bug 54410 Opened 24 years ago Closed 24 years ago

UMR: TextRun::AddSegment

Categories

(Core :: DOM: Editor, defect, P2)

x86
Windows 2000
defect

Tracking

()

VERIFIED INVALID

People

(Reporter: hjtoi-bugzilla, Assigned: waterson)

Details

(Whiteboard: [rtm-][p:2])

I set deferred free queue length and treshold to 20000 and I got a bunch of new UMRs from Purify. Here is one: TextRun::AddSegment(int,int,int) [nsTextFrame.cpp:3575] mTotalNumChars += aNumChars; mBreaks[mNumSegments] = mTotalNumChars; => mSegments[mNumSegments].mIsWhitespace = aIsWhitespace; mTotalContentLen += aContentLen; mSegments[mNumSegments].mContentLen = PRUint32(mTotalContentLen); mNumSegments++; nsTextFrame::MeasureText(nsIPresContext *,nsHTMLReflowState const&,nsTextTransformer&,nsILineBreaker *,TextStyle::nsTextFrame&,TextReflowData::nsTextFrame&) [nsTextFrame.cpp:3770] nsTextFrame::Reflow(nsIPresContext *,nsHTMLReflowMetrics&,nsHTMLReflowState const&,UINT&) [nsTextFrame.cpp:4247] nsLineLayout::ReflowFrame(nsIFrame *,nsIFrame * *,UINT&,nsHTMLReflowMetrics *,int&) [nsLineLayout.cpp:919] nsBlockFrame::ReflowInlineFrame(nsBlockReflowState&,nsLineLayout&,nsLineBox *,nsIFrame *,BYTE *) [nsBlockFrame.cpp:4357] nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&,nsLineLayout&,nsLineBox *,int *,BYTE *,int,int) [nsBlockFrame.cpp:4241] nsBlockFrame::DoReflowInlineFramesMalloc(nsBlockReflowState&,nsLineBox *,int *,BYTE *,int,int) [nsBlockFrame.cpp:4154] nsBlockFrame::ReflowInlineFrames(nsBlockReflowState&,nsLineBox *,int *,int,int) [nsBlockFrame.cpp:4115] nsBlockFrame::ReflowLine(nsBlockReflowState&,nsLineBox *,int *,int) [nsBlockFrame.cpp:3256] nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) [nsBlockFrame.cpp:2945]
setting to m19 for performance/stability work. anthonyd
Assignee: beppe → anthonyd
Target Milestone: --- → M19
adding mjudge to cc anthony, you might want to connect with Heikki and look at the purify run, otherwise you and mike can dig into it
Keywords: correctness
Priority: P3 → P2
nominate
Keywords: rtm
Whiteboard: [rtm+][p:2]
Okay, it looks like I might have been premature in accepting this bug, as text run is layout heay stuff. BUT, I will work on it anyway. What I need from Heikki is what do I do to get these UMR's? Where can I get a license of Purify (I don't have Purify)? Thanks, anthonyd
OS: Windows NT → Windows 2000
Whiteboard: [rtm+][p:2] → [rtm+][p:2][NEED INFO]
I set the queue length and treshold to 20000 in Purify options. I just started Mozilla, waited for the start page & sidebar to load up and then quit. I guess I sould use some setting to get a bigger stack trace so it would be easier to see where we fail to initialize the variable... To get Purify I think you need your manager's approval and then it needs to be ordered somehow. I do not know the details on how to order stuff like this (I am new here). For me and jst it "just worked", nisheeth@netscape.com and our administrator arranged it for us. I don't think there should be any problems in getting Purify, though. It is a pretty essential tool. It is not available for Linux, though, if I remember correctly. To get started now, I think it is possible to create a temporary license when you install Purify. You could work with that until you get a real license. You might be able to download a copy from Rational, or maybe install it from somebody else's discs. The actual license can only be activated by Rational.
there is no Linux version of Purify. heikki: there should be a line under the stack trace that gives some identifying information about the variable that was uninitialized and where it came from. (there is on Solaris anyway)
okay, so all you did in purify was launch the app, wait for everything to load, and the close it? And you got UMR's in TextRun? The reason I ask is I'm wondering why this was assigned to editor? Sounds like this really really needs to goto layout (vidur or someone) if this wants to get fixed quickly. anthonyd
Right. The reason it went to editor was because I thought TextRun was the responsibility of the editor team. Of course the place that fails to initialize the variable might be somewhere else...
Anthony, please include the required information per the rtm checkin rules
Whiteboard: [rtm+][p:2][NEED INFO] → [rtm+ NEED INFO][p:2]
it doesnt matter, TextRun is the responsibilty of the layout team. who in the layout should this goto?
PDT marking rtm- unless there's a compelling user-visible reason to fix. Like a crash.
Whiteboard: [rtm+ NEED INFO][p:2] → [rtm-][p:2]
we can start by asking buster, since this is not rtm worthy, it doesn't have such a high priority buster -- any ideas as to who should get this?
Assignee: anthonyd → buster
Target Milestone: M19 → ---
uninitialized variables can have bad consequences that vary across platform or even machines with the same OS, and often these problems show up in optimized builds and not debug builds. Further, these problems can be difficult to reproduce. So, if there's a trivial fix for this one, I'd go ahead and take the fix for rtm. If there's any question at all, then rtm-. Pushing over to waterson, who looked at text run recently. Chris, if I'm smokin' banana leaves, send it back to me and I'll take a look.
Assignee: buster → waterson
It was actually nsTextRun, and I removed the class altogether, but that's ok ;-). bruce: what does this mean? - Is "aIsWhitespace" unitialized? If so, I don't see how it could be: it's a parameter to the routine. The two calls to AddSegment() have it set, directly. - Is "mNumSegments" uninitialized? I don't see how: mNumSegments is intialized in the ctor at line 3643 (unless your compiler isn't running the ctor?), and re-initialized at line 3921. There are no other uses of TextRun. - Is "mSegments[mNumSegments].mIsWhitespace" uninitialized? We're trying to initialize it *right now*: why would pfy think we're trying to read it? - Or is purify off-by-n in the line numbering? Lemme know if you have any insight...
I didn't report this and my build is broken from a bad pull last week. (I'll update tonight and build again.) Heikki: any reproduction steps on this or did it happen all the time?
I just did this once. I will run this again to see if I can learn something new.
Bruce, I do you mean this with the additional info about the variable (this appears before the stack trace on NT)? All in all I get about 1,500 instances of UMR in this place by simply starting the app. [W] UMR: Uninitialized memory read in TextRun::AddSegment(int,int,int) {661 occurrences} Reading 4 bytes from 0x001348d4 (4 bytes at 0x001348d4 uninitialized) Address 0x001348d4 points into a thread's stack Address 0x001348d4 is 272 bytes past the start of local variable 'textRun' in nsTextFrame::MeasureText(nsIPresContext *,nsHTMLReflowState const&,nsTextTransformer&,nsILineBreaker *,TextStyle::nsTextFrame&,TextReflowData::nsTextFrame&) Thread ID: 0x128 Error location TextRun::AddSegment(int,int,int) [nsTextFrame.cpp:3575]
Oh, I know why this is. We're using packed fields here (see the definition of SegmentData), and what's happening is it's loading an unitialized word so it can twiddle some of the bits. This is confusing Purify's UMR detection, because it thinks you're reading an unitialized word and then AND-ing and OR-ing with its bits.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → INVALID
verified in 9/29 build.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.