Closed Bug 325213 Opened 19 years ago Closed 15 years ago

Crashes when I start Firefox on Sparc64 [@ nsTemplateRule.operator=]

Categories

(Core :: XUL, defect)

1.8 Branch
Other
All
defect
Not set
major

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: ganesh.mozilla, Unassigned)

References

Details

(Keywords: 64bit, crash, Whiteboard: [needs retesting on Sparc64])

Crash Data

Attachments

(6 files)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111 Firefox/1.5 Build Identifier: When I try to launch 64-bit "firefox-1.5", the browser comes up and stays for a while. But after some time it crashed with a segmentation fault every time (Even if you don't do anything in the browser). Stack trace shows that "FatalSignalHandler" is called from "nsProfileLock.cpp" before it crashes. Reproducible: Always Actual Results: firefox-1.5 crashes with a segmentation fault. Expected Results: Browser should not be crashed.
Attached file Stack trace (deleted) —
Attaching the stack trace.
OS: Other → AIX
for reference, the fatalsignalhandler is called *after* it crashes precisely because it has *crashed*
Component: General → XP Toolkit/Widgets: XUL
Keywords: crash
Product: Firefox → Core
QA Contact: general → xptoolkit.xul
Summary: crashes with a segmentation fault when I start 64-bit "firefox-1.5" → crashes with a segmentation fault when I start 64-bit "firefox-1.5" [@ nsTemplateRule.operator=]
Version: unspecified → 1.8 Branch
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: 64bit
At the time of segmentation fault, the value of aSet is NULL (code snippet given below). So accessing its member causes the segmentation fault. The aSet (NULL) value is passed to this function from RecomputeBindings function in "nsTemplateRule.cpp". http://lxr.mozilla.org/seamonkey/source/content/xul/templates/src/nsRuleNetwork.h#451 nsAssignmentSet& operator=(const nsAssignmentSet& aSet) { NS_IF_RELEASE(mAssignments); mAssignments = aSet.mAssignments; NS_IF_ADDREF(mAssignments); return *this; } From RecomputeBindings function the NULL (aMatch->mInstantiation.mAssignments) is passed to the above function since, this function itself gets a NULL value from SynchronizeAll function in nsTemplateMatchSet.h. http://lxr.mozilla.org/seamonkey/source/content/xul/templates/src/nsTemplateRule.cpp#224 // Truncate the match's assignments to only include // assignments made via condition tests. We'll add back // assignments as they are recomputed. aMatch->mAssignments = aMatch->mInstantiation.mAssignments; In SynchronizeAll function, NULL is gettting from nsTemplateMatch* get() in nsTemplateMatchSet.h. This get () function returns the value depending on mInlineMatches.mCount and kMaxInlineMatches values. http://lxr.mozilla.org/seamonkey/source/content/xul/templates/src/nsTemplateMatchSet.h#269 nsTemplateMatch* get() const { return mSet->mStorageElements.mInlineMatches.mCount > PRUint32(kMaxInlineMatches) ? mTableEntry->mMatch : *mInlineEntry; } For InlineMatches structure, I can see some comments written like there may be some problem on 64 bit system. http://lxr.mozilla.org/seamonkey/source/content/xul/templates/src/nsTemplateMatchSet.h#221 Since we are building "64-bit browser", I suspect this comment is related with this problem.
With the comments written at http://lxr.mozilla.org/seamonkey/source/content/xul/templates/src/nsTemplateMatchSet.h#221, It seems to be the 'mCount' of 'InlineMatches' structure overlaps 'ops' member of PLDHashTable. So, I believe both of them should be in same size. In 32 bit compilation, both are 4 bytes and in 64 bit compilation 'mCount' is 4 bytes and 'ops' is 8 bytes. With the below change, I don't see the seg fault. Can somebody familiar with this code comment on this? struct InlineMatches { PRUint32 mCount; =======>PRUint64 mCount; nsTemplateMatch* mEntries[kMaxInlineMatches]; };
you can't change it like that as PRUint64 and friends require different operators. given that the code can't possibly use a 64bit number, i'd rather we didn't waste the space on 32bit platforms or the code instructions to properly pretend to deal w/ a 64bit number. I think the right way to do it is probably another union struct InlineMatches { union { PRUint32 mCount; void* unusedForAlignment; }; nsTemplateMatch* mEntries[kMaxInlineMatches]; }; making the union anonymous will probably earn me a compilation warning, so it should probably be named and properly accessed everywhere, but i don't feel like doing that.
Did the assertion at line 225 of nsTemplateMatchSet.cpp fire?
Yes, it hits the Assertion at line 225 of nsTemplateMatchSet.cpp. And also because of this, it fires the Assertion at line 1226 of nsXULContentBuilder.cpp.
Note that on trunk this file is getting removed altogether in bug 285631. So if we do anything here it's branch-only. Neil Deakin, any thoughts on how to best fix this on branch? I think we certainly want the fix for 1.8.1....
Depends on: 285631
Hi, Any plans of fixing this problem?
Flags: blocking1.8.1?
rupesh: does my suggestion work? if so, could you attach a patch that implements it?
Timeless, I have tried your suggession with anonymous union, but still seeing the same seg fault.
(In reply to comment #11) > Timeless, I have tried your suggession with anonymous union, but still seeing > the same seg fault. > Rupesh - can you verify the memory layout with timeless' suggestion? His idea should work - forcing things to be 64 bit on 64 bit platforms, but 32bit on 32bit platforms. Are you sure it is the same crash?
Not going to block 1.8.1 for this, but we'll happily consider a baked-on-trunk patch.
Flags: blocking1.8.1? → blocking1.8.1-
It fires assertion here at 225 of nsTemplateMatchSet.cpp on sparc64, but does not crash (firefox 2.0.0.1). Live bookmarks and some other things just do not work because of that.
Attached file Output compiled with --enable-debug (deleted) —
Attached file Configure log (deleted) —
Now rebuilding the it with the change timeless suggested. Will let you know as soon as it finishes.
Severity: critical → major
OS: AIX → All
Tested timeless' suggestion -- does not fix it. The patch i'm attaching fixes for us some major problems on sparc64 (e.g. live bookmarks). This was a long-time problem (appeared since 1.5, i suppose). It is not reproducible for all 64bit architectures (tried on alpha and amd64); so i'm assuming the problem might be mixing 32/64-bit access (usually works on little-endian; but fails on big-endian). So i didn't touch other archs (who touch things which just work). This should be common for all architectures; however i've tried everything under OpenBSD only. So if you are against committing it, please just surround it with extra __OpenBSD__ ifdefs, because we are using this in our ports-tree; would be nice to shrink our patches with every release.
Comment on attachment 255760 [details] [diff] [review] patch-content_xul_templates_src_nsTemplateMatchSet_h stop this. please post enough make foo.i output to show *why* my suggestion doesn't work. you're free to zip the foo.i's together and attach a single zip.
Attachment #255760 - Flags: review-
Sure. Will do.
timeless, sorry if that's too less... Just let me know if/which ones do you need. The sparc64.. I don't have it. I'm asking a friend of mine so it's limited for me too.
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
Martynas, do newer versions of Firefox work for you (on Sparc64)? I see bug reports for Sparc64 other than startup crashes, so it's likely that it works now.
Summary: crashes with a segmentation fault when I start 64-bit "firefox-1.5" [@ nsTemplateRule.operator=] → Crashes when I start Firefox on Sparc64 [@ nsTemplateRule.operator=]
Whiteboard: [needs retesting on Sparc64]
New versions (firefox3+) work fine on sparc64 and don't need the nsTemplateMatchSet.h quirk.
Yay!
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → WORKSFORME
Crash Signature: [@ nsTemplateRule.operator=]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: