diff mbox series

gcc: fix uclibc build with gcc-8 for xtensa

Message ID 20180619202706.17347-1-jcmvbkbc@gmail.com
State Accepted
Commit 91e0fc0bf46fba21af72ccf753db7f012ebdc169
Headers show
Series gcc: fix uclibc build with gcc-8 for xtensa | expand

Commit Message

Max Filippov June 19, 2018, 8:27 p.m. UTC 
gcc-8.1 for xtensa miscompiles uClibc dynamic linker due to gcc PR
target/65416. The build completes successfully, but the binary is
non-functional because the following fragment in the _dl_get_ready_to_run
in ld-uClibc.so overwrites register spill area on stack causing register
corruption in the previous call frame and a subsequent crash:

    419f:       f0c1b2          addi    a11, a1, -16
    41a2:       1ba9            s32i.n  a10, a11, 4
    41a4:       0bc9            s32i.n  a12, a11, 0
    41a6:       5127f2          l32i    a15, a7, 0x144
    41a9:       1765b2          s32i    a11, a5, 92
    41ac:       4e2782          l32i    a8, a7, 0x138
    41af:       146af2          s32i    a15, a10, 80
    41b2:       001b10          movsp   a1, a11

The crash terminates the init process and causes kernel panic.
The fix prevents reordering of movsp opcode and any access to the stack
frame memory and is applicable to all existing gcc versions..

Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 .../gcc/6.4.0/871-xtensa-fix-PR-target-65416.patch | 101 +++++++++++++++++++++
 .../7.3.0/0002-xtensa-fix-PR-target-65416.patch    | 101 +++++++++++++++++++++
 .../8.1.0/0004-xtensa-fix-PR-target-65416.patch    | 101 +++++++++++++++++++++
 3 files changed, 303 insertions(+)
 create mode 100644 package/gcc/6.4.0/871-xtensa-fix-PR-target-65416.patch
 create mode 100644 package/gcc/7.3.0/0002-xtensa-fix-PR-target-65416.patch
 create mode 100644 package/gcc/8.1.0/0004-xtensa-fix-PR-target-65416.patch

Comments

Peter Korsgaard June 20, 2018, 9:06 a.m. UTC | #1 
>>>>> "Max" == Max Filippov <jcmvbkbc@gmail.com> writes:

 > gcc-8.1 for xtensa miscompiles uClibc dynamic linker due to gcc PR
 > target/65416. The build completes successfully, but the binary is
 > non-functional because the following fragment in the _dl_get_ready_to_run
 > in ld-uClibc.so overwrites register spill area on stack causing register
 > corruption in the previous call frame and a subsequent crash:

 >     419f:       f0c1b2          addi    a11, a1, -16
 >     41a2:       1ba9            s32i.n  a10, a11, 4
 >     41a4:       0bc9            s32i.n  a12, a11, 0
 >     41a6:       5127f2          l32i    a15, a7, 0x144
 >     41a9:       1765b2          s32i    a11, a5, 92
 >     41ac:       4e2782          l32i    a8, a7, 0x138
 >     41af:       146af2          s32i    a15, a10, 80
 >     41b2:       001b10          movsp   a1, a11

 > The crash terminates the init process and causes kernel panic.
 > The fix prevents reordering of movsp opcode and any access to the stack
 > frame memory and is applicable to all existing gcc versions..

As this only triggers at runtime I've reworded the subject to 'fix
uclibc runtime issue with gcc-8 for xtensa' and committed, thanks.
Romain Naour June 25, 2018, 8:50 p.m. UTC | #2 
Hi Max,

Le 19/06/2018 à 22:27, Max Filippov a écrit :
> gcc-8.1 for xtensa miscompiles uClibc dynamic linker due to gcc PR
> target/65416. The build completes successfully, but the binary is
> non-functional because the following fragment in the _dl_get_ready_to_run
> in ld-uClibc.so overwrites register spill area on stack causing register
> corruption in the previous call frame and a subsequent crash:
> 
>     419f:       f0c1b2          addi    a11, a1, -16
>     41a2:       1ba9            s32i.n  a10, a11, 4
>     41a4:       0bc9            s32i.n  a12, a11, 0
>     41a6:       5127f2          l32i    a15, a7, 0x144
>     41a9:       1765b2          s32i    a11, a5, 92
>     41ac:       4e2782          l32i    a8, a7, 0x138
>     41af:       146af2          s32i    a15, a10, 80
>     41b2:       001b10          movsp   a1, a11
> 
> The crash terminates the init process and causes kernel panic.
> The fix prevents reordering of movsp opcode and any access to the stack
> frame memory and is applicable to all existing gcc versions..
> 

Thanks for the patch!

Do you know what changed in gcc 8.0 that trigger this issue ?

I tried to use git bisect to find the culprit commit, the last working gcc
commit is 5266910fed23d6d7f101a878dd8a28d178697ec5 (2017-06-28) and this "first"
broken commit is 7051d2393d15d0d1b848768a6f2767c75aa4b5cd (2017-07-06) but I
gave up due to lack of time...

Best regards,
Romain
Max Filippov June 25, 2018, 9:13 p.m. UTC | #3 
Hi Romain,

On Mon, Jun 25, 2018 at 1:50 PM, Romain Naour <romain.naour@gmail.com> wrote:
> Do you know what changed in gcc 8.0 that trigger this issue ?

No. My guess is that it's something innocent that tweaks instruction stream
in some of the optimization passes.

> I tried to use git bisect to find the culprit commit, the last working gcc
> commit is 5266910fed23d6d7f101a878dd8a28d178697ec5 (2017-06-28) and this "first"
> broken commit is 7051d2393d15d0d1b848768a6f2767c75aa4b5cd (2017-07-06) but I
> gave up due to lack of time...

I used the following check to drive creduce to minimize the testcase,
I guess it may be reused for gcc bisection:

--->8---
grep -B 10 movsp < ldso.s > tmp
sed -ne '/add.*, sp,/,/movsp/p' < tmp > tmp1
while : ; do
        read cmd reg1 reg2
        SR=$reg1
        while read cmd reg1 reg2 reg3 ; do
                if [ "$cmd" = "movsp" ] ; then break ; fi
                if [ "$cmd" = "s32i" -o "$cmd" = "s32i.n" ] && [ $SR =
$reg2 ] ; then exit 0 ; fi
        done
done < tmp1
exit 1
---8<---

I didn't do it because it was easier to just debug the libc startup.
Peter Korsgaard July 18, 2018, 9:21 p.m. UTC | #4 
>>>>> "Max" == Max Filippov <jcmvbkbc@gmail.com> writes:

 > gcc-8.1 for xtensa miscompiles uClibc dynamic linker due to gcc PR
 > target/65416. The build completes successfully, but the binary is
 > non-functional because the following fragment in the _dl_get_ready_to_run
 > in ld-uClibc.so overwrites register spill area on stack causing register
 > corruption in the previous call frame and a subsequent crash:

 >     419f:       f0c1b2          addi    a11, a1, -16
 >     41a2:       1ba9            s32i.n  a10, a11, 4
 >     41a4:       0bc9            s32i.n  a12, a11, 0
 >     41a6:       5127f2          l32i    a15, a7, 0x144
 >     41a9:       1765b2          s32i    a11, a5, 92
 >     41ac:       4e2782          l32i    a8, a7, 0x138
 >     41af:       146af2          s32i    a15, a10, 80
 >     41b2:       001b10          movsp   a1, a11

 > The crash terminates the init process and causes kernel panic.
 > The fix prevents reordering of movsp opcode and any access to the stack
 > frame memory and is applicable to all existing gcc versions..

 > Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
 > ---
 >  .../gcc/6.4.0/871-xtensa-fix-PR-target-65416.patch | 101 +++++++++++++++++++++
 >  .../7.3.0/0002-xtensa-fix-PR-target-65416.patch    | 101 +++++++++++++++++++++
 >  .../8.1.0/0004-xtensa-fix-PR-target-65416.patch    | 101 +++++++++++++++++++++

Committed to 2018.02.x and 2018.05.x after dropping the gcc-8.x patch,
thanks.
diff mbox series

Patch

diff --git a/package/gcc/6.4.0/871-xtensa-fix-PR-target-65416.patch b/package/gcc/6.4.0/871-xtensa-fix-PR-target-65416.patch
new file mode 100644
index 000000000000..7ead575439cc
--- /dev/null
+++ b/package/gcc/6.4.0/871-xtensa-fix-PR-target-65416.patch
@@ -0,0 +1,101 @@ 
+From 87fda0741d210727672cba5e54a37a189e8ac04e Mon Sep 17 00:00:00 2001
+From: Max Filippov <jcmvbkbc@gmail.com>
+Date: Sun, 17 Jun 2018 21:18:39 -0700
+Subject: [PATCH] xtensa: fix PR target/65416
+
+The issue is caused by reordering of stack pointer update after stack
+space allocation with instructions that write to the allocated stack
+space. In windowed ABI register spill area for the previous call frame
+is located just below the stack pointer and may be reloaded back into
+the register file on movsp.
+Implement allocate_stack pattern for windowed ABI configuration and
+insert an instruction that prevents reordering of frame memory access
+and stack pointer update.
+
+gcc/
+2018-06-19  Max Filippov  <jcmvbkbc@gmail.com>
+
+	* config/xtensa/xtensa.md (UNSPEC_FRAME_BLOCKAGE): New unspec
+	constant.
+	(allocate_stack, frame_blockage, *frame_blockage): New patterns.
+
+Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
+Backported from: r261755
+---
+ gcc/config/xtensa/xtensa.md | 46 +++++++++++++++++++++++++++++++++++++++++++++
+ 1 file changed, 46 insertions(+)
+
+diff --git a/gcc/config/xtensa/xtensa.md b/gcc/config/xtensa/xtensa.md
+index 84967dbedc08..209f839cfb0f 100644
+--- a/gcc/config/xtensa/xtensa.md
++++ b/gcc/config/xtensa/xtensa.md
+@@ -38,6 +38,7 @@
+   (UNSPEC_MEMW		11)
+   (UNSPEC_LSETUP_START  12)
+   (UNSPEC_LSETUP_END    13)
++  (UNSPEC_FRAME_BLOCKAGE 14)
+ 
+   (UNSPECV_SET_FP	1)
+   (UNSPECV_ENTRY	2)
+@@ -1676,6 +1677,32 @@
+ 
+ ;; Miscellaneous instructions.
+ 
++;; In windowed ABI stack pointer adjustment must happen before any access
++;; to the space allocated on stack is allowed, otherwise register spill
++;; area may be clobbered.  That's what frame blockage is supposed to enforce.
++
++(define_expand "allocate_stack"
++  [(set (match_operand 0 "nonimmed_operand")
++        (minus (reg A1_REG) (match_operand 1 "add_operand")))
++   (set (reg A1_REG)
++        (minus (reg A1_REG) (match_dup 1)))]
++  "TARGET_WINDOWED_ABI"
++{
++  if (CONST_INT_P (operands[1]))
++    {
++      rtx neg_op0 = GEN_INT (-INTVAL (operands[1]));
++      emit_insn (gen_addsi3 (stack_pointer_rtx, stack_pointer_rtx, neg_op0));
++    }
++  else
++    {
++      emit_insn (gen_subsi3 (stack_pointer_rtx, stack_pointer_rtx,
++			     operands[1]));
++    }
++  emit_move_insn (operands[0], virtual_stack_dynamic_rtx);
++  emit_insn (gen_frame_blockage ());
++  DONE;
++})
++
+ (define_expand "prologue"
+   [(const_int 0)]
+   ""
+@@ -1767,6 +1794,25 @@
+   [(set_attr "length" "0")
+    (set_attr "type" "nop")])
+ 
++;; Do not schedule instructions accessing memory before this point.
++
++(define_expand "frame_blockage"
++  [(set (match_dup 0)
++        (unspec:BLK [(match_dup 1)] UNSPEC_FRAME_BLOCKAGE))]
++  ""
++{
++  operands[0] = gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (Pmode));
++  MEM_VOLATILE_P (operands[0]) = 1;
++  operands[1] = stack_pointer_rtx;
++})
++
++(define_insn "*frame_blockage"
++  [(set (match_operand:BLK 0 "" "")
++        (unspec:BLK [(match_operand:SI 1 "" "")] UNSPEC_FRAME_BLOCKAGE))]
++  ""
++  ""
++  [(set_attr "length" "0")])
++
+ (define_insn "trap"
+   [(trap_if (const_int 1) (const_int 0))]
+   ""
+-- 
+2.11.0
+
diff --git a/package/gcc/7.3.0/0002-xtensa-fix-PR-target-65416.patch b/package/gcc/7.3.0/0002-xtensa-fix-PR-target-65416.patch
new file mode 100644
index 000000000000..7ead575439cc
--- /dev/null
+++ b/package/gcc/7.3.0/0002-xtensa-fix-PR-target-65416.patch
@@ -0,0 +1,101 @@ 
+From 87fda0741d210727672cba5e54a37a189e8ac04e Mon Sep 17 00:00:00 2001
+From: Max Filippov <jcmvbkbc@gmail.com>
+Date: Sun, 17 Jun 2018 21:18:39 -0700
+Subject: [PATCH] xtensa: fix PR target/65416
+
+The issue is caused by reordering of stack pointer update after stack
+space allocation with instructions that write to the allocated stack
+space. In windowed ABI register spill area for the previous call frame
+is located just below the stack pointer and may be reloaded back into
+the register file on movsp.
+Implement allocate_stack pattern for windowed ABI configuration and
+insert an instruction that prevents reordering of frame memory access
+and stack pointer update.
+
+gcc/
+2018-06-19  Max Filippov  <jcmvbkbc@gmail.com>
+
+	* config/xtensa/xtensa.md (UNSPEC_FRAME_BLOCKAGE): New unspec
+	constant.
+	(allocate_stack, frame_blockage, *frame_blockage): New patterns.
+
+Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
+Backported from: r261755
+---
+ gcc/config/xtensa/xtensa.md | 46 +++++++++++++++++++++++++++++++++++++++++++++
+ 1 file changed, 46 insertions(+)
+
+diff --git a/gcc/config/xtensa/xtensa.md b/gcc/config/xtensa/xtensa.md
+index 84967dbedc08..209f839cfb0f 100644
+--- a/gcc/config/xtensa/xtensa.md
++++ b/gcc/config/xtensa/xtensa.md
+@@ -38,6 +38,7 @@
+   (UNSPEC_MEMW		11)
+   (UNSPEC_LSETUP_START  12)
+   (UNSPEC_LSETUP_END    13)
++  (UNSPEC_FRAME_BLOCKAGE 14)
+ 
+   (UNSPECV_SET_FP	1)
+   (UNSPECV_ENTRY	2)
+@@ -1676,6 +1677,32 @@
+ 
+ ;; Miscellaneous instructions.
+ 
++;; In windowed ABI stack pointer adjustment must happen before any access
++;; to the space allocated on stack is allowed, otherwise register spill
++;; area may be clobbered.  That's what frame blockage is supposed to enforce.
++
++(define_expand "allocate_stack"
++  [(set (match_operand 0 "nonimmed_operand")
++        (minus (reg A1_REG) (match_operand 1 "add_operand")))
++   (set (reg A1_REG)
++        (minus (reg A1_REG) (match_dup 1)))]
++  "TARGET_WINDOWED_ABI"
++{
++  if (CONST_INT_P (operands[1]))
++    {
++      rtx neg_op0 = GEN_INT (-INTVAL (operands[1]));
++      emit_insn (gen_addsi3 (stack_pointer_rtx, stack_pointer_rtx, neg_op0));
++    }
++  else
++    {
++      emit_insn (gen_subsi3 (stack_pointer_rtx, stack_pointer_rtx,
++			     operands[1]));
++    }
++  emit_move_insn (operands[0], virtual_stack_dynamic_rtx);
++  emit_insn (gen_frame_blockage ());
++  DONE;
++})
++
+ (define_expand "prologue"
+   [(const_int 0)]
+   ""
+@@ -1767,6 +1794,25 @@
+   [(set_attr "length" "0")
+    (set_attr "type" "nop")])
+ 
++;; Do not schedule instructions accessing memory before this point.
++
++(define_expand "frame_blockage"
++  [(set (match_dup 0)
++        (unspec:BLK [(match_dup 1)] UNSPEC_FRAME_BLOCKAGE))]
++  ""
++{
++  operands[0] = gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (Pmode));
++  MEM_VOLATILE_P (operands[0]) = 1;
++  operands[1] = stack_pointer_rtx;
++})
++
++(define_insn "*frame_blockage"
++  [(set (match_operand:BLK 0 "" "")
++        (unspec:BLK [(match_operand:SI 1 "" "")] UNSPEC_FRAME_BLOCKAGE))]
++  ""
++  ""
++  [(set_attr "length" "0")])
++
+ (define_insn "trap"
+   [(trap_if (const_int 1) (const_int 0))]
+   ""
+-- 
+2.11.0
+
diff --git a/package/gcc/8.1.0/0004-xtensa-fix-PR-target-65416.patch b/package/gcc/8.1.0/0004-xtensa-fix-PR-target-65416.patch
new file mode 100644
index 000000000000..7ead575439cc
--- /dev/null
+++ b/package/gcc/8.1.0/0004-xtensa-fix-PR-target-65416.patch
@@ -0,0 +1,101 @@ 
+From 87fda0741d210727672cba5e54a37a189e8ac04e Mon Sep 17 00:00:00 2001
+From: Max Filippov <jcmvbkbc@gmail.com>
+Date: Sun, 17 Jun 2018 21:18:39 -0700
+Subject: [PATCH] xtensa: fix PR target/65416
+
+The issue is caused by reordering of stack pointer update after stack
+space allocation with instructions that write to the allocated stack
+space. In windowed ABI register spill area for the previous call frame
+is located just below the stack pointer and may be reloaded back into
+the register file on movsp.
+Implement allocate_stack pattern for windowed ABI configuration and
+insert an instruction that prevents reordering of frame memory access
+and stack pointer update.
+
+gcc/
+2018-06-19  Max Filippov  <jcmvbkbc@gmail.com>
+
+	* config/xtensa/xtensa.md (UNSPEC_FRAME_BLOCKAGE): New unspec
+	constant.
+	(allocate_stack, frame_blockage, *frame_blockage): New patterns.
+
+Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
+Backported from: r261755
+---
+ gcc/config/xtensa/xtensa.md | 46 +++++++++++++++++++++++++++++++++++++++++++++
+ 1 file changed, 46 insertions(+)
+
+diff --git a/gcc/config/xtensa/xtensa.md b/gcc/config/xtensa/xtensa.md
+index 84967dbedc08..209f839cfb0f 100644
+--- a/gcc/config/xtensa/xtensa.md
++++ b/gcc/config/xtensa/xtensa.md
+@@ -38,6 +38,7 @@
+   (UNSPEC_MEMW		11)
+   (UNSPEC_LSETUP_START  12)
+   (UNSPEC_LSETUP_END    13)
++  (UNSPEC_FRAME_BLOCKAGE 14)
+ 
+   (UNSPECV_SET_FP	1)
+   (UNSPECV_ENTRY	2)
+@@ -1676,6 +1677,32 @@
+ 
+ ;; Miscellaneous instructions.
+ 
++;; In windowed ABI stack pointer adjustment must happen before any access
++;; to the space allocated on stack is allowed, otherwise register spill
++;; area may be clobbered.  That's what frame blockage is supposed to enforce.
++
++(define_expand "allocate_stack"
++  [(set (match_operand 0 "nonimmed_operand")
++        (minus (reg A1_REG) (match_operand 1 "add_operand")))
++   (set (reg A1_REG)
++        (minus (reg A1_REG) (match_dup 1)))]
++  "TARGET_WINDOWED_ABI"
++{
++  if (CONST_INT_P (operands[1]))
++    {
++      rtx neg_op0 = GEN_INT (-INTVAL (operands[1]));
++      emit_insn (gen_addsi3 (stack_pointer_rtx, stack_pointer_rtx, neg_op0));
++    }
++  else
++    {
++      emit_insn (gen_subsi3 (stack_pointer_rtx, stack_pointer_rtx,
++			     operands[1]));
++    }
++  emit_move_insn (operands[0], virtual_stack_dynamic_rtx);
++  emit_insn (gen_frame_blockage ());
++  DONE;
++})
++
+ (define_expand "prologue"
+   [(const_int 0)]
+   ""
+@@ -1767,6 +1794,25 @@
+   [(set_attr "length" "0")
+    (set_attr "type" "nop")])
+ 
++;; Do not schedule instructions accessing memory before this point.
++
++(define_expand "frame_blockage"
++  [(set (match_dup 0)
++        (unspec:BLK [(match_dup 1)] UNSPEC_FRAME_BLOCKAGE))]
++  ""
++{
++  operands[0] = gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (Pmode));
++  MEM_VOLATILE_P (operands[0]) = 1;
++  operands[1] = stack_pointer_rtx;
++})
++
++(define_insn "*frame_blockage"
++  [(set (match_operand:BLK 0 "" "")
++        (unspec:BLK [(match_operand:SI 1 "" "")] UNSPEC_FRAME_BLOCKAGE))]
++  ""
++  ""
++  [(set_attr "length" "0")])
++
+ (define_insn "trap"
+   [(trap_if (const_int 1) (const_int 0))]
+   ""
+-- 
+2.11.0
+