Lots of assertion with SPARC after s/PRBool/bool/

RESOLVED FIXED in mozilla10

Status

()

Core
XPCOM
--
blocker
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Ginn Chen, Assigned: Ginn Chen)

Tracking

Trunk
mozilla10
Sun
Solaris
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
I got lots of assertion on SPARC after Bug 675553.

Most of the assertions are:

###!!! ASSERTION: null ptr: 'aOldTarget != nsnull', file /export/home/ginn/work/mozilla-central/rdf/base/src/nsInMemoryDataSource.cpp, line 1576
###!!! ASSERTION: null ptr: 'aOldTarget != nsnull', file /export/home/ginn/work/mozilla-central/rdf/base/src/nsInMemoryDataSource.cpp, line 1576
************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "Component returned failure code: 0x80004003 (NS_ERROR_INVALID_POINTER) [nsIRDFDataSource.Change]"  nsresult: "0x80004003 (NS_ERROR_INVALID_POINTER)"  location: "JS frame :: file:///export/home/ginn/work/mozilla-central/debug/dist/bin/components/nsHandlerService.js :: HS__setTarget :: line 1265"  data: no]
************************************************************

And some others like
###!!! ASSERTION: Oops!  You're asking for a weak reference to an object that doesn't support that.: 'factoryPtr', file /export/home/ginn/work/mozilla-central/debug/xpcom/build/nsWeakReference.cpp, line 109
JavaScript error: , line 0: uncaught exception: [Exception... "Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]"  nsresult: "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)"  location: "JS frame :: resource://gre/modules/XPCOMUtils.jsm :: XPCU_serviceLambda :: line 232"  data: no]

or

###!!! ASSERTION: Won't check if this is chrome, you want to set aWantsUntrusted to PR_FALSE or make the aWantsUntrusted explicit by making aOptionalArgc non-zero.: '!aWantsUntrusted || aOptionalArgc > 1', file /export/home/ginn/work/mozilla-central/content/base/src/nsGenericElement.cpp, line 1078
JavaScript strict warning: file:///export/home/ginn/work/mozilla-central/debug/dist/bin/components/nsSessionStore.js, line 4360: reference to undefined property aBrowser.__SS_restoreState
(Assignee)

Comment 1

6 years ago
Looks like we should revert the change for
xpcom/reflect/xptcall/src/md/unix
at least for big endian machines.

Or perhaps s/PRBool/uint32

mwu, suggestions?
Severity: normal → blocker
Hardware: x86 → Sun
Component: General → XPCOM
QA Contact: general → xpcom
Version: unspecified → Trunk

Comment 2

6 years ago
I don't know which files are built for your platform, but if it's anything like xptcinvoke_sparc_solaris.cpp, I suspect that 

case nsXPTType::T_BOOL   : *((bool*) l_d) = l_s->val.b;           break;

should be changed to

case nsXPTType::T_BOOL   : *((uint32*) l_d) = l_s->val.b;           break;

in order to match the handling of other data types that are less than 4 bytes.
(Assignee)

Comment 3

6 years ago
Created attachment 567320 [details] [diff] [review]
patch

I hope I didn't miss a file.

Fix xptcstubs_sparcv9_solaris.cpp according to xptcstubs_sparc64_openbsd.cpp
Assignee: nobody → ginn.chen
Status: NEW → ASSIGNED
Attachment #567320 - Flags: review?(mwu)

Comment 4

6 years ago
Comment on attachment 567320 [details] [diff] [review]
patch

I'm not comfortable enough with this code to review.

However, in the xptcstubs_netbsd_m68k.cpp case, I think we may be able to get away with a bool there as long as the + 3 offset remains there. This would probably avoid a cast and eliminate an instruction or two.
Attachment #567320 - Flags: review?(mwu) → review?(benjamin)
Comment on attachment 567320 [details] [diff] [review]
patch

glandium said he would look at this, since I don't know the assembly and he knows at least some of the architectures here.
Attachment #567320 - Flags: review?(benjamin) → review?(mh+mozilla)
Comment on attachment 567320 [details] [diff] [review]
patch

Review of attachment 567320 [details] [diff] [review]:
-----------------------------------------------------------------

::: xpcom/reflect/xptcall/src/md/unix/xptcstubs_linux_m68k.cpp
@@ +94,5 @@
>              case nsXPTType::T_U32    : dp->val.u32 = *((PRUint32*)ap);       break;
>              case nsXPTType::T_U64    : dp->val.u64 = *((PRUint64*)ap); ap++; break;
>              case nsXPTType::T_FLOAT  : dp->val.f   = *((float*)   ap);       break;
>              case nsXPTType::T_DOUBLE : dp->val.d   = *((double*)  ap); ap++; break;
> +            case nsXPTType::T_BOOL   : dp->val.b   = *(((char*)   ap) + 3);  break;

I understand this is the style in this file, but it might be better to do like on other architectures, using *((PRUint32*)ap)

Other than that, r=me.
Attachment #567320 - Flags: review?(mh+mozilla) → review+
(Assignee)

Comment 7

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ee987590740
https://hg.mozilla.org/mozilla-central/rev/2ee987590740
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
You need to log in before you can comment on or make changes to this bug.