Closed
Bug 325213
Opened 19 years ago
Closed 15 years ago
Crashes when I start Firefox on Sparc64 [@ nsTemplateRule.operator=]
Categories
(Core :: XUL, defect)
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.
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
Updated•19 years ago
|
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.
Comment 6•19 years ago
|
||
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.
Comment 8•19 years ago
|
||
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
Updated•19 years ago
|
Flags: blocking1.8.1?
Comment 10•19 years ago
|
||
rupesh: does my suggestion work? if so, could you attach a patch that implements it?
Comment 11•19 years ago
|
||
Timeless, I have tried your suggession with anonymous union, but still seeing the same seg fault.
Comment 12•19 years ago
|
||
(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?
Comment 13•18 years ago
|
||
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-
Comment 14•18 years ago
|
||
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.
Comment 15•18 years ago
|
||
Comment 16•18 years ago
|
||
Now rebuilding the it with the change timeless suggested. Will let you know as soon as it finishes.
Updated•18 years ago
|
Severity: critical → major
OS: AIX → All
Comment 17•18 years ago
|
||
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 18•18 years ago
|
||
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-
Comment 19•18 years ago
|
||
Sure. Will do.
Comment 20•18 years ago
|
||
Comment 21•18 years ago
|
||
Comment 22•18 years ago
|
||
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.
Comment 23•18 years ago
|
||
More .i's: http://www.altroot.org/i.tar.gz
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
Comment 24•15 years ago
|
||
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=]
Updated•15 years ago
|
Whiteboard: [needs retesting on Sparc64]
Comment 25•15 years ago
|
||
New versions (firefox3+) work fine on sparc64 and don't need the nsTemplateMatchSet.h quirk.
Comment 26•15 years ago
|
||
Yay!
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → WORKSFORME
Assignee | ||
Updated•13 years ago
|
Crash Signature: [@ nsTemplateRule.operator=]
You need to log in
before you can comment on or make changes to this bug.
Description
•