Re: crashes in ipfilter on i386

Greg Troxel wrote:
> Darren Reed <darrenrnetbsd.org> writes:
>
> > Greg Troxel wrote:
> >
> >> Here's the diff in my source tree, producing a
kernel that doesn't
> >> crash.  Note there is a lot of s/INLINE//
noise because I found it hard
> >> to follow the asm code, but the real change is
dropping the IP6_NEQ
> >> test.  Obviously my change causes a
functionality regression - it was
> >> intended to isolate the bug, not proposed as a
fix.
> >>
> >> I still suspect failure to pull up enough
bytes, but I can't point to
> >> the wrong line of code.
> >
> > I'd like you to try it without the #if 0 but with
this change in ip_fil.h:
> >
> > ! #define       I60(x)  (((i6addr_t
*)(x))->i6[0])
> > ! #define       I61(x)  (((i6addr_t
*)(x))->i6[1])
> > ! #define       I62(x)  (((i6addr_t
*)(x))->i6[2])
> > ! #define       I63(x)  (((i6addr_t
*)(x))->i6[3])
> > ! #define       HI60(x) ntohl(((i6addr_t
*)(x))->i6[0])
> > ! #define       HI61(x) ntohl(((i6addr_t
*)(x))->i6[1])
> > ! #define       HI62(x) ntohl(((i6addr_t
*)(x))->i6[2])
> > ! #define       HI63(x) ntohl(((i6addr_t
*)(x))->i6[3])
> >
> >  #define       IP6_EQ(a,b)     ((I63(a) == I63(b))
&& (I62(a) ==
> > I62(b)) && 
> >                         (I61(a) == I61(b))
&& (I60(a) == I60(b)))
> > --- 156,169 ----
> >  #define       iplookupptr     vptr[0]
> >  #define       iplookupfunc    lptr[1]
> >
> > ! #define       I60(x)  (((u_32_t *)(x))[0])
> > ! #define       I61(x)  (((u_32_t *)(x))[1])
> > ! #define       I62(x)  (((u_32_t *)(x))[2])
> > ! #define       I63(x)  (((u_32_t *)(x))[3])
> > ! #define       HI60(x) ntohl(((u_32_t *)(x))[0])
> > ! #define       HI61(x) ntohl(((u_32_t *)(x))[1])
> > ! #define       HI62(x) ntohl(((u_32_t *)(x))[2])
> > ! #define       HI63(x) ntohl(((u_32_t *)(x))[3])
>
> I might not have mentioned: the system I'm having
trouble on is i386 and
> netbsd-4.  The above fix was for strict alignment
machines and the
> problem appeared on sparc64.
>
> I already have that change; it's been pulled up to
netbsd-4 - it's
> precisely this commit:
>
> revision 1.6.12.5
> date: 2007/07/21 12:56:46;  author: liamjfoy;  state:
Exp;  lines: +10 -9
> Pull up following revision(s) (requested by gdt in
ticket #779):
> 	sys/dist/ipf/netinet/fil.c: revision 1.39
> 	sys/dist/ipf/netinet/ip_fil.h: revision 1.14
> Avoid casting to "i6addr_t *", because that
type requires 64-bit
> alignment and nothing guarantees that IPv6 packets in
mbufs are 8-byte
> aligned.  gcc was coalescing adjacent 32-bit compares
into "ldx" on
> sparc64, leading to alignment faults when processing
icmp6 arriving on
> gif with IPv4 outer addresses.
> Fix mostly from darrenr.  Discussed extensively on
port-sparc64.
>   

So you're saying that doesn't work?!
What assembly got generated as the result of the above?

Darren