Re: pkg/39358

[Kernigh <xkernigh%netscape.net@localhost>]

The following reply was made to PR pkg/39358; it has been noted by GNATS.

From: Kernigh <xkernigh%netscape.net@localhost>
To: gnats-bugs%gnats.NetBSD.org@localhost
Cc: 
Subject: Re: pkg/39358
Date: Wed, 20 Aug 2008 18:59:14 +0000

 In PR 39538, I provided a patch-af for the www/firefox3 package, but I
 forgot of the error that it fixed. I recently built firefox3 again and
 now I know what the error was.
 
 My patch-af disables the WARNINGS_AS_ERRORS feature in
 netwerk/cookie/src/Makefile.in so that a particular compiler warning
 does not cause an error and stop the build. The compiler warning looks
 like
 
 .....nsVoidArray.h:193: warning: cast from 'char*' to
 'nsVoidArray::Impl*' increases required alignment of target type
 
 where ..... is the path to the nsVoidArray.h file. The full path is 
 /usr/pkgsrc/www/firefox3/work/mozilla/xpcom/glue/nsVoidArray.h
 
 Line 193 refers to a reinterpret_cast in this class declaration:
 
 // A zero-based array with a bit of automatic internal storage
 class NS_COM_GLUE nsAutoVoidArray : public nsVoidArray {
 public:
   nsAutoVoidArray();
 
   void ResetToAutoBuffer()
   {
     SetArray(reinterpret_cast<Impl*>(mAutoBuf), kAutoBufSize, 0, PR_FALSE,
              PR_TRUE);
   }
   
 protected:
   // The internal storage
   char mAutoBuf[sizeof(Impl) + (kAutoBufSize - 1) * sizeof(void*)];
 };
 
 Here, mAutoBuf is an array of char, and the cast is from char* to
 Impl*. [[I forgot how reinterpret_cast differs from (Impl*)mAutoBuf,
 also known as static_cast, so I asked the internet. The internet told me
 that in C++, a static_cast will adjust the pointer so that
 multiple-inheritance will work. A reinterpet_cast does not adjust the
 pointer, but it works between incompatible pointer types like char* and
 Impl*.]]
 
 .....nsVoidArray.h:193: warning: cast from 'char*' to
 'nsVoidArray::Impl*' increases required alignment of target type
 
 This warning message comes from the -Wcast-align flag (not -Wconversion
 as I stated before). The makefiles pass both flags to cc(1). The cc(1)
 manual page describes the -Wcast-align flag this way:
 
        -Wcast-align
            Warn whenever a pointer is cast such that the required alignment of
            the target is increased.  For example, warn if a "char *" is cast
            to an "int *" on machines where integers can only be accessed at
            two- or four-byte boundaries.
 
 nsVoidArray::Impl refers to the struct declared at line 135. The
 required alignment of a char* is one. Therefore, the required alignment
 of a struct (or class, same thing) on a powerpc machine must be greater
 than one. It might be two or four?
 
 The mAutoBuf declared at line 199 is the only field in class
 nsAutoVoidArray, which inherits from class nsVoidArray, which seems to
 have only these fields:
 
   Impl* mImpl;
 #if DEBUG_VOIDARRAY
   PRInt32 mMaxCount;
   PRInt32 mMaxSize;
   PRBool  mIsAuto;
 #endif
 
 Assuming that DEBUG_VOIDARRAY is off, then I guess that the class
 nsVoidArray will look like
 
 struct {
   Impl* mImpl;      // a 4-byte pointer
   char mAutoBuf[...];
 };
 
 So mAutoBuf is four bytes after the beginning of the struct. Thus I
 guess that mAutoBuf has enough alignment and that I may safely ignore
 the compiler warning. That is, my patch-af to disable
 WARNINGS_AS_ERRORS is a sufficient fix.
 
 I wonder how to actually solve the warning so that -Wcast-align does
 not show a warning. Meanwhile, I feel bad about appending this rant to
 a PR that should be about the asm code in xpcom.