Last Comment Bug 693240 - Lots of assertion with SPARC after s/PRBool/bool/
: Lots of assertion with SPARC after s/PRBool/bool/
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: Trunk
: Sun Solaris
-- blocker (vote)
: mozilla10
Assigned To: Ginn Chen
: Nathan Froyd [:froydnj]
Depends on:
Blocks: 675553
  Show dependency treegraph
Reported: 2011-10-09 22:47 PDT by Ginn Chen
Modified: 2011-11-04 11:36 PDT (History)
5 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (24.59 KB, patch)
2011-10-16 00:29 PDT, Ginn Chen
mh+mozilla: review+
Details | Diff | Splinter Review

Description User image Ginn Chen 2011-10-09 22:47:53 PDT
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]


###!!! 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
Comment 1 User image Ginn Chen 2011-10-09 23:33:14 PDT
Looks like we should revert the change for
at least for big endian machines.

Or perhaps s/PRBool/uint32

mwu, suggestions?
Comment 2 User image Michael Wu [:mwu] 2011-10-10 09:27:43 PDT
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.
Comment 3 User image Ginn Chen 2011-10-16 00:29:21 PDT
Created attachment 567320 [details] [diff] [review]

I hope I didn't miss a file.

Fix xptcstubs_sparcv9_solaris.cpp according to xptcstubs_sparc64_openbsd.cpp
Comment 4 User image Michael Wu [:mwu] 2011-10-16 03:38:40 PDT
Comment on attachment 567320 [details] [diff] [review]

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.
Comment 5 User image Benjamin Smedberg [:bsmedberg] 2011-10-26 10:52:18 PDT
Comment on attachment 567320 [details] [diff] [review]

glandium said he would look at this, since I don't know the assembly and he knows at least some of the architectures here.
Comment 6 User image Mike Hommey [:glandium] 2011-11-01 02:54:33 PDT
Comment on attachment 567320 [details] [diff] [review]

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.
Comment 8 User image Matt Brubeck (:mbrubeck) 2011-11-04 11:36:48 PDT

Note You need to log in before you can comment on or make changes to this bug.