[win64] sizeof(long) != sizeof(void*) assertion in nsScriptSecurityManager.cpp

RESOLVED FIXED in mozilla1.9.2a1

Status

()

RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Mook, Assigned: Mook)

Tracking

Trunk
mozilla1.9.2a1
x86_64
Windows XP
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

10 years ago
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
(Assignee)

Comment 2

10 years ago
Created attachment 363527 [details] [diff] [review]
use PRWord, not long, and drop assertion

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.
Attachment #363527 - Flags: superreview?(dveditz)
Attachment #363527 - Flags: review?(dveditz)
Attachment #363527 - Flags: superreview?(dveditz)
Attachment #363527 - Flags: superreview+
Attachment #363527 - Flags: review?(dveditz)
Attachment #363527 - Flags: review+
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."
(Assignee)

Comment 4

10 years ago
Created attachment 364282 [details] [diff] [review]
address comment 3
[Checkin: Comment 6]

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 :)
Assignee: dveditz → mook.moz+mozbz
Attachment #363527 - Attachment is obsolete: true
Attachment #364282 - Flags: superreview+
Attachment #364282 - Flags: review+
(Assignee)

Updated

10 years ago
Keywords: checkin-needed
(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
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
(Assignee)

Comment 8

10 years ago
Created attachment 366299 [details] [diff] [review]
PRUptrdiff

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)
Attachment #366299 - Flags: superreview?(peterv)
Attachment #366299 - Flags: review?(peterv)
Attachment #366299 - Flags: superreview?(peterv)
Attachment #366299 - Flags: superreview+
Attachment #366299 - Flags: review?(peterv)
Attachment #366299 - Flags: review+
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.