Skip to content

Commit

Permalink
Revert "ARM: 7668/1: fix memset-related crashes caused by recent GCC …
Browse files Browse the repository at this point in the history
…(4.7.2) optimizations"

This reverts commit 690c641.

There have been various reports of FAT partition corruption following this commit. Reverting for now to see if things improve.
  • Loading branch information
popcornmix committed Mar 29, 2013
1 parent 79ec5aa commit bb9948f
Showing 1 changed file with 41 additions and 44 deletions.
85 changes: 41 additions & 44 deletions arch/arm/lib/memset.S
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,20 @@
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
strltb r1, [r0], #1 @ 1
strleb r1, [r0], #1 @ 1
strb r1, [r0], #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?
ands r3, r0, #3 @ 1 unaligned?
bne 1b @ 1
/*
* we know that the pointer in ip is aligned to a word boundary.
* we know that the pointer in r0 is aligned to a word boundary.
*/
orr r1, r1, r1, lsl #8
orr r1, r1, r1, lsl #16
Expand All @@ -47,28 +43,29 @@ ENTRY(memset)
#if ! CALGN(1)+0

/*
* We need 2 extra registers for this loop - use r8 and the LR
* We need an extra register for this loop - save the return address and
* use the LR
*/
stmfd sp!, {r8, lr}
mov r8, r1
str lr, [sp, #-4]!
mov ip, r1
mov lr, r1

2: subs r2, r2, #64
stmgeia ip!, {r1, r3, r8, lr} @ 64 bytes at a time.
stmgeia ip!, {r1, r3, r8, lr}
stmgeia ip!, {r1, r3, r8, lr}
stmgeia ip!, {r1, r3, r8, lr}
stmgeia r0!, {r1, r3, ip, lr} @ 64 bytes at a time.
stmgeia r0!, {r1, r3, ip, lr}
stmgeia r0!, {r1, r3, ip, lr}
stmgeia r0!, {r1, r3, ip, lr}
bgt 2b
ldmeqfd sp!, {r8, pc} @ Now <64 bytes to go.
ldmeqfd sp!, {pc} @ Now <64 bytes to go.
/*
* No need to correct the count; we're only testing bits from now on
*/
tst r2, #32
stmneia ip!, {r1, r3, r8, lr}
stmneia ip!, {r1, r3, r8, lr}
stmneia r0!, {r1, r3, ip, lr}
stmneia r0!, {r1, r3, ip, lr}
tst r2, #16
stmneia ip!, {r1, r3, r8, lr}
ldmfd sp!, {r8, lr}
stmneia r0!, {r1, r3, ip, lr}
ldr lr, [sp], #4

#else

Expand All @@ -77,54 +74,54 @@ ENTRY(memset)
* whole cache lines at once.
*/

stmfd sp!, {r4-r8, lr}
stmfd sp!, {r4-r7, lr}
mov r4, r1
mov r5, r1
mov r6, r1
mov r7, r1
mov r8, r1
mov ip, r1
mov lr, r1

cmp r2, #96
tstgt ip, #31
tstgt r0, #31
ble 3f

and r8, ip, #31
rsb r8, r8, #32
sub r2, r2, r8
movs r8, r8, lsl #(32 - 4)
stmcsia ip!, {r4, r5, r6, r7}
stmmiia ip!, {r4, r5}
tst r8, #(1 << 30)
mov r8, r1
strne r1, [ip], #4
and ip, r0, #31
rsb ip, ip, #32
sub r2, r2, ip
movs ip, ip, lsl #(32 - 4)
stmcsia r0!, {r4, r5, r6, r7}
stmmiia r0!, {r4, r5}
tst ip, #(1 << 30)
mov ip, r1
strne r1, [r0], #4

3: subs r2, r2, #64
stmgeia ip!, {r1, r3-r8, lr}
stmgeia ip!, {r1, r3-r8, lr}
stmgeia r0!, {r1, r3-r7, ip, lr}
stmgeia r0!, {r1, r3-r7, ip, lr}
bgt 3b
ldmeqfd sp!, {r4-r8, pc}
ldmeqfd sp!, {r4-r7, pc}

tst r2, #32
stmneia ip!, {r1, r3-r8, lr}
stmneia r0!, {r1, r3-r7, ip, lr}
tst r2, #16
stmneia ip!, {r4-r7}
ldmfd sp!, {r4-r8, lr}
stmneia r0!, {r4-r7}
ldmfd sp!, {r4-r7, lr}

#endif

4: tst r2, #8
stmneia ip!, {r1, r3}
stmneia r0!, {r1, r3}
tst r2, #4
strne r1, [ip], #4
strne r1, [r0], #4
/*
* When we get here, we've got less than 4 bytes to zero. We
* may have an unaligned pointer as well.
*/
5: tst r2, #2
strneb r1, [ip], #1
strneb r1, [ip], #1
strneb r1, [r0], #1
strneb r1, [r0], #1
tst r2, #1
strneb r1, [ip], #1
strneb r1, [r0], #1
mov pc, lr
ENDPROC(memset)

2 comments on commit bb9948f

@fastcat
Copy link

@fastcat fastcat commented on bb9948f Apr 5, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this was found to be buggy, shouldn't the upstream proper fix (https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/arch/arm/lib/memset.S?id=418df63adac56841ef6b0f1fcf435bc64d4ed177 as you noted in another spot) be pulled in instead of backing out to the prior also buggy state?

@hglm
Copy link

@hglm hglm commented on bb9948f Jun 27, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I second the last comment. The commit was fixed upstream with a second commit, and the complete proper fix should enable gcc 4.8 to be used to compile the kernel while also fixing potential issues with the current implementation.

https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/arch/arm/lib/memset.S?id=455bd4c430b0c0a361f38e8658a0d6cb469942b5

https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/arch/arm/lib/memset.S?id=418df63adac56841ef6b0f1fcf435bc64d4ed177

Please sign in to comment.