From patchwork Sun Mar 10 19:23:28 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicolas Pitre X-Patchwork-Id: 2245791 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork2.kernel.org (Postfix) with ESMTP id C8181DF24C for ; Sun, 10 Mar 2013 19:27:09 +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 1UElqL-0006dP-Fb; Sun, 10 Mar 2013 19:23:38 +0000 Received: from mail-qc0-x229.google.com ([2607:f8b0:400d:c01::229]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1UElqG-0006d6-TJ for linux-arm-kernel@lists.infradead.org; Sun, 10 Mar 2013 19:23:33 +0000 Received: by mail-qc0-f169.google.com with SMTP id t2so1236088qcq.0 for ; Sun, 10 Mar 2013 12:23:31 -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=wsBInFMkKse6FAsuqc9DAdZLjsytNXNgkBSe9z4KO0M=; b=OxPH7OaPWY1YMME1ktg+iUskgb9jqPGdEHvsKW7egFlylTxVH7rOunJvHi8MZF90UG AbTGQ542LIS7pd0JhRSkJg8HHcOhzXhL4k+Wu7K6IFc7SfF6b5gilUuPZQwjGrshsRHb cIsfntaK07vbHO7M0ci1IAJMEzP9TUjXyhpMlLmzvQWdcr9HhNdlixdF3WkXt0Jx7xgR PTmF2FVM6ypfIK9z5PYe48En99bDuTfPsTXOgHABN8kLHmiod6pkUyraNBFkCG3Nqx7B 5ZfPSMTV45PQ0f32ifzfV37YjtrlseUwYqcR/3CYKByjkTKWQZRTlw6Bn+niu7l1GPp0 CDtg== X-Received: by 10.224.201.201 with SMTP id fb9mr14111873qab.54.1362943411471; Sun, 10 Mar 2013 12:23:31 -0700 (PDT) Received: from xanadu.home (modemcable203.213-202-24.mc.videotron.ca. [24.202.213.203]) by mx.google.com with ESMTPS id et9sm21321373qab.9.2013.03.10.12.23.29 (version=TLSv1 cipher=RC4-SHA bits=128/128); Sun, 10 Mar 2013 12:23:30 -0700 (PDT) Date: Mon, 11 Mar 2013 03:23:28 +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: 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: ALoCoQnL3gAkxBIYqMoKd9pEMRU8ym9QjEbrhTPgsRIyeoqfF/p4OJvGZEw23kULFHl5kUVCcGDF X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130310_152333_079910_9646A10D X-CRM114-Status: GOOD ( 23.91 ) 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 Mon, 11 Mar 2013, Nicolas Pitre wrote: > 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. Look carefully at this > > fragment of code: > > Dang. Indeed. > > Sorry about that. Here's what I'd fold into the original patch to fix it. I also moved the alignment fixup code to the end as the entry alignment isn't right with Thumb mode anyway. And reworked the initial test to help dual issue pipelines. > > > Nicolas > diff --git a/arch/arm/lib/memset.S b/arch/arm/lib/memset.S index d912e7397e..cf34788237 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 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)) + b 1b ENDPROC(memset)