Closed Bug 472032 Opened 12 years ago Closed 12 years ago
[win64] sizeof(long) != sizeof(void*) assertion in ns
Script Security Manager .cpp
See URL. This assertion gets triggered because sizeof(long) = 4, sizeof(void*) = 8. See bug 119646 comment 9 to comment 20. It's guarding union SecurityLevel at http://mxr.mozilla.org/mozilla-central/source/caps/include/nsScriptSecurityManager.h?rev=9f497b1505d2&mark=149#144 This triggers on startup of any win64 build. I have no idea how the other win64 people manage to not see it... ###!!! ASSERTION: long and void* have different lengths on this platform. This may cause a security failure.: 'sizeof(long) == sizeof(void*)', file .../mozilla/src/caps/src/nsScriptSecurityManager.cpp, line 3220
According to CVS log (http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/caps/src&command=DIFF_FRAMESET&file=nsScriptSecurityManager.cpp&rev2=1.162&rev1=1.161), this was checked in bug 119646. I believe that this assertion code is not necessary.
insert of asserting, just use the type that's guaranteed to be sizeof(void*) instead. Thanks to timeless for helping tracking down the initial reason.
Comment on attachment 363527 [details] [diff] [review] use PRWord, not long, and drop assertion r/sr=dveditz Would it be better to leave the assertion and change it to sizeof(PRWord) instead? It seems to be the right thing to use here (jst and sicking agree) but I'm slightly concerned by the comment in prtypes.h saying it's deprecated. Updating the assertion might help if they stop maintaining it for future platforms. If you do leave the assertion change the text to end "This may cause a security failure with the SecurityLevel union."
carrying forward r/sr (In reply to comment #3) > (From update of attachment 363527 [details] [diff] [review]) > Would it be better to leave the assertion and change it to sizeof(PRWord) > instead? Done. > I'm slightly concerned by the comment in prtypes.h saying it's deprecated. Unfortunately, unless NSPR decides to provide a type that actually is guaranteed to be sizeof(void*), we're stuck with it. C99 got intptr_t, so having such a type must make sense to somebody :)
(In reply to comment #4) > Unfortunately, unless NSPR decides to provide a type that actually is > guaranteed to be sizeof(void*) See PRUptrdiff.
Comment on attachment 364282 [details] [diff] [review] address comment 3 [Checkin: Comment 6] http://hg.mozilla.org/mozilla-central/rev/04579855a2b7
Attachment #364282 - Attachment description: address comment 3 → address comment 3 [Checkin: Comment 5]
Attachment #364282 - Attachment description: address comment 3 [Checkin: Comment 5] → address comment 3 [Checkin: Comment 6]
(In reply to comment #5) > See PRUptrdiff. Ah well: reopen as needed.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Bah, sorry, I thought I had already attached this, then my internet access went down :( Here's a trivial version to use PRUptrdiff, if you think it's necessary? (Feel free to clear the requests out if not.) For what it's worth, I used PRWord because it's not a pointer difference (as in, intptr_t vs ptrdiff_t)
Comment on attachment 366299 [details] [diff] [review] PRUptrdiff >diff --git a/caps/include/nsScriptSecurityManager.h b/caps/include/nsScriptSecurityManager.h >+ PRUptrdiff level; >+ char* capability; Nit: alignment is not right. >+ NS_ASSERTION(sizeof(PRUptrdiff) == sizeof(void*), Make this a PR_STATIC_ASSERT.
You need to log in before you can comment on or make changes to this bug.