From owner-freebsd-arm@FreeBSD.ORG Wed Feb 17 17:08:45 2010 Return-Path: Delivered-To: freebsd-arm@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E0C52106566C for ; Wed, 17 Feb 2010 17:08:44 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from harmony.bsdimp.com (bsdimp.com [199.45.160.85]) by mx1.freebsd.org (Postfix) with ESMTP id 90E148FC0A for ; Wed, 17 Feb 2010 17:08:44 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by harmony.bsdimp.com (8.14.3/8.14.1) with ESMTP id o1HGxoiq048489; Wed, 17 Feb 2010 09:59:50 -0700 (MST) (envelope-from imp@bsdimp.com) Date: Wed, 17 Feb 2010 10:00:04 -0700 (MST) Message-Id: <20100217.100004.321689434032786752.imp@bsdimp.com> To: ticso@cicely.de, ticso@cicely7.cicely.de From: "M. Warner Losh" In-Reply-To: <20100217152900.GX43625@cicely7.cicely.de> References: <20100217151607.GU43625@cicely7.cicely.de> <20100217151941.GV43625@cicely7.cicely.de> <20100217152900.GX43625@cicely7.cicely.de> X-Mailer: Mew version 6.3 on Emacs 22.3 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: rpaulo@gmail.com, freebsd-arm@FreeBSD.org Subject: Re: kdump on ARM X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting FreeBSD to the StrongARM Processor List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Feb 2010 17:08:45 -0000 In message: <20100217152900.GX43625@cicely7.cicely.de> Bernd Walter writes: : On Wed, Feb 17, 2010 at 04:19:41PM +0100, Bernd Walter wrote: : > On Wed, Feb 17, 2010 at 04:16:07PM +0100, Bernd Walter wrote: : > > On Wed, Feb 17, 2010 at 02:54:05PM +0000, Rui Paulo wrote: : > > > On 17 Feb 2010, at 14:18, Grzegorz Bernacki wrote: : > > > I wonder if this can't be made non arm conditional? : > : > Ups - I'd just recovered from Mr. Sandman's work. : > So we all agree about. : > Nevertheless it should be verified if this is just a faulty struct : > definition. : > On the other hand I think it is not because someone else wrote it is : > a brokem on mips as well. : : I'm really still sleeping - noone mentioned mips at all. : > > Either this struct is properly aligned or not. : > > So why should this be made conditional? : > > Non strict alignment architecturs also have problems with this, but : > > it is usualy just speed penalties. : > > There is one ARM sepcific struct missalignment problem. : > > In this case we usually add __packed macro to structure definition. : > > For most structures this usually means no change on other : > > archtitectures and we only declare the struct to forcibly be what the : > > programmer already expected. : > > Only a few programmers are aware that they expect something from : > > structures, which is not garantied. This code is clearly nutso when it comes to alignment. I've come up with a slightly better patch. I'd though about doing the structure assignment that I suggested in a prior note, but the compiler is free to assume alignment when copying the structures, which may end badly. There's no way we can add __packed or __aligned easily to this code (although the ktrstat and ktrsockaddr routines should be able to have that annotation, a quick test suggests that the annotations I tried didn't take right). I don't have a good ARM setup at the moment to actually test these changes. Can others test them? They seem to work for me on x86, but that isn't saying much. Warner Index: kdump.c =================================================================== --- kdump.c (revision 203976) +++ kdump.c (working copy) @@ -1328,6 +1328,8 @@ char *name, *data; size_t namelen, datalen; int i; + struct stat sb; + struct sockaddr_storage ss; for (name = buf, namelen = 0; namelen < buflen && name[namelen] != '\0'; @@ -1348,12 +1350,16 @@ if (strcmp(name, "stat") == 0) { if (datalen != sizeof(struct stat)) goto invalid; - ktrstat((struct stat *)data); + memcpy(&sb, data, datalen); + ktrstat(&sb); } else if (strcmp(name, "sockaddr") == 0) { + if (datalen > sizeof(ss)) + goto invalid; + memcpy(&ss, data, datalen); if (datalen < sizeof(struct sockaddr) || - datalen != ((struct sockaddr *)(data))->sa_len) + datalen != ss.ss_len) goto invalid; - ktrsockaddr((struct sockaddr *)data); + ktrsockaddr((struct sockaddr *)&ss); } else { printf("unknown structure\n"); }