226609 - xpcom compilation failure under cygwin/mingw 3.3.1 & 3.3.3

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Gábor Lipták]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031122
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031122

make[4]: Entering directory `/cygdrive/c/tmp/mozilla/mozilla/xpcom/tests'
/cygdrive/c/tmp/mozilla/mozilla/build/cygwin-wrapper g++ -mno-cygwin   -fno-rtti
-fno-exceptions -Wall -Wconversion -Wpointer-arith -Wcast-align
-Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-long-long -pedantic
-mms-bitfields -pipe  -DDEBUG -D_DEBUG -DDEBUG_tmp -DTRACING -g -fexceptions -o
TestDeque.exe TestDeque.o          -L../../dist/bin -L../../dist/lib
-L../../dist/lib -lxpcom -L../../dist/lib -lnspr4 -lplc4 -lplds4  -lm     
TestDeque.o(.text$_ZN14nsDequeFunctorC2Ev+0x7): In function `_ZN8_DeallocclEPv':
c:/tmp/mozilla/mozilla/xpcom/tests/TestDeque.cpp: undefined reference to
`__imp___ZTV14nsDequeFunctor'
collect2: ld returned 1 exit status
make[4]: *** [TestDeque.exe] Error 1
make[4]: Leaving directory `/cygdrive/c/tmp/mozilla/mozilla/xpcom/tests'
make[3]: *** [libs] Error 2
make[3]: Leaving directory `/cygdrive/c/tmp/mozilla/mozilla/xpcom'
make[2]: *** [tier_2] Error 2
make[2]: Leaving directory `/cygdrive/c/tmp/mozilla/mozilla'
make[1]: *** [default] Error 2
make[1]: Leaving directory `/cygdrive/c/tmp/mozilla/mozilla'
make: *** [build] Error 2

bash-2.05b$ g++ --version
g++ (GCC) 3.3.1 (cygming special)

If this file is removed, than there are other files too which complain on
missing nsDequeFunctor

Reproducible: Always

Steps to Reproduce:
1. Download latest CVS source
2. Run make -f client.mk build
3. Observe build failure

Actual Results:  
Build failure

Expected Results:  
Build correctly

Comment 1

[Wan-Teh Chang]

Changed product to Browser:XPCOM.

Comment 2

[Marcus G. Daniels]

Attachment: disabling NS_COM for some private classes -- facilitates building on Cygwin

Comment 3

[Marcus G. Daniels]

I had this build problem too, and several related to it.  The preceding
attachment results in a working build of 1.7a for me.

The nsDequeFunctor is only passed as an argument, and so doesn't need to be
exported.  And as I understand it nsMsgLineBufferHandler and nsIOFileStream are
only used internally (the latter being obsolete from XPCOM).

Comment 4

[Marcus G. Daniels]

Attachment: another pair of NS_COM disables for optimized builds

Comment 5

[cls]

The second patch is for the same problem that we were discussing in bug 231195.
 I haven't seen the first error mentioned by I use mingw.org's gcc not cygwin's.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 140916 [details] [diff] [review]
another pair of NS_COM disables for optimized builds

The code being modified here doesn't exist anymore.

Comment 7

[cls]

*sigh* that should have been bug 231995.

Comment 8

[cls]

Attachment: Explicitly mark functions as inlined

Adding the 'inline' declaration to the inlined function causes gcc to properly
ignore the dllimport attribute of the functions.

Comment 9

[cls]

Attachment: inline vs dllimport testcase (testcase.tar.gz)

Here's a small testcase that I created which also demonstrates the problem that
we're seeing.  Regardless of whether the function is explicitly marked |inline|
or not, gcc appears to attempt to inline the function.	Even using -fno-inline
has no affect.

Comment 10

[cls]

FWIW, I'm seeing the nsDequeFunctor problem in a debug build when using mingw
gcc 3.3.3RC1 (release announced today).  My guess is that gcc is inlining or
somehow omitting that class when compiling nsDeque.cpp so the symbols never show
up in the import library that's built.  However, when someone outside of the
library attempts to use the class, gcc doesn't know that the class was
"optimized" away so it complains that the import library is missing the function.

Comment 11

[cls]

Attachment: Explicilty mark functions as inlined w/o NS_COM (checked in)

Per irc conversation with Darin, just mark inlined functions with |inline| and
remove NS_COM declarations.

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 142060 [details] [diff] [review]
Explicilty mark functions as inlined w/o NS_COM (checked in)

It's possible you could break something else with this, but sr=dbaron.

Comment 13

[cls]

Attachment: Fix leftover mingw 3.3.1 & 3.3.3 issues

Ok, this patch fixes the problems that I saw with mingw gcc 3.3.3 and finally
addresses some of the dllimport/dllexport issues that I saw with gcc 3.3.1 in
bug 217009.  

Instead of exporting all symbols in non-component libs, we rely upon
dllimport/dllexport declarations as MSVC does.	This meant explicitly creating
a NS_COM wrapped function for XPTC_InvokeByIndex since that function is written
entirely in asm for gcc and I don't know how to write the asm for the dllexport
stuff.	This also meant using NS_COM instead of NS_IMETHODIMP for
nsSupportsWeakReference::GetWeakReference.

Rather than removing the NS_COM_BASE declaration from nsMsgLineBufferHandler, I
made nsMsgLineBuffer inherit from nsMsgLineBufferHandler instead of from
nsByteHandler (which they both inherit from).  nsMsgLineBufferHandler only
contains a virtual function which nsMsgLineBufferHandler also contains.  This
change causes the vtable for nsMsgLineBufferHandler to be emitted as gcc
expects when importing that class.  

I couldn't figure out a way around the
lack-of-a-vtable-for-class-nsIOFileStream problem.  I just removed the
declaration and it worked.  That code (xpcom/obsolete) needs to die anyway.

Comment 14

[cls]

And this fixes the mingw debug build issue with nsDequeFunctor's table not being
emitted:

Index: xpcom/ds/nsDeque.h
===================================================================
RCS file: /cvsroot/mozilla/xpcom/ds/nsDeque.h,v
retrieving revision 3.26
diff -u -r3.26 nsDeque.h
--- xpcom/ds/nsDeque.h  15 Mar 2003 01:04:09 -0000      3.26
+++ xpcom/ds/nsDeque.h  28 Feb 2004 06:38:12 -0000
@@ -88,6 +88,7 @@
  
 class NS_COM nsDeque {
   friend class nsDequeIterator;
+  friend class nsDequeFunctor;
   public:
    nsDeque(nsDequeFunctor* aDeallocator);
   ~nsDeque();

Comment 15

[Gábor Lipták]

I still have the

../../dist/lib/libnkconv_s.a(nsStreamConverterService.o.b)(.text$_ZN14nsDequeFun
ctorC2Ev+0x8):nsStreamConverterService.cpp: undefined reference to `vtable for n
sDequeFunctor'

problem with latest CVS and #14 patch applied.

My build options are:

  --disable-accessibility
  --disable-activex
  --disable-activex-scripting
  --disable-installer
  --disable-tests
  --enable-crypto
  --enable-image-decoders=icon,png,gif,jpeg,bmp

(so I'm building debug and non-optimized?)

make[4]: Leaving directory `/cygdrive/c/mozilla/mozilla/netwerk/protocol'
make[4]: Entering directory `/cygdrive/c/mozilla/mozilla/netwerk/build'
rm -f necko.dll
/cygdrive/c/mozilla/mozilla/build/cygwin-wrapper g++ -mno-cygwin -shared -o
necko.dll  nsNetModule.o   ./module.res          -Wl,--whole-archive
../../dist/lib/libneckobase_s.a ../../dist/lib/libneckodns_s.a
../../dist/lib/libneckosocket_s.a ../../dist/lib/libneckocookie_s.a
../../dist/lib/libnkconv_s.a ../../dist/lib/libnkcnvts_s.a
../../dist/lib/libnkmime_s.a ../../dist/lib/libnkcache_s.a
../../dist/lib/libnkabout_s.a  ../../dist/lib/libnkfile_s.a 
../../dist/lib/libnkftp_s.a  ../../dist/lib/libnkhttp_s.a 
../../dist/lib/libnkjar_s.a  ../../dist/lib/libnkres_s.a  -Wl,--no-whole-archive
-L../../dist/bin -L../../dist/lib  ../../dist/lib/libunicharutil_s.a
-L../../dist/lib -lxpcom -L../../dist/bin -L../../dist/lib -lnspr4 -lplc4
-lplds4 -L../../dist/lib -lmozz   -lm  -lws2_32   
../../dist/lib/libnkconv_s.a(nsStreamConverterService.o.b)(.text$_ZN14nsDequeFunctorC2Ev+0x8):nsStreamConverterService.cpp:
undefined reference to `vtable for nsDequeFunctor'
collect2: ld returned 1 exit status
make[4]: *** [necko.dll] Error 1

Comment 16

[cls]

Whoops!  You are correct; adding the friend declaration doesn't fix the debug
builds.  I thought that I had backed the NS_COM change out of nsDeque.h when I
was testing the friend change but apparently I didn't.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 142494 [details] [diff] [review]
Fix leftover mingw 3.3.1 & 3.3.3 issues

>Index: xpcom/glue/nsWeakReference.cpp
>-NS_IMETHODIMP
>+NS_COM nsresult

Are NS_IMETHODIMP and NS_COM mutually exclusive?  They shouldn't be, I don't
think.

>Index: xpcom/reflect/xptcall/src/md/unix/xptcinvoke_gcc_x86_unix.cpp

>+#ifdef XP_WIN32
>+	".globl " SYMBOL_UNDERSCORE "_XPTC_InvokeByIndex\n\t"
>+	".type  " SYMBOL_UNDERSCORE "_XPTC_InvokeByIndex,@function\n"
>+	SYMBOL_UNDERSCORE "_XPTC_InvokeByIndex:\n\t"
>+#else
> 	".globl " SYMBOL_UNDERSCORE "XPTC_InvokeByIndex\n\t"
> 	".type  " SYMBOL_UNDERSCORE "XPTC_InvokeByIndex,@function\n"
> 	SYMBOL_UNDERSCORE "XPTC_InvokeByIndex:\n\t"
>+#endif

All the changes from this point to the end should be done instead by defining
MOZ_NEED_LEADING_UNDERSCORE.  See LXR.

There are a bunch of things in this patch that I'd need a good bit of time to
understand, or at least be more awake...

Comment 18

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 142494 [details] [diff] [review]
Fix leftover mingw 3.3.1 & 3.3.3 issues

>Index: xpcom/obsolete/nsFileStream.h
>-class NS_COM_OBSOLETE nsIOFileStream
>+class nsIOFileStream

This also seems odd.

Comment 19

[cls]

(In reply to comment #17)
> (From update of attachment 142494 [details] [diff] [review])
> >Index: xpcom/glue/nsWeakReference.cpp
> >-NS_IMETHODIMP
> >+NS_COM nsresult
> 
> Are NS_IMETHODIMP and NS_COM mutually exclusive?  They shouldn't be, I don't
> think.

The difference is that NS_COM also sets __declspec(dllexport|dllimport), making
the function externally available.

> >Index: xpcom/reflect/xptcall/src/md/unix/xptcinvoke_gcc_x86_unix.cpp
> 
> >+#ifdef XP_WIN32
> >+	".globl " SYMBOL_UNDERSCORE "_XPTC_InvokeByIndex\n\t"
> >+	".type  " SYMBOL_UNDERSCORE "_XPTC_InvokeByIndex,@function\n"
> >+	SYMBOL_UNDERSCORE "_XPTC_InvokeByIndex:\n\t"
> >+#else
> > 	".globl " SYMBOL_UNDERSCORE "XPTC_InvokeByIndex\n\t"
> > 	".type  " SYMBOL_UNDERSCORE "XPTC_InvokeByIndex,@function\n"
> > 	SYMBOL_UNDERSCORE "XPTC_InvokeByIndex:\n\t"
> >+#endif
> 
> All the changes from this point to the end should be done instead by defining
> MOZ_NEED_LEADING_UNDERSCORE.  See LXR.

MOZ_NEED_LEADING_UNDERSCORE is already set.  I need the extra underscore so that
I can differentiate between the asm-implemented XPTC_InvokeByIndex function and
the C wrapper function which will also have a leading underscore when converted
to asm.

Comment 20

[cls]

(In reply to comment #18)
> (From update of attachment 142494 [details] [diff] [review])
> >Index: xpcom/obsolete/nsFileStream.h
> >-class NS_COM_OBSOLETE nsIOFileStream
> >+class nsIOFileStream
> 
> This also seems odd.

Right, that's the oddball class that I cannot get gcc to emit a vtable for under
any circumstances.  From previous testing, I believe you have to use the
dllimport/dllexport pair for each class that nsIOFileStream & its members
inherit from.  After that approach took me deep into the new string classes, I
gave up.  It's in xpcom_obsolete which is deprecated and has been slated for
removal for years so I'm not too worried about it.

Comment 21

[Mike Shaver (:shaver emeritus)]

Comment on attachment 142494 [details] [diff] [review]
Fix leftover mingw 3.3.1 & 3.3.3 issues

>+ * expliclitly dllexported.

Tyop: "expliclitly".

Otherwise, the xptcall change looks fine.

Comment 22

[cls]

Attachment: Updated patch

This patch incorporates the debug changes needed (nsDequeFunctor &
nsXPTCStubBase), the nsParserMailbox.h change needed in a non-static build (so
many build variations) and the typo fix.

Comment 23

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 143685 [details] [diff] [review]
Updated patch

I really don't like the xptcinvoke hacks, but sr=dbaron.

Comment 24

[chris hofmann]

Comment on attachment 143685 [details] [diff] [review]
Updated patch

a=chofmann for 1.7b

Comment 25

[cls]

Attachment: Including MSVC fixes

MSVC complains when functions are marked as dllexport inside a class that's
also marked dllexport.	This patch includes further changes to nsXPTCStubBase
so that none of its functions, including the 250+ generated stubs use the
export declaration.

Comment 26

[Stephen Walker]

My VC7.1 seamonkey clobber build was fine with cls' removal of XPTC_EXPORT from
xptcall.h and xptcstubsdecl.inc.

Comment 27

[cls]

The changes landed a couple of days ago.  There has been one complaint that has
popped up about builds failing when using the latest binutils 2.15.90 release
with an older version of gcc.  On the mingw mailing list, there has been mention
of the latest gcc (3.3.3) not working well with and older binutils (2.14.90) so
it wouldn't surprise me if binutils had the same issue.  I tested builds using
gcc3.3.1/binutils 2.14.90, gcc3.3.3/binutils2.15.90 & gcc3.3.1/binutils2.15.90.
 The first two worked & the last failed.  I guess I should update the build page
or something.

Comment 28

[Gábor Lipták]

Anybody seeing the problem described in

http://bugzilla.mozilla.org/show_bug.cgi?id=236635

Thanks