pkgsrc-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

pkg/39358: www/firefox3 needs asm fixes for NetBSD 4.0 powerpc



>Number:         39358
>Category:       pkg
>Synopsis:       www/firefox3 needs asm fixes for NetBSD 4.0 powerpc
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    pkg-manager
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Aug 15 15:35:00 +0000 2008
>Originator:     Kernigh
>Release:        NetBSD 4.0 with pkgsrc-2008Q2
>Organization:
>Environment:
System: NetBSD ghostborough.local 4.0 NetBSD 4.0 (GENERIC) #0: Sun Dec 16 
00:27:58 PST 2007 
builds@wb30:/home/builds/ab/netbsd-4-0-RELEASE/macppc/200712160005Z-obj/home/builds/ab/netbsd-4-0-RELEASE/src/sys/arch/macppc/compile/GENERIC
 macppc
Architecture: powerpc
Machine: macppc
>Description:
The www/firefox3 package, which contains Firefox 3.0.1, failed to build on my
NetBSD 4.0 macppc system. The pkgsrc happily and automatically built all of
the dependencies for Firefox (including perl, python, GTK +2 and some other
stuff). Then it stopped in the directory
/usr/pkgsrc/www/firefox3/work/mozilla/xpcom/reflect/xptcall/src/md/unix/
with the error message, "XPTCall not implemented on this platform!"

I discovered that there was some asm (and some C glue code) specific to
NetBSD/powerpc, but that no one had maintained it. I made some changes,
and I eventually had a Firefox that installed and that did not dump core.
I used mkpatches (from pkgtools/pkgdiff) to generate five new patches,
which I provide below.

With the below fixes, my Firefox (or "Minefield" browser as it names itself)
basicly works. I used my newly built Firefox to query the NetBSD PRs, and
I used the Chatzilla add-on to reach #NetBSD on Freenode. (I still have a
fonts problem, but that seems to be a separate bug that affects all my
GTK +2 apps. I will not report that bug here, but I will mention that
disabling "Allow pages to choose their own fonts" (in Edit -> Preferences ->
Content -> Fonts & Colors -> Advanced...) mitigates the problem.)

This bug might need a report in the upstream bugzilla.mozilla.org but I
did not report the bug there.

>How-To-Repeat:
$ cd /usr/pkgsrc/www/firefox3
$ sudo make

>Fix:
I generated new versions of patch-ac, patch-bx and patch-cb. Also mkpatches
decided to make two new patches and call them patch-af and patch-ag. I
believe that these five patches together fix www/firefox3 for NetBSD 4.0
powerpc. However, I did not try a clean build after I made the patches.

Most of patch-ac was already there; I changed a line in the NetBSD/PPC
section. OS_TEST contains 'powerpc' instead of 'macppc' now, so that is
what the ifeq line needs to test for.

$NetBSD: patch-ac,v 1.1.1.1 2008/06/28 10:01:07 tnn Exp $

--- xpcom/reflect/xptcall/src/md/unix/Makefile.in.orig  2008-04-09 
06:34:20.000000000 +0000
+++ xpcom/reflect/xptcall/src/md/unix/Makefile.in
@@ -64,14 +64,21 @@ include $(topsrcdir)/config/config.mk
 #
 # Lots of Unixish x86 flavors
 #
-ifneq (,$(filter FreeBSD NetBSD OpenBSD BSD_OS Darwin,$(OS_ARCH)))
+ifneq (,$(filter DragonFly FreeBSD NetBSD OpenBSD BSD_OS Darwin,$(OS_ARCH)))
+ifeq (x86_64,$(OS_TEST))
+CPPSRCS                := xptcinvoke_x86_64_linux.cpp 
xptcstubs_x86_64_linux.cpp
+else
 ifeq (86,$(findstring 86,$(OS_TEST)))
 CPPSRCS                := xptcinvoke_unixish_x86.cpp xptcstubs_unixish_x86.cpp
+endif
+ifeq (amd64,$(OS_TEST))
+CPPSRCS                := xptcinvoke_x86_64_linux.cpp 
xptcstubs_x86_64_linux.cpp
+endif
+endif
 ifeq (Darwin,$(OS_ARCH))
 DEFINES                += -DKEEP_STACK_16_BYTE_ALIGNED
 endif
 endif
-endif
 #
 # New code for Linux, et. al., with gcc
 # Migrate other platforms here after testing
@@ -185,7 +192,7 @@ endif
 # NetBSD/ARM
 #
 ifeq ($(OS_ARCH),NetBSD)
-ifneq (,$(filter arm% sa110,$(OS_TEST)))
+ifneq (,$(filter arm%,$(TARGET_CPU)))
 CPPSRCS                := xptcinvoke_arm_netbsd.cpp xptcstubs_arm_netbsd.cpp
 endif
 endif
@@ -240,7 +247,7 @@ endif
 # NetBSD/m68k
 #
 ifeq ($(OS_ARCH),NetBSD)
-ifneq (,$(filter amiga atari hp300 mac68k mvme68k next68k sun3 sun3x 
x68k,$(OS_TEST)))
+ifneq (,$(filter m68k,$(TARGET_CPU)))
 CPPSRCS                := xptcinvoke_netbsd_m68k.cpp xptcstubs_netbsd_m68k.cpp
 endif
 endif
@@ -329,9 +336,10 @@ endif
 #
 # NetBSD/PPC
 #
-ifneq (,$(filter NetBSDmacppc NetBSDbebox NetBSDofppc NetBSDprep 
NetBSDamigappc,$(OS_ARCH)$(OS_TEST)))                           
+ifeq ($(OS_ARCH)$(OS_TEST),NetBSDpowerpc)
 CPPSRCS                := xptcinvoke_ppc_netbsd.cpp xptcstubs_ppc_netbsd.cpp
 ASFILES                := xptcinvoke_asm_ppc_netbsd.s 
xptcstubs_asm_ppc_netbsd.s
+AS             := $(CC) -c -x assembler-with-cpp
 endif
 
 #
@@ -403,6 +411,13 @@ CPPSRCS            := xptcinvoke_sparc64_openbsd.c
 ASFILES                := xptcinvoke_asm_sparc64_openbsd.s 
xptcstubs_asm_sparc64_openbsd.s
 endif
 #
+# NetBSD/SPARC64
+#
+ifeq ($(OS_ARCH)$(OS_TEST),NetBSDsparc64)
+CPPSRCS                := xptcinvoke_sparc64_netbsd.cpp 
xptcstubs_sparc64_netbsd.cpp
+ASFILES                := xptcinvoke_asm_sparc64_netbsd.s 
xptcstubs_asm_sparc64_netbsd.s
+endif
+#
 # Solaris/SPARC
 #
 ifeq ($(OS_ARCH),SunOS)

The above change allowed the build to find the powerpc_netbsd stuff and
fixed the first error. But the powerpc_netbsd stuff still had some compiler
errors. I looked in the other implementations (especially powerpc_linux
and powerpc_openbsd) to find what changes I needed to make.

In patch-bx, I updated xptcstubs_ppc_netbsd.cpp to use the new interface
to GetMethodInfo and CallMethod. I also included xptiprivate.h because
the other implementations included it. The other changes in patch-bx
(involving the __GXX_ABI_VERSION and the gcc-3 asm) were already in pkgsrc.

$NetBSD: patch-bx,v 1.1.1.1 2008/06/28 10:01:07 tnn Exp $

diff -ruN 
../Orig/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcstubs_ppc_netbsd.cpp 
./xpcom/reflect/xptcall/src/md/unix/xptcstubs_ppc_netbsd.cpp
--- xpcom/reflect/xptcall/src/md/unix/xptcstubs_ppc_netbsd.cpp.orig     
2004-04-18 14:18:18.000000000 +0000
+++ xpcom/reflect/xptcall/src/md/unix/xptcstubs_ppc_netbsd.cpp
@@ -41,6 +41,7 @@
 // Implement shared vtbl methods.
 
 #include "xptcprivate.h"
+#include "xptiprivate.h"
 
 // The Linux/PPC ABI (aka PPC/SYSV ABI) passes the first 8 integral
 // parameters and the first 8 floating point parameters in registers
@@ -71,7 +72,6 @@ PrepareAndDispatch(nsXPTCStubBase* self,
 {
     nsXPTCMiniVariant paramBuffer[PARAM_BUFFER_COUNT];
     nsXPTCMiniVariant* dispatchParams = NULL;
-    nsIInterfaceInfo* iface_info = NULL;
     const nsXPTMethodInfo* info;
     PRUint32 paramCount;
     PRUint32 i;
@@ -79,12 +79,7 @@ PrepareAndDispatch(nsXPTCStubBase* self,
 
     NS_ASSERTION(self,"no self");
 
-    self->GetInterfaceInfo(&iface_info);
-    NS_ASSERTION(iface_info,"no interface info");
-    if (! iface_info)
-        return NS_ERROR_UNEXPECTED;
-
-    iface_info->GetMethodInfo(PRUint16(methodIndex), &info);
+    self->mEntry->GetMethodInfo(PRUint16(methodIndex), &info);
     NS_ASSERTION(info,"no method info");
     if (! info)
         return NS_ERROR_UNEXPECTED;
@@ -119,8 +114,10 @@ PrepareAndDispatch(nsXPTCStubBase* self,
                 if ((PRUint32) ap & 4) ap++; // doubles are 8-byte aligned on 
stack
                 dp->val.d = *(double*) ap;
                 ap += 2;
+#if __GXX_ABI_VERSION < 100
                if (gpr < GPR_COUNT)
                    gpr += 2;
+#endif
             }
             continue;
         }
@@ -130,8 +127,10 @@ PrepareAndDispatch(nsXPTCStubBase* self,
             else {
                 dp->val.f = *(float*) ap;
                ap += 1;
+#if __GXX_ABI_VERSION < 100
                if (gpr < GPR_COUNT)
                    gpr += 1;
+#endif
             }
             continue;
         }
@@ -179,9 +178,9 @@ PrepareAndDispatch(nsXPTCStubBase* self,
         }
     }
 
-    result = self->CallMethod((PRUint16) methodIndex, info, dispatchParams);
-
-    NS_RELEASE(iface_info);
+    result = self->mOuter->CallMethod((PRUint16) methodIndex,
+                                      info,
+                                      dispatchParams);
 
     if (dispatchParams != paramBuffer)
         delete [] dispatchParams;
@@ -195,7 +194,9 @@ PrepareAndDispatch(nsXPTCStubBase* self,
 // however, it's quick, dirty, and'll break when the ABI changes on
 // us, which is what we want ;-).
 
-#define STUB_ENTRY(n)                                       \
+#if __GXX_ABI_VERSION < 100
+// gcc-2 version
+# define STUB_ENTRY(n)                                       \
 __asm__ (                                                   \
         ".section \".text\" \n\t"                           \
         ".align 2 \n\t"                                     \
@@ -206,6 +207,46 @@ __asm__ (                               
        "li     11,"#n" \n\t"                               \
        "b      SharedStub@local \n"                        \
 );
+#else
+// gcc-3 version
+//
+// As G++3 ABI contains the length of the functionname in the mangled
+// name, it is difficult to get a generic assembler mechanism like
+// in the G++ 2.95 case.
+// Create names would be like:
+// _ZN14nsXPTCStubBase5Stub1Ev
+// _ZN14nsXPTCStubBase6Stub12Ev
+// _ZN14nsXPTCStubBase7Stub123Ev
+// _ZN14nsXPTCStubBase8Stub1234Ev
+// etc.
+// Use assembler directives to get the names right...
+
+# define STUB_ENTRY(n)                                                 \
+__asm__ (                                                              \
+       ".align 2 \n\t"                                                 \
+       ".if    "#n" < 10 \n\t"                                         \
+       ".globl _ZN14nsXPTCStubBase5Stub"#n"Ev \n\t"                    \
+       ".type  _ZN14nsXPTCStubBase5Stub"#n"Ev,@function \n\n"          \
+"_ZN14nsXPTCStubBase5Stub"#n"Ev: \n\t"                                 \
+                                                                       \
+       ".elseif "#n" < 100 \n\t"                                       \
+       ".globl _ZN14nsXPTCStubBase6Stub"#n"Ev \n\t"                    \
+       ".type  _ZN14nsXPTCStubBase6Stub"#n"Ev,@function \n\n"          \
+"_ZN14nsXPTCStubBase6Stub"#n"Ev: \n\t"                                 \
+                                                                       \
+       ".elseif "#n" < 1000 \n\t"                                      \
+       ".globl _ZN14nsXPTCStubBase7Stub"#n"Ev \n\t"                    \
+       ".type  _ZN14nsXPTCStubBase7Stub"#n"Ev,@function \n\n"          \
+"_ZN14nsXPTCStubBase7Stub"#n"Ev: \n\t"                                 \
+                                                                       \
+       ".else \n\t"                                                    \
+       ".err   \"stub number "#n" >= 1000 not yet supported\"\n"       \
+       ".endif \n\t"                                                   \
+                                                                       \
+       "li     11,"#n" \n\t"                                           \
+       "b      SharedStub@local \n"                                    \
+);
+#endif
 
 #define SENTINEL_ENTRY(n)                            \
 nsresult nsXPTCStubBase::Sentinel##n()               \

In patch-cb, I needed to update xptcinvoke_ppc_netbsd.cpp to change
XPTC_InvokeByIndex to NS_InvokeByIndex_P, to avoid a link error later.
Also XPTC_PUBLIC_API becomes EXPORT_XPCOM_API. The change from
XPTC_PUBLIC_API to EXPORT_XPCOM_API was one of the first changes that
I made; I wasted a lot of time thinking that XPTC_PUBLIC_API was in some
header file, before noticing that I should use EXPORT_XPCOM_API instead.
The __GXX_ABI_VERSION changes were already in pkgsrc.

$NetBSD: patch-cb,v 1.1.1.1 2008/06/28 10:01:07 tnn Exp $

diff -ruN 
../Orig/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_ppc_netbsd.cpp 
./xpcom/reflect/xptcall/src/md/unix/xptcinvoke_ppc_netbsd.cpp
--- xpcom/reflect/xptcall/src/md/unix/xptcinvoke_ppc_netbsd.cpp.orig    
2008-01-06 03:46:24.000000000 +0000
+++ xpcom/reflect/xptcall/src/md/unix/xptcinvoke_ppc_netbsd.cpp
@@ -40,9 +40,9 @@
 
 // Platform specific code to invoke XPCOM methods on native objects
 
-// The purpose of XPTC_InvokeByIndex() is to map a platform
+// The purpose of NS_InvokeByIndex_P() is to map a platform
 // indepenpent call to the platform ABI. To do that,
-// XPTC_InvokeByIndex() has to determine the method to call via vtable
+// NS_InvokeByIndex_P() has to determine the method to call via vtable
 // access. The parameters for the method are read from the
 // nsXPTCVariant* and prepared for the native ABI.  For the Linux/PPC
 // ABI this means that the first 8 integral and floating point
@@ -104,8 +104,10 @@ invoke_copy_to_stack(PRUint32* d,
                 if ((PRUint32) d & 4) d++; // doubles are 8-byte aligned on 
stack
                 *((double*) d) = s->val.d;
                 d += 2;
+#if __GXX_ABI_VERSION < 100
                if (gpr < GPR_COUNT)
                    gpr += 2;
+#endif
             }
         }
         else if (!s->IsPtrData() && s->type == nsXPTType::T_FLOAT) {
@@ -114,8 +116,10 @@ invoke_copy_to_stack(PRUint32* d,
             else {
                 *((float*) d) = s->val.f;
                d += 1;
+#if __GXX_ABI_VERSION < 100
                if (gpr < GPR_COUNT)
                    gpr += 1;
+#endif
            }
         }
         else if (!s->IsPtrData() && (s->type == nsXPTType::T_I64
@@ -142,6 +146,6 @@ invoke_copy_to_stack(PRUint32* d,
 }
 
 extern "C"
-XPTC_PUBLIC_API(nsresult)
-XPTC_InvokeByIndex(nsISupports* that, PRUint32 methodIndex,
+EXPORT_XPCOM_API(nsresult)
+NS_InvokeByIndex_P(nsISupports* that, PRUint32 methodIndex,
                    PRUint32 paramCount, nsXPTCVariant* params);

To build a firefox-bin that does not dump core, I also had to make changes
to xptcinvoke_asm_ppc_netbsd.s; this is a new patch and mkpatches decided
to call it patch-ag. First I made the obvious change from XPTC_InvokeByIndex
to NS_InvokeByIndex_P. Then firefox-bin started to dump core from this
function. I had to struggle with the asm to make it work.

I know almost nothing about asm, but I made some guesses, and I found some
web site with a PowerPC instruction reference. I read the asm in
xptcinvoke_asm_ppc_{linux,openbsd}.s and tried to make the netbsd
implementation do something similar. I found a section of asm, dealing with
the virtual function table, that seemed to be very wrong. Fortunately I
understood that a virtual function table is an array of pointers.

So I changed the "convert to offset" line to multiply by 4 instead of 8
(shift left by 2 instead of 3) because PowerPC pointers are only 4 bytes
each. Then I deleted the stuff about the "virtual base offset", so that
I could pass the 'that' pointer (in register 'r3') unmodified. The
instruction "lwzx r0,r4,r5", which I copied from {linux,openbsd}, does 
"r0 = *(r4 + r5)" to get the function pointer from the virtual function
table.

Here is patch-ag

$NetBSD$

--- xpcom/reflect/xptcall/src/md/unix/xptcinvoke_asm_ppc_netbsd.s.orig  
2006-12-11 09:45:39.000000000 +0000
+++ xpcom/reflect/xptcall/src/md/unix/xptcinvoke_asm_ppc_netbsd.s
@@ -55,15 +55,15 @@
                      
         .section ".text"
        .align 2
-       .globl XPTC_InvokeByIndex
-       .type  XPTC_InvokeByIndex,@function
+       .globl NS_InvokeByIndex_P
+       .type  NS_InvokeByIndex_P,@function
 
 #
-# XPTC_InvokeByIndex(nsISupports* that, PRUint32 methodIndex,
+# NS_InvokeByIndex_P(nsISupports* that, PRUint32 methodIndex,
 #                    PRUint32 paramCount, nsXPTCVariant* params)
 #
 
-XPTC_InvokeByIndex:
+NS_InvokeByIndex_P:
        stwu    sp,-32(sp)                      # setup standard stack frame
        mflr    r0                              # save LR
        stw     r3,8(sp)                        # r3 <= that
@@ -101,13 +101,8 @@ XPTC_InvokeByIndex:
        lwz     r3,8(r31)                       # r3 <= that
        lwz     r4,12(r31)                      # r4 <= methodIndex
        lwz     r5,0(r3)                        # r5 <= vtable ( == *that )
-       slwi    r4,r4,3                         # convert to offset ( *= 8 )
-       addi    r4,r4,8                         # skip first two vtable entries
-       add     r4,r4,r5
-       lhz     r0,0(r4)                        # virtual base offset
-       extsh   r0,r0
-       add     r3,r3,r0
-       lwz     r0,4(r4)                        # r0 <= methodpointer ( == 
vtable + offset )
+       slwi    r4,r4,2                         # convert to offset ( *= 4 )
+       lwzx    r0,r4,r5                        # r0 <= methodpointer ( == 
vtable + offset )
 
         lwz     r4,4(r30)                      # load GP regs with method 
parameters
        lwz     r5,8(r30)   

Finally, I almost forgot my patch-af. While compiling something in
work/mozilla/netwerk/cookie/src, the compiler promoted some warning into
an error and broke the build. So I disabled the warning-to-error feature,
which had been only enabled in that one directory.

However I forgot exactly where the error was. Some header file had a cast
from char * to some other type of pointer, and -Wconversion was complaining
about a possible alignment problem. (I might be wrong, but it seems that
PowerPC, unlike i386, is a "strict alignment" processor. Thus one cannot
access a pointer unless the pointed-to thing is aligned correctly.) I
noticed that the char * pointed to a char[] declared at the top of a struct
(though I forgot to check if the struct inherits from another struct) so
I guessed that there was no alignment problem, and I did not fix the
warning.

Here is patch-af

$NetBSD$

--- netwerk/cookie/src/Makefile.in.orig 2007-08-28 16:02:43.000000000 +0000
+++ netwerk/cookie/src/Makefile.in
@@ -60,5 +60,5 @@ CPPSRCS               = \
 
 include $(topsrcdir)/config/rules.mk
 
-CXXFLAGS += $(WARNINGS_AS_ERRORS)
+#CXXFLAGS += $(WARNINGS_AS_ERRORS)
 DEFINES += -DIMPL_NS_NET



Home | Main Index | Thread Index | Old Index