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

Status: RESOLVED WORKSFORME

(Core :: XPConnect, defect)

Description

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

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*)).

Comment 1

[Bradley Baetz (:bbaetz)]

Theres a patch in bug 101528 for m68k

Comment 2

[Mike Shaver (:shaver emeritus)]

*** Bug 103887 has been marked as a duplicate of this bug. ***

Comment 3

[Jon Smirl]

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.

Comment 4

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

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. :-)

Comment 5

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

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 6

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

Attachment: build changes needed to build GCC3 version for sparc

Comment 7

[Mike Shaver (:shaver emeritus)]

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.)

Comment 8

[hacker formerly known as [e-mail]]

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

Comment 9

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

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?)

Comment 10

[Mike Shaver (:shaver emeritus)]

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.

Comment 11

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

Attachment: Patch that I think worked for ppc/gcc3 except the machine was too slow to really test

Comment 12

[hacker formerly known as [e-mail]]

Except the one tiny little fact that we don't set CPP. Easily changed though.

Comment 13

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

Well, CPP was set to gcc -E when I tried it. :-)

Comment 14

[tilps]

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.

Comment 15

[Nick Blievers]

Attachment: Fix for gcc-3 on IRIX

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.

Comment 16

[Nick Blievers]

added myself to cc list

Comment 17

[Christiaan Welvaart]

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.

Comment 18

[Nick Blievers]

Could someone please have a look at the IRIX patch? It needs a review, Thanks!

Comment 19

[Mike Pinkerton (not reading bugmail)]

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?

Comment 20

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

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.

Comment 21

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

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.

Comment 22

[Owen Anderson]

I can confirm that Mozilla will not compile with Apple's April Dev Tools 
(including GCC3.1).  I'll post the exact error later.

Comment 23

[Nick Blievers]

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 24

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

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.

Comment 25

[Nick Blievers]

Attachment: gcc-3 on IRIX... with suggested improvements

Comment 26

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

Comment on attachment 93245 [details] [diff] [review]
gcc-3 on IRIX... with suggested improvements

r=dbaron

Comment 27

[Steve Dagley]

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.

Comment 28

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

We should be good on gcc3 on OS X 10.2, barring one problem in asdecode.  See
bug 153525.

Comment 29

[John Steele Scott]

Attachment: patch to compile with gcc3 on linux-mips

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 :)

Comment 30

[Christopher Blizzard (:blizzard)]

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?

Comment 31

[Mike Shaver (:shaver emeritus)]

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.

Comment 32

[Nick Blievers]

Please dont close this... it is not fixed! The patches exist, but are not
checked in.

Comment 33

[Miles Nordin]

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.

Comment 34

[Nick Blievers]

> 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.

Comment 35

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

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?

Comment 36

[Nick Blievers]

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.

Comment 37

[Benjamin Smedberg]

Any value that this bug may previously have had is gone. Please ping me if there are outstanding xptcall patches that need attention.