From owner-freebsd-arm@FreeBSD.ORG Mon Dec 23 19:45:22 2013 Return-Path: Delivered-To: freebsd-arm@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id A6884CD5; Mon, 23 Dec 2013 19:45:22 +0000 (UTC) Received: from h2.funkthat.com (gate2.funkthat.com [208.87.223.18]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 7D78A199F; Mon, 23 Dec 2013 19:45:22 +0000 (UTC) Received: from h2.funkthat.com (localhost [127.0.0.1]) by h2.funkthat.com (8.14.3/8.14.3) with ESMTP id rBNJjFma025793 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 23 Dec 2013 11:45:16 -0800 (PST) (envelope-from jmg@h2.funkthat.com) Received: (from jmg@localhost) by h2.funkthat.com (8.14.3/8.14.3/Submit) id rBNJjFfQ025792; Mon, 23 Dec 2013 11:45:15 -0800 (PST) (envelope-from jmg) Date: Mon, 23 Dec 2013 11:45:15 -0800 From: John-Mark Gurney To: Guy Yur Subject: Re: 10.0-RC1: net/mpd5 crashes in NgMkSockNode due to stack alignment on ARM EABI Message-ID: <20131223194515.GS99167@funkthat.com> Mail-Followup-To: Guy Yur , freebsd-arm@freebsd.org, freebsd-net@freebsd.org References: <20131221191552.GE99167@funkthat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i X-Operating-System: FreeBSD 7.2-RELEASE i386 X-PGP-Fingerprint: 54BA 873B 6515 3F10 9E88 9322 9CB1 8F74 6D3F A396 X-Files: The truth is out there X-URL: http://resnet.uoregon.edu/~gurney_j/ X-Resume: http://resnet.uoregon.edu/~gurney_j/resume.html X-to-the-FBI-CIA-and-NSA: HI! HOW YA DOIN? can i haz chizburger? X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.2.2 (h2.funkthat.com [127.0.0.1]); Mon, 23 Dec 2013 11:45:16 -0800 (PST) Cc: freebsd-net@freebsd.org, freebsd-arm@freebsd.org X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "Porting FreeBSD to ARM processors." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 23 Dec 2013 19:45:22 -0000 Guy Yur wrote this message on Sun, Dec 22, 2013 at 00:46 +0200: > On Sat, Dec 21, 2013 at 9:15 PM, John-Mark Gurney wrote: > > Guy Yur wrote this message on Sat, Dec 21, 2013 at 19:24 +0200: > >> I am running 10.0-RC1 on the BeagleBone Black and the net/mpd5 port is > >> crashing in libnetgraph NgMkSockNode due to stack alignment. > >> > >> 10.0-RC1 World and kernel were compiled in a VirtualBox VM running > >> 9.2-RELEASE-p2 i386. > >> clang and ARM_EABI used as the default make options. > >> > >> Added prints in NgMkSockNode show rbuf is aligned on 2-byte and not > >> 4-byte which is needed to access ni->id (a uint32_t). > >> > >> ni = 0xbfffe87a > >> rbuf = 0xbfffe842 > >> sizeof(resp->header) = 56 > >> > >> > >> (gdb) bt > >> #0 0x201529a0 in NgMkSockNode (name=, csp=0xbfffe95c, > >> dsp=0xbfffe958) at /usr/src/lib/libnetgraph/sock.c:134 > >> #1 0x00037b9c in MppcTestCap () at ccp_mppc.c:754 > >> #2 0x0007c1f4 in main (ac=4, av=0xbfffeb90) at main.c:248 > >> #3 0x0000d1b0 in __start (argc=4, argv=0xbfffeb90, env=0xbfffeba4, > >> ps_strings=, obj=, > >> cleanup=) at /usr/src/lib/csu/arm/crt1.c:115 > >> #4 0x203e9dc0 in _thr_ast (curthread=0x200fd000) > >> at /usr/src/lib/libthr/thread/thr_sig.c:265 > >> > >> > >> Putting rbuf in a union with struct ng_mesg sorted the alignment to > >> 4-byte and mpd5 didn't crash. > >> I attached the changes I used to test mpd5 doesn't crash with correct alignment. > > > > The patch looks correct, but lets make sure that the -net people don't > > have an issue with it... > > > > I've reattached Guy's patch for review. > > > > Guy, bug me in a week or so if I haven't committed it, and I will... > > Should I still file a PR? Yes, please. It provides a link from the mailing list to the commit and provides historical reference... > 1. I noticed my patch causes a style bug with the rbuf line now taking > 87 columns. > 2. Since the union now has a ng_mesg struct, the casting to resp can > be skipped and the union member used directly. Thanks for these. > Attached new patch breaking the rbuf line and swapping resp usage with > res.res and &res.res > Maybe a different name than res should be used for the union or the member. Maybe, but at the same time, since you define the union locally, the reader shouldn't be TOO confused... :) > I only tested the new patch compiles for arm.armv6, haven't verified it. Can you please verify it? Even though I agree you didn't change anything w/ the new patch, you'd be surprised what computers think. :) Thanks. -- John-Mark Gurney Voice: +1 415 225 5579 "All that I will do, has been done, All that I have, has not."