659546 - clang warns about alignment requirement increase in nsID::Equals after bug 164580

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[(no longer active)]

../../../../dist/include/nsID.h:77:8: warning: cast from 'const PRUint32 *' (aka 'const unsigned int *') to 'PRUint64 *' (aka 'unsigned long long *') increases required alignment from 4 to 8
      [-Wcast-align]
      ((PRUint64*) &m0)[0] == ((PRUint64*) &other.m0)[0] &&
       ^~~~~~~~~~~~~~~
../../../../dist/include/nsID.h:77:32: warning: cast from 'const PRUint32 *' (aka 'const unsigned int *') to 'PRUint64 *' (aka 'unsigned long long *') increases required alignment from 4 to 8
      [-Wcast-align]
      ((PRUint64*) &m0)[0] == ((PRUint64*) &other.m0)[0] &&
                               ^~~~~~~~~~~~~~~~~~~~~
../../../../dist/include/nsID.h:78:8: warning: cast from 'const PRUint32 *' (aka 'const unsigned int *') to 'PRUint64 *' (aka 'unsigned long long *') increases required alignment from 4 to 8
      [-Wcast-align]
      ((PRUint64*) &m0)[1] == ((PRUint64*) &other.m0)[1];
       ^~~~~~~~~~~~~~~
../../../../dist/include/nsID.h:78:32: warning: cast from 'const PRUint32 *' (aka 'const unsigned int *') to 'PRUint64 *' (aka 'unsigned long long *') increases required alignment from 4 to 8
      [-Wcast-align]
      ((PRUint64*) &m0)[1] == ((PRUint64*) &other.m0)[1];
                               ^~~~~~~~~~~~~~~~~~~~~

We're basically relying on the correct knowledge of alignment requirements here, so we'd better silence the compiler.

Comment 1

[(no longer active)]

Attachment: Patch (v1)

Comment 2

[Justin Lebar (not reading bugmail)]

Wow, those are really pretty compile errors.  I like the aka's on the typedefs.  Man.

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #2)
> Wow, those are really pretty compile errors.  I like the aka's on the
> typedefs.  Man.

Reason #73 why clang is eventually going to kill off gcc.

Comment 4

[Benjamin Smedberg]

Do we know that nsID is actually 64-bit aligned? Given that none of the members require 64-bit alignment, I'm not sure that the struct is actually always going to be 64-bit aligned.

Comment 5

[Justin Lebar (not reading bugmail)]

It would probably be worth forcing 64-bit alignment, if only to be safe.  Do we have any macros wrapping the compiler directives for this?  I can't seem to find them.

Comment 6

[(no longer active)]

One possible way to force 64-bit alignment that I know of is to wrap its members into two unions, each with a 64-bit member.  But that would break the POD-ness of the type... :(

Comment 7

[Justin Lebar (not reading bugmail)]

See bug 660335 for forcing alignment of nsID.

Comment 8

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/15bfb9729ff3