From patchwork Mon Mar 11 04:10:34 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicolas Pitre X-Patchwork-Id: 2246441 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork1.kernel.org (Postfix) with ESMTP id 1A76C3FCF6 for ; Mon, 11 Mar 2013 04:13:52 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1UEu4R-0004IV-TT; Mon, 11 Mar 2013 04:10:43 +0000 Received: from mail-qc0-x231.google.com ([2607:f8b0:400d:c01::231]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1UEu4N-0004IC-LA for linux-arm-kernel@lists.infradead.org; Mon, 11 Mar 2013 04:10:40 +0000 Received: by mail-qc0-f177.google.com with SMTP id u28so1329595qcs.36 for ; Sun, 10 Mar 2013 21:10:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:date:from:to:cc:subject:in-reply-to:message-id :references:user-agent:mime-version:content-type:x-gm-message-state; bh=W6/lA2628jMVG5QeOhikkgbZCPgYvPB9ykLac3sXKFY=; b=XHdl+BkY9wnY2c4QMtxdKzO7QAJgnwpm2woGX2VJldtyWIimhq3huUK2mdJ4VElxq7 JwBuvK0P6XTYDOxuxKL+CWXB/U4t/rbr2cqeAzh8S/GWqDifAtlRMEJPLlxv1ktsC0r/ mdR+0qwHjd6tCoExfScXkueMgUr0ZfOXbWfsZltFK5kke41PA9m42isSLe9Epj27yq04 spPNLxG2X78aFvSAwRBF3WhTbuER9RGa48uezWrrr2475qN+5X3jXUTRuM4U51DYIULC i11cqS3Amc1qIu8Bby7plIkL8MC+IO37ebtshaZCUysYSo3HtckpVOkxEPA0hLLnZD5d qd4g== X-Received: by 10.224.216.8 with SMTP id hg8mr15203932qab.74.1362975036952; Sun, 10 Mar 2013 21:10:36 -0700 (PDT) Received: from xanadu.home (modemcable203.213-202-24.mc.videotron.ca. [24.202.213.203]) by mx.google.com with ESMTPS id o5sm23641530qao.12.2013.03.10.21.10.35 (version=TLSv1 cipher=RC4-SHA bits=128/128); Sun, 10 Mar 2013 21:10:36 -0700 (PDT) Date: Mon, 11 Mar 2013 12:10:34 +0800 (HKT) From: Nicolas Pitre To: Russell King - ARM Linux Subject: Re: [PATCH v2] arm: fix memset-related crashes caused by recent GCC (4.7.2) optimizations In-Reply-To: <20130310172854.GH4977@n2100.arm.linux.org.uk> Message-ID: References: <1360587435-28386-1-git-send-email-ivan.djelic@parrot.com> <513795C5.4050608@gmail.com> <20130307151755.GB4977@n2100.arm.linux.org.uk> <513CBD83.7040909@ahsoftware.de> <20130310172854.GH4977@n2100.arm.linux.org.uk> User-Agent: Alpine 2.03 (LFD 1266 2009-07-14) MIME-Version: 1.0 X-Gm-Message-State: ALoCoQn9fKZekxdp8PYnq+gcoFGz6hOxZrxnZTx1T1Hu+V3XAWEvHC122Xsc36ERc8+GecbkglK5 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130311_001039_885329_36EAB6B1 X-CRM114-Status: GOOD ( 24.96 ) X-Spam-Score: -1.9 (-) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-1.9 points) pts rule name description ---- ---------------------- -------------------------------------------------- -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Greg Kroah-Hartman , Dirk Behme , Catalin Marinas , Alexander Holler , Ivan Djelic , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org On Sun, 10 Mar 2013, Russell King - ARM Linux wrote: > On Sun, Mar 10, 2013 at 06:06:11PM +0100, Alexander Holler wrote: > > Am 07.03.2013 16:17, schrieb Russell King - ARM Linux: > >> On Wed, Mar 06, 2013 at 08:15:17PM +0100, Dirk Behme wrote: > >>> Am 11.02.2013 13:57, schrieb Ivan Djelic: > >>>> Recent GCC versions (e.g. GCC-4.7.2) perform optimizations based on > >>>> assumptions about the implementation of memset and similar functions. > >>>> The current ARM optimized memset code does not return the value of > >>>> its first argument, as is usually expected from standard implementations. > > > > I've just tried this patch with kernel 4.8.2 on an armv5-system where I > > use gcc 4.7.2 since several months and where most parts of the system > > are compiled with gcc 4.7.2 too. > > > > And I had at least one problem which manifested itself with > > Yes, the patch _is_ wrong. Reverted. I was trusting Nicolas' review > of it, but the patch is definitely wrong. Worse: it is in v3.9-rc2 already. Here's a fix. Patch system? ---------- >8 Subject: fix the memset fix Commit 455bd4c430b0 ("ARM: 7668/1: fix memset-related crashes caused by recent GCC (4.7.2) optimizations") attempted to fix a compliance issue with the memset return value. However the memset itself was broken by that patch in the misaligned pointer case. This fixes the above by branching over the entry code from the misaligned fixup code to avoid reloading the original pointer. Also, because the function entry alignment is wrong in the Thumb mode compilation, that fixup code is moved to the end. While at it, the entry instructions are slightly reworked to help dual issue pipelines. Signed-off-by: Nicolas Pitre Tested-by: Alexander Holler diff --git a/arch/arm/lib/memset.S b/arch/arm/lib/memset.S index d912e7397e..94b0650ea9 100644 --- a/arch/arm/lib/memset.S +++ b/arch/arm/lib/memset.S @@ -14,31 +14,15 @@ .text .align 5 - .word 0 - -1: subs r2, r2, #4 @ 1 do we have enough - blt 5f @ 1 bytes to align with? - cmp r3, #2 @ 1 - strltb r1, [ip], #1 @ 1 - strleb r1, [ip], #1 @ 1 - strb r1, [ip], #1 @ 1 - add r2, r2, r3 @ 1 (r2 = r2 - (4 - r3)) -/* - * The pointer is now aligned and the length is adjusted. Try doing the - * memset again. - */ ENTRY(memset) -/* - * Preserve the contents of r0 for the return value. - */ - mov ip, r0 - ands r3, ip, #3 @ 1 unaligned? - bne 1b @ 1 + ands r3, r0, #3 @ 1 unaligned? + mov ip, r0 @ preserve r0 as return value + bne 6f @ 1 /* * we know that the pointer in ip is aligned to a word boundary. */ - orr r1, r1, r1, lsl #8 +1: orr r1, r1, r1, lsl #8 orr r1, r1, r1, lsl #16 mov r3, r1 cmp r2, #16 @@ -127,4 +111,13 @@ ENTRY(memset) tst r2, #1 strneb r1, [ip], #1 mov pc, lr + +6: subs r2, r2, #4 @ 1 do we have enough + blt 5b @ 1 bytes to align with? + cmp r3, #2 @ 1 + strltb r1, [ip], #1 @ 1 + strleb r1, [ip], #1 @ 1 + strb r1, [ip], #1 @ 1 + add r2, r2, r3 @ 1 (r2 = r2 - (4 - r3)) + b 1b ENDPROC(memset)