xpcom compilation failure under cygwin/mingw 3.3.1 & 3.3.3

RESOLVED FIXED in mozilla1.7beta

Status

()

Core
XPCOM
RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: Gábor Lipták, Assigned: dougt)

Tracking

Trunk
mozilla1.7beta
x86
Windows XP
Points:
---
Bug Flags:
blocking1.7b +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 5 obsolete attachments)

(Reporter)

Description

14 years ago
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

14 years ago
Changed product to Browser:XPCOM.
Assignee: wchang0222 → dougt
Component: NSPR → XPCOM
Product: NSPR → Browser
Version: other → Trunk

Updated

14 years ago
QA Contact: wchang0222 → nobody

Comment 2

14 years ago
Created attachment 140872 [details] [diff] [review]
disabling NS_COM for some private classes -- facilitates building on Cygwin

Comment 3

14 years ago
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

14 years ago
Created attachment 140916 [details] [diff] [review]
another pair of NS_COM disables for optimized builds

Comment 5

14 years ago
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 on attachment 140916 [details] [diff] [review]
another pair of NS_COM disables for optimized builds

The code being modified here doesn't exist anymore.
Attachment #140872 - Attachment is patch: true

Comment 7

14 years ago
*sigh* that should have been bug 231995.

Comment 8

14 years ago
Created attachment 141989 [details] [diff] [review]
Explicitly mark functions as inlined

Adding the 'inline' declaration to the inlined function causes gcc to properly
ignore the dllimport attribute of the functions.
Attachment #140916 - Attachment is obsolete: true

Comment 9

14 years ago
Created attachment 141990 [details]
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.

Updated

14 years ago
Attachment #141989 - Flags: superreview?(dbaron)

Comment 10

14 years ago
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.

Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

14 years ago
Attachment #141989 - Flags: review?(darin)

Comment 11

14 years ago
Created attachment 142060 [details] [diff] [review]
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.
Attachment #141989 - Attachment is obsolete: true

Updated

14 years ago
Attachment #141989 - Flags: superreview?(dbaron)
Attachment #141989 - Flags: review?(darin)

Updated

14 years ago
Attachment #142060 - Flags: superreview?(dbaron)
Attachment #142060 - Flags: review?(darin)
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.
Attachment #142060 - Flags: superreview?(dbaron) → superreview+

Updated

14 years ago
Attachment #142060 - Flags: review?(darin) → review+

Updated

14 years ago
Attachment #142060 - Attachment description: Explicilty mark functions as inlined w/o NS_COM → Explicilty mark functions as inlined w/o NS_COM (checked in)

Comment 13

14 years ago
Created attachment 142494 [details] [diff] [review]
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.

Updated

14 years ago
Attachment #140872 - Attachment is obsolete: true

Updated

14 years ago
Summary: xpcom compilation failure under cygwin/mingw → xpcom compilation failure under cygwin/mingw 3.3.1 & 3.3.3

Comment 14

14 years ago
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();

Updated

14 years ago
Attachment #142494 - Flags: superreview?(dbaron)
Attachment #142494 - Flags: review?(BradleyJunk)
(Reporter)

Comment 15

14 years ago
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

14 years ago
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 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 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

14 years ago
(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

14 years ago
(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.

Updated

14 years ago
Flags: blocking1.7b?

Updated

14 years ago
Attachment #142494 - Flags: review?(BradleyJunk) → review?(shaver)
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.
Attachment #142494 - Flags: review?(shaver) → review+

Comment 22

14 years ago
Created attachment 143685 [details] [diff] [review]
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.
Attachment #142494 - Attachment is obsolete: true

Updated

14 years ago
Attachment #142494 - Flags: superreview?(dbaron)

Updated

14 years ago
Attachment #143685 - Flags: superreview?(dbaron)
Comment on attachment 143685 [details] [diff] [review]
Updated patch

I really don't like the xptcinvoke hacks, but sr=dbaron.
Attachment #143685 - Flags: superreview?(dbaron) → superreview+

Updated

14 years ago
Attachment #143685 - Flags: approval1.7b?

Comment 24

14 years ago
Comment on attachment 143685 [details] [diff] [review]
Updated patch

a=chofmann for 1.7b
Attachment #143685 - Flags: approval1.7b? → approval1.7b+

Updated

14 years ago
Flags: blocking1.7b? → blocking1.7b+

Comment 25

14 years ago
Created attachment 143997 [details] [diff] [review]
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.
Attachment #143685 - Attachment is obsolete: true

Comment 26

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

Comment 27

14 years ago
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.

Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.7beta
(Reporter)

Comment 28

14 years ago
Anybody seeing the problem described in

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

Thanks
You need to log in before you can comment on or make changes to this bug.