Closed Bug 71627 Opened 24 years ago Closed 18 years ago

probably don't run using gcc-3.0 on non x86 platforms

Categories

(Core :: XPConnect, defect)

defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: dbaron, Unassigned)

References

Details

Attachments

(4 files, 1 obsolete file)

DESCRIPTION:  The change to the new G++ V3 ABI for gcc 3.0 may well break the
xptcall code on all platforms where we use gcc.  (If the ABI was changed the
same way on all platforms, it will.)  I just checked in the fix for x86 (see
bug 63604), but we probably need similar fixes on other platforms.  The fix
for x86 involved conditionally
 ( #if defined(__GXX_ABI_VERSION) && __GXX_ABI_VERSION >= 100 /* G++ V3 ABI */ )
looking for the address of the nth virtual function pointer at
(vptr + n*sizeof(void*)) rather than (vptr + (2 + n)*sizeof(void*)).
Theres a patch in bug 101528 for m68k
Depends on: 101528
Depends on: 102492
*** Bug 103887 has been marked as a duplicate of this bug. ***
My experience in 103887 make me think this is broken on Sparc too.
I am running 64bit Solaris 8, with gcc 3.0.1 and failed.
I added xptcinvoke code for Solaris/sparc with gcc3 that works for me in
TestXPTCInvoke with gcc 3.0.1.  I added it as a separate file since there
doesn't seem to be a good way to |#if| assembler with gcc (although I could
always remove it later).  So if you want to test it, just:

Index: Makefile.in
===================================================================
RCS file: /cvsroot/mozilla/xpcom/reflect/xptcall/src/md/unix/Makefile.in,v
retrieving revision 1.53
diff -u -d -u -0 -r1.53 Makefile.in
--- Makefile.in	2001/09/06 08:23:09	1.53
+++ Makefile.in	2001/10/10 06:35:38
@@ -250 +250 @@
-ASFILES
	:= xptcinvoke_asm_sparc_solaris_GCC.s xptcstubs_asm_sparc_solaris.s
+ASFILES
	:= xptcinvoke_asm_sparc_solaris_GCC3.s xptcstubs_asm_sparc_solaris.s

Though we really need to figure out how to let the build system know whether
we're using gcc3 (probably via an autoconf test testing using the |#if| above).
 I also need to get the file I added reviewed. :-)
The necessary change for PPC linux is just (after bug 86446) the following --
I'm not sure how to conditionally build this change for gcc3:

Index: xptcinvoke_asm_ppc_linux.s
===================================================================
RCS file:
/cvsroot/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_asm_ppc_linux.s,v
retrieving revision 1.4
diff -u -d -u -0 -r1.4 xptcinvoke_asm_ppc_linux.s
--- xptcinvoke_asm_ppc_linux.s	2000/01/08 00:30:05	1.4
+++ xptcinvoke_asm_ppc_linux.s	2001/10/11 00:44:08
@@ -88 +87,0 @@
-
addi
r4,r4,2
			# skip first two vtable entries
Comment on attachment 54195 [details] [diff] [review]
build changes needed to build GCC3 version for sparc

sr=shaver on this patch.

What's with the wonky comment style in the GCC3.s file?  Tidy up the
XPTC_InvokeByIndex intro comment, and sr=shaver there, too.

(My sr is enough for the xptcall stuff to go in, but you'll need at least an
irc-nod from cls to land the build-fu.)
Attachment #54195 - Flags: superreview+
Comment on attachment 54195 [details] [diff] [review]
build changes needed to build GCC3 version for sparc

The AC_SUBST(HAVE_GCC3_ABI) needs to go outside of that GNU_CC if statement. 
Otherwise, the non-gcc3 builds will set HAVE_GCC3_ABI=@HAVE_GCC3_ABI@ which is
enough to trigger your ifdef in the xptcall makefile.  
Fix that && r=cls
Attachment #54195 - Flags: review+
I just checked in the solaris/sparc patch above.

So, does anybody have any bright ideas for how to ifdef within
xptcinvoke_asm_ppc_linux.s, or should I just copy the file and make the one line
change?  (m4 is currently only a build requirement for those who want to rerun
autoconf.  Are we comfortable changing that for some platforms?)
If you want to #ifdef it, why not just run it through the C pre-processor?  We
figure out how to do that as part of the configure run, I think.
Except the one tiny little fact that we don't set CPP. Easily changed though.
Well, CPP was set to gcc -E when I tried it. :-)
This looks to be a problem on OSF alpha as well - however i have no knowledge 
of alpha asm so i am only going by the comments in the existing asm.

I guess I could learn, unless someone else fixes it first.
Attached patch Fix for gcc-3 on IRIX (obsolete) — Splinter Review
After applying a fix for bug 86446, this patch will allow compiling with gcc-3
on IRIX (note: there is a gcc bug that causes an ICE with gcc-3.0.4 and below).

Applying this patch WITHOUT applying a fix for bug 86446 will not cause a
problem
for gcc-2.X builds.
added myself to cc list
Patch 59277 does not seem to work, tried it with patch 53024 with gcc
3.1-prerelease (ppc linux) and got crashing mozillas. Where could I find
information about the "new" G++ ABI specifically for powerpc, apart from the gcc
sources? Note: __GXX_ABI_VERSION is not defined by cpp, gcc or as, but only when
g++ is used, at least with this version of gcc.
Could someone please have a look at the IRIX patch? It needs a review, Thanks!
this'll be an issue for the osx mach-o build when apple releases the next round
of dev tools. beard, care to take a look at this since you did the last xptcall
for mach-o?
If the ABI changes for darwin are the same as for other platforms, the change
will probably just be removing the line:

        addi    r5,r5,8                 ; (methodIndex * 4) + 8

from xptcinvoke_asm_ppc_rhapsody.s .  The fun part is getting the assembly run
through the preprocessor so you can #ifdef it.
Note that attachment 59277 [details] [diff] [review] and attachment 73845 [details] [diff] [review] both have different approaches
to ifdef-ing.  If the one in the latter patch works for you, then that's
probably easier.
Blocks: 144368
I can confirm that Mozilla will not compile with Apple's April Dev Tools 
(including GCC3.1).  I'll post the exact error later.
Can someone please review 73845? Is there any chance this will get checked in soon?
Also, it really doesn't seem to depend on 102492 (or is that just me?), and it 
actually DOES depend on 86446.
Comment on attachment 73845 [details] [diff] [review]
Fix for gcc-3 on IRIX

>@@ -102,12 +114,16 @@
> 	lw	t0, 0(a0)
> 	addu	t0, t1
> #ifdef __GNUC__
>+#if (__GNUC__ == 2)
> 	lh      t0, 8(t0)
>+	addu	a0, a0, t0
> #else
>-	lw	t0, 12(t0)
>+	# no change for GCC 3 -- that = this
> #endif
>-	
>+#else
>+	lw	t0, 12(t0)
> 	addu	a0, a0, t0
>+#endif
> 
> 	# get register save area from invoke_copy_to_stack
> 	subu	t1, t3, 64

One comment about this change:	I don't see how gcc3 even needs the two lines
before where you changed the ifdefs -- I don't see any uses of t0 later in the
code.  It might also be clearer if you just put the whole chunk of code inside
of "#if !defined(__GNUC__) || __GNUC__ == 2" rather than changing the gcc vs.
native ifdefs to handle gcc3 not doing this-adjustment at all.

Other than that these changes make sense to me.
Attachment #73845 - Attachment is obsolete: true
Comment on attachment 93245 [details] [diff] [review]
gcc-3 on IRIX... with suggested improvements

r=dbaron
Attachment #93245 - Flags: review+
Blocks: 149461
What's the status of this with gcc3 on Mac OS X 10.2?  I _know_ we're building
Chimera under 10.2 and we want to move to that for the Mozilla mach-o builds.
We should be good on gcc3 on OS X 10.2, barring one problem in asdecode.  See
bug 153525.
No longer blocks: 149461
I believe this patch fixes the problem on linux-mips. I have tested it and it
now compiles with gcc-3.3. I can't get it to run yet, but this is one step on
the way :)
I think that's the name mangling change, there.  Do you want to use an ifdef for
gcc3 like a lot of the other platforms do?
I'm tempted to close this out, since I'm sure we'd have heard more violent noise
if we still didn't work with gcc3 on non-x86 systems.  Right?

For now, though, to the orphanage.
Assignee: shaver → nobody
Please dont close this... it is not fixed! The patches exist, but are not
checked in.
there are patches for this problem for alpha in Gentoo, and for ppc in netbsd.  there are probably more Linux patches floating out there as well.  This bug has been open for THREE YEARS.  someone needs to take ownership of it, and start collecting all the halfass workarounds other ``distros'' have done to work around mozilla's non-support of noti386, and commit them to mozilla.org CVS.

Why was this bug not fixed as part of #63604?  It is a pretty important one.  It seems to have slipped through the cracks.
> Why was this bug not fixed as part of #63604?  It is a pretty important one. 
It seems to have slipped through the cracks.

That bug may well have fixed x86/nt, but it has not fixed this one. Please take
ownership...! There are several parts to this bug, but all the patches (for at
least IRIX, and probably others) are available.
Which patches on this bug have not been checked in?

Why have the other patches you mention not been contributed back to the Mozilla
project?  Where are they available?
93245 for eg is not checked in. It does require a patch from 86446 to also be
checked in as well (I'm not withholding patches from you ;-) they are all
available). 

There doesn't seem to be any dependency on 102492 however.

There also seems to be outstanding patches for ppc and linux/mips... but I could
easily be wrong on those ones.
QA Contact: pschwartau → xpconnect
Any value that this bug may previously have had is gone. Please ping me if there are outstanding xptcall patches that need attention.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: