linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] arm: fix memset-related crashes caused by recent GCC (4.7.2) optimizations
Date: Sun, 10 Mar 2013 17:28:54 +0000	[thread overview]
Message-ID: <20130310172854.GH4977@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <513CBD83.7040909@ahsoftware.de>

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:

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

and consider what happens when 'r0' is not aligned to a word... We end
up aligning the pointer in "1:" and then fall through into memset again
which reloads the old misaligned pointer.

  reply	other threads:[~2013-03-10 17:28 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-11 12:57 [PATCH v2] arm: fix memset-related crashes caused by recent GCC (4.7.2) optimizations Ivan Djelic
2013-03-06 19:15 ` Dirk Behme
2013-03-07  8:13   ` Ivan Djelic
2013-03-07 15:17   ` Russell King - ARM Linux
2013-03-10 17:06     ` Alexander Holler
2013-03-10 17:28       ` Russell King - ARM Linux [this message]
2013-03-10 17:41         ` Alexander Holler
2013-03-10 18:46         ` Nicolas Pitre
2013-03-10 19:23           ` Nicolas Pitre
2013-03-10 20:46         ` Ivan Djelic
2013-03-11  4:10         ` Nicolas Pitre
2013-03-12 10:06           ` Alexander Holler
2014-02-26  0:43             ` Florian Fainelli
2014-02-26 17:13               ` Dirk Behme

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130310172854.GH4977@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).