Re: Bug#516616: bind9 locks up on start on mipsel and mips

[Niko Tyni <ntyni@debian.org>]

On Sun, Feb 22, 2009 at 06:02:51PM +0100, Rainer Canavan wrote:
> Package: bind9
> Version: 1:9.5.1.dfsg.P1-1
> Severity: grave
> Justification: renders package unusable
> 
> bind9 fails to start on mipsel architecture machines, without
> even reading any of its configuration files. As can be seen
> from the output of strace included below,  one of the two 
> processes forked gets stuck in epoll() the other in futex().

I can reproduce this with 9.6.1.dfsg.P3-1 in a qemu mips virtual machine.
There's a report of it being broken since early 2008 at
 http://lists.debian.org/debian-mips/2009/02/msg00039.html

The lockup is at
 
 #0  0x2b2a95cc in pthread_cond_wait@@GLIBC_2.3.2 () from /lib/libpthread.so.0
 #1  0x2b014b3c in isc_rwlock_lock (rwl=0x2adaf5c8, type=isc_rwlocktype_write) at rwlock.c:316
 #2  0x2ab46dc4 in dns_db_register (name=0x4abd18 "_builtin", create=0x2acc9acc <dns_sdb_create>, 
     driverarg=0x2baa6fd8, mctx=0x4d0158, dbimp=0x2baa7000) at db.c:821
 #3  0x2acc4c48 in dns_sdb_register (drivername=0x4abd18 "_builtin", methods=0x4ca0cc, driverdata=0x0, 
     flags=3, mctx=0x4d0158, sdbimp=0x4cef10) at sdb.c:239
 #4  0x0040dbc0 in ns_builtin_init () at builtin.c:296
 #5  0x0042f0ec in setup () at ./main.c:741
 #6  0x0042f860 in main (argc=3, argv=0x7ff270d4) at ./main.c:925

and happens because the MIPS assembly implementation of isc_atomic_xadd()
is broken, as seen by:

    #include <isc/atomic.h>
    #include <stdio.h>
    
    int main(void) {
        int p = 5;
        int q;
        q = isc_atomic_xadd(&p, 3);
        printf("%d + 3 = %d\n", q, p);
        return 0;
    }

which gives 
 8 + 3 = 8
on mips and the expected
 5 + 3 = 8
on amd64.

I'm attaching a patch that fixes this. I've verified that 9.6.1.dfsg.P3-1
works for me as a caching resolver with the patch, but I don't have a
real mips machine to test it on.

I don't understand how the current implementation can ever have worked. I
see it was added as a Debian specific patch due to #406409. Cc'ing the
debian-mips list in case somebody wants to review my patch and the
other #406409 changes (isc_atomic_store() and isc_atomic_cmpxchg()).
-- 
Niko Tyni   ntyni@debian.org

--- bind9-9.6.1.dfsg.P3/lib/isc/mips/include/isc/atomic.h
+++ bind9-9.6.1.dfsg.P3/lib/isc/mips/include/isc/atomic.h
@@ -40,7 +40,7 @@
 	"	addu	%0, $1, %2	\n"
 	"	sc	%0, %1		\n"
 	"	beqz	%0, 1b		\n"
-	"	 addu	%0, $1, %2	\n"
+	"	 addu	%0, $0, $1	\n"
 	"	.set	pop		\n"
 	: "=&r" (orig), "+R" (*p)
 	: "r" (val)