Closed Bug 226609 Opened 21 years ago Closed 20 years ago

xpcom compilation failure under cygwin/mingw 3.3.1 & 3.3.3

Categories

(Core :: XPCOM, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.7beta

People

(Reporter: gabor.liptak, Assigned: dougt)

Details

Attachments

(3 files, 5 obsolete files)

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
Changed product to Browser:XPCOM.
Assignee: wchang0222 → dougt
Component: NSPR → XPCOM
Product: NSPR → Browser
Version: other → Trunk
QA Contact: wchang0222 → nobody
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).
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.
*sigh* that should have been bug 231995.
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
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.
Attachment #141989 - Flags: superreview?(dbaron)
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
Attachment #141989 - Flags: review?(darin)
Per irc conversation with Darin, just mark inlined functions with |inline| and
remove NS_COM declarations.
Attachment #141989 - Attachment is obsolete: true
Attachment #141989 - Flags: superreview?(dbaron)
Attachment #141989 - Flags: review?(darin)
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+
Attachment #142060 - Flags: review?(darin) → review+
Attachment #142060 - Attachment description: Explicilty mark functions as inlined w/o NS_COM → Explicilty mark functions as inlined w/o NS_COM (checked in)
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.
Attachment #140872 - Attachment is obsolete: true
Summary: xpcom compilation failure under cygwin/mingw → xpcom compilation failure under cygwin/mingw 3.3.1 & 3.3.3
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();
Attachment #142494 - Flags: superreview?(dbaron)
Attachment #142494 - Flags: review?(BradleyJunk)
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
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.
(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.
(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.
Flags: blocking1.7b?
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+
Attached patch Updated patch (obsolete) — Splinter Review
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
Attachment #142494 - Flags: superreview?(dbaron)
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+
Attachment #143685 - Flags: approval1.7b?
Comment on attachment 143685 [details] [diff] [review]
Updated patch

a=chofmann for 1.7b
Attachment #143685 - Flags: approval1.7b? → approval1.7b+
Flags: blocking1.7b? → blocking1.7b+
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
My VC7.1 seamonkey clobber build was fine with cls' removal of XPTC_EXPORT from
xptcall.h and xptcstubsdecl.inc.
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
Closed: 20 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.7beta
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.

Attachment

General

Created:
Updated:
Size: