[mickledore][PATCH] gcc: backport a fix for ICE caused by CVE-2023-4039.patch

[mickledore][PATCH] gcc: backport a fix for ICE caused by CVE-2023-4039.patch

[Martin Jansa]

* see:
  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111418
  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111411

Signed-off-by: Martin Jansa <martin.jansa@gmail.com>
---
 meta/recipes-devtools/gcc/gcc-12.3.inc        |   1 +
 ...ch64-Fix-loose-ldpstp-check-PR111411.patch | 117 ++++++++++++++++++
 2 files changed, 118 insertions(+)
 create mode 100644 meta/recipes-devtools/gcc/gcc/0026-aarch64-Fix-loose-ldpstp-check-PR111411.patch

diff --git a/meta/recipes-devtools/gcc/gcc-12.3.inc b/meta/recipes-devtools/gcc/gcc-12.3.inc
index 5896f26e1a..5655b6f46d 100644
--- a/meta/recipes-devtools/gcc/gcc-12.3.inc
+++ b/meta/recipes-devtools/gcc/gcc-12.3.inc
@@ -64,6 +64,7 @@ SRC_URI = "${BASEURI} \
            file://prefix-map-realpath.patch \
            file://hardcoded-paths.patch \
            file://CVE-2023-4039.patch \
+           file://0026-aarch64-Fix-loose-ldpstp-check-PR111411.patch \
 "
 SRC_URI[sha256sum] = "949a5d4f99e786421a93b532b22ffab5578de7321369975b91aec97adfda8c3b"
 
diff --git a/meta/recipes-devtools/gcc/gcc/0026-aarch64-Fix-loose-ldpstp-check-PR111411.patch b/meta/recipes-devtools/gcc/gcc/0026-aarch64-Fix-loose-ldpstp-check-PR111411.patch
new file mode 100644
index 0000000000..a408a98698
--- /dev/null
+++ b/meta/recipes-devtools/gcc/gcc/0026-aarch64-Fix-loose-ldpstp-check-PR111411.patch
@@ -0,0 +1,117 @@
+From adb60dc78e0da4877747f32347cee339364775be Mon Sep 17 00:00:00 2001
+From: Richard Sandiford <richard.sandiford@arm.com>
+Date: Fri, 15 Sep 2023 09:19:14 +0100
+Subject: [PATCH] aarch64: Fix loose ldpstp check [PR111411]
+
+aarch64_operands_ok_for_ldpstp contained the code:
+
+  /* One of the memory accesses must be a mempair operand.
+     If it is not the first one, they need to be swapped by the
+     peephole.  */
+  if (!aarch64_mem_pair_operand (mem_1, GET_MODE (mem_1))
+       && !aarch64_mem_pair_operand (mem_2, GET_MODE (mem_2)))
+    return false;
+
+But the requirement isn't just that one of the accesses must be a
+valid mempair operand.  It's that the lower access must be, since
+that's the access that will be used for the instruction operand.
+
+gcc/
+	PR target/111411
+	* config/aarch64/aarch64.cc (aarch64_operands_ok_for_ldpstp): Require
+	the lower memory access to a mem-pair operand.
+
+gcc/testsuite/
+	PR target/111411
+	* gcc.dg/rtl/aarch64/pr111411.c: New test.
+
+Upstream-Status: Backport [https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=2d38f45bcca62ca0c7afef4b579f82c5c2a01610]
+Signed-off-by: Martin Jansa <martin.jansa@gmail.com>
+---
+ gcc/config/aarch64/aarch64.cc               |  8 ++-
+ gcc/testsuite/gcc.dg/rtl/aarch64/pr111411.c | 57 +++++++++++++++++++++
+ 2 files changed, 60 insertions(+), 5 deletions(-)
+ create mode 100644 gcc/testsuite/gcc.dg/rtl/aarch64/pr111411.c
+
+diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
+index 6118a3354ac..9b1f791ca8b 100644
+--- a/gcc/config/aarch64/aarch64.cc
++++ b/gcc/config/aarch64/aarch64.cc
+@@ -26154,11 +26154,9 @@ aarch64_operands_ok_for_ldpstp (rtx *operands, bool load,
+   gcc_assert (known_eq (GET_MODE_SIZE (GET_MODE (mem_1)),
+ 			GET_MODE_SIZE (GET_MODE (mem_2))));
+ 
+-  /* One of the memory accesses must be a mempair operand.
+-     If it is not the first one, they need to be swapped by the
+-     peephole.  */
+-  if (!aarch64_mem_pair_operand (mem_1, GET_MODE (mem_1))
+-       && !aarch64_mem_pair_operand (mem_2, GET_MODE (mem_2)))
++  /* The lower memory access must be a mem-pair operand.  */
++  rtx lower_mem = reversed ? mem_2 : mem_1;
++  if (!aarch64_mem_pair_operand (lower_mem, GET_MODE (lower_mem)))
+     return false;
+ 
+   if (REG_P (reg_1) && FP_REGNUM_P (REGNO (reg_1)))
+diff --git a/gcc/testsuite/gcc.dg/rtl/aarch64/pr111411.c b/gcc/testsuite/gcc.dg/rtl/aarch64/pr111411.c
+new file mode 100644
+index 00000000000..ad07e9c6c89
+--- /dev/null
++++ b/gcc/testsuite/gcc.dg/rtl/aarch64/pr111411.c
+@@ -0,0 +1,57 @@
++/* { dg-do compile { target aarch64*-*-* } } */
++/* { dg-require-effective-target lp64 } */
++/* { dg-options "-O -fdisable-rtl-postreload -fpeephole2 -fno-schedule-fusion" } */
++
++extern int data[];
++
++void __RTL (startwith ("ira")) foo (void *ptr)
++{
++  (function "foo"
++    (param "ptr"
++      (DECL_RTL (reg/v:DI <0> [ ptr ]))
++      (DECL_RTL_INCOMING (reg/v:DI x0 [ ptr ]))
++    ) ;; param "ptr"
++    (insn-chain
++      (block 2
++	(edge-from entry (flags "FALLTHRU"))
++	(cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
++	(insn 4 (set (reg:DI <0>) (reg:DI x0)))
++	(insn 5 (set (reg:DI <1>)
++		     (plus:DI (reg:DI <0>) (const_int 768))))
++	(insn 6 (set (mem:SI (plus:DI (reg:DI <0>)
++				      (const_int 508)) [1 &data+508 S4 A4])
++		     (const_int 0)))
++	(insn 7 (set (mem:SI (plus:DI (reg:DI <1>)
++				      (const_int -256)) [1 &data+512 S4 A4])
++		     (const_int 0)))
++	(edge-to exit (flags "FALLTHRU"))
++      ) ;; block 2
++    ) ;; insn-chain
++  ) ;; function
++}
++
++void __RTL (startwith ("ira")) bar (void *ptr)
++{
++  (function "bar"
++    (param "ptr"
++      (DECL_RTL (reg/v:DI <0> [ ptr ]))
++      (DECL_RTL_INCOMING (reg/v:DI x0 [ ptr ]))
++    ) ;; param "ptr"
++    (insn-chain
++      (block 2
++	(edge-from entry (flags "FALLTHRU"))
++	(cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
++	(insn 4 (set (reg:DI <0>) (reg:DI x0)))
++	(insn 5 (set (reg:DI <1>)
++		     (plus:DI (reg:DI <0>) (const_int 768))))
++	(insn 6 (set (mem:SI (plus:DI (reg:DI <1>)
++				      (const_int -256)) [1 &data+512 S4 A4])
++		     (const_int 0)))
++	(insn 7 (set (mem:SI (plus:DI (reg:DI <0>)
++				      (const_int 508)) [1 &data+508 S4 A4])
++		     (const_int 0)))
++	(edge-to exit (flags "FALLTHRU"))
++      ) ;; block 2
++    ) ;; insn-chain
++  ) ;; function
++}
-- 
2.42.0

Re: [OE-core] [mickledore][PATCH] gcc: backport a fix for ICE caused by CVE-2023-4039.patch

[Steve Sakoman]

The CVE-2023-4039.patch was also submitted for kirkstone and dunfell.
Should I attempt to cherry-pick this fix for those branches too?

Steve

On Fri, Sep 15, 2023 at 10:42 AM Martin Jansa <martin.jansa@gmail.com> wrote:
>
> * see:
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111418
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111411
>
> Signed-off-by: Martin Jansa <martin.jansa@gmail.com>
> ---
>  meta/recipes-devtools/gcc/gcc-12.3.inc        |   1 +
>  ...ch64-Fix-loose-ldpstp-check-PR111411.patch | 117 ++++++++++++++++++
>  2 files changed, 118 insertions(+)
>  create mode 100644 meta/recipes-devtools/gcc/gcc/0026-aarch64-Fix-loose-ldpstp-check-PR111411.patch
>
> diff --git a/meta/recipes-devtools/gcc/gcc-12.3.inc b/meta/recipes-devtools/gcc/gcc-12.3.inc
> index 5896f26e1a..5655b6f46d 100644
> --- a/meta/recipes-devtools/gcc/gcc-12.3.inc
> +++ b/meta/recipes-devtools/gcc/gcc-12.3.inc
> @@ -64,6 +64,7 @@ SRC_URI = "${BASEURI} \
>             file://prefix-map-realpath.patch \
>             file://hardcoded-paths.patch \
>             file://CVE-2023-4039.patch \
> +           file://0026-aarch64-Fix-loose-ldpstp-check-PR111411.patch \
>  "
>  SRC_URI[sha256sum] = "949a5d4f99e786421a93b532b22ffab5578de7321369975b91aec97adfda8c3b"
>
> diff --git a/meta/recipes-devtools/gcc/gcc/0026-aarch64-Fix-loose-ldpstp-check-PR111411.patch b/meta/recipes-devtools/gcc/gcc/0026-aarch64-Fix-loose-ldpstp-check-PR111411.patch
> new file mode 100644
> index 0000000000..a408a98698
> --- /dev/null
> +++ b/meta/recipes-devtools/gcc/gcc/0026-aarch64-Fix-loose-ldpstp-check-PR111411.patch
> @@ -0,0 +1,117 @@
> +From adb60dc78e0da4877747f32347cee339364775be Mon Sep 17 00:00:00 2001
> +From: Richard Sandiford <richard.sandiford@arm.com>
> +Date: Fri, 15 Sep 2023 09:19:14 +0100
> +Subject: [PATCH] aarch64: Fix loose ldpstp check [PR111411]
> +
> +aarch64_operands_ok_for_ldpstp contained the code:
> +
> +  /* One of the memory accesses must be a mempair operand.
> +     If it is not the first one, they need to be swapped by the
> +     peephole.  */
> +  if (!aarch64_mem_pair_operand (mem_1, GET_MODE (mem_1))
> +       && !aarch64_mem_pair_operand (mem_2, GET_MODE (mem_2)))
> +    return false;
> +
> +But the requirement isn't just that one of the accesses must be a
> +valid mempair operand.  It's that the lower access must be, since
> +that's the access that will be used for the instruction operand.
> +
> +gcc/
> +       PR target/111411
> +       * config/aarch64/aarch64.cc (aarch64_operands_ok_for_ldpstp): Require
> +       the lower memory access to a mem-pair operand.
> +
> +gcc/testsuite/
> +       PR target/111411
> +       * gcc.dg/rtl/aarch64/pr111411.c: New test.
> +
> +Upstream-Status: Backport [https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=2d38f45bcca62ca0c7afef4b579f82c5c2a01610]
> +Signed-off-by: Martin Jansa <martin.jansa@gmail.com>
> +---
> + gcc/config/aarch64/aarch64.cc               |  8 ++-
> + gcc/testsuite/gcc.dg/rtl/aarch64/pr111411.c | 57 +++++++++++++++++++++
> + 2 files changed, 60 insertions(+), 5 deletions(-)
> + create mode 100644 gcc/testsuite/gcc.dg/rtl/aarch64/pr111411.c
> +
> +diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> +index 6118a3354ac..9b1f791ca8b 100644
> +--- a/gcc/config/aarch64/aarch64.cc
> ++++ b/gcc/config/aarch64/aarch64.cc
> +@@ -26154,11 +26154,9 @@ aarch64_operands_ok_for_ldpstp (rtx *operands, bool load,
> +   gcc_assert (known_eq (GET_MODE_SIZE (GET_MODE (mem_1)),
> +                       GET_MODE_SIZE (GET_MODE (mem_2))));
> +
> +-  /* One of the memory accesses must be a mempair operand.
> +-     If it is not the first one, they need to be swapped by the
> +-     peephole.  */
> +-  if (!aarch64_mem_pair_operand (mem_1, GET_MODE (mem_1))
> +-       && !aarch64_mem_pair_operand (mem_2, GET_MODE (mem_2)))
> ++  /* The lower memory access must be a mem-pair operand.  */
> ++  rtx lower_mem = reversed ? mem_2 : mem_1;
> ++  if (!aarch64_mem_pair_operand (lower_mem, GET_MODE (lower_mem)))
> +     return false;
> +
> +   if (REG_P (reg_1) && FP_REGNUM_P (REGNO (reg_1)))
> +diff --git a/gcc/testsuite/gcc.dg/rtl/aarch64/pr111411.c b/gcc/testsuite/gcc.dg/rtl/aarch64/pr111411.c
> +new file mode 100644
> +index 00000000000..ad07e9c6c89
> +--- /dev/null
> ++++ b/gcc/testsuite/gcc.dg/rtl/aarch64/pr111411.c
> +@@ -0,0 +1,57 @@
> ++/* { dg-do compile { target aarch64*-*-* } } */
> ++/* { dg-require-effective-target lp64 } */
> ++/* { dg-options "-O -fdisable-rtl-postreload -fpeephole2 -fno-schedule-fusion" } */
> ++
> ++extern int data[];
> ++
> ++void __RTL (startwith ("ira")) foo (void *ptr)
> ++{
> ++  (function "foo"
> ++    (param "ptr"
> ++      (DECL_RTL (reg/v:DI <0> [ ptr ]))
> ++      (DECL_RTL_INCOMING (reg/v:DI x0 [ ptr ]))
> ++    ) ;; param "ptr"
> ++    (insn-chain
> ++      (block 2
> ++      (edge-from entry (flags "FALLTHRU"))
> ++      (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
> ++      (insn 4 (set (reg:DI <0>) (reg:DI x0)))
> ++      (insn 5 (set (reg:DI <1>)
> ++                   (plus:DI (reg:DI <0>) (const_int 768))))
> ++      (insn 6 (set (mem:SI (plus:DI (reg:DI <0>)
> ++                                    (const_int 508)) [1 &data+508 S4 A4])
> ++                   (const_int 0)))
> ++      (insn 7 (set (mem:SI (plus:DI (reg:DI <1>)
> ++                                    (const_int -256)) [1 &data+512 S4 A4])
> ++                   (const_int 0)))
> ++      (edge-to exit (flags "FALLTHRU"))
> ++      ) ;; block 2
> ++    ) ;; insn-chain
> ++  ) ;; function
> ++}
> ++
> ++void __RTL (startwith ("ira")) bar (void *ptr)
> ++{
> ++  (function "bar"
> ++    (param "ptr"
> ++      (DECL_RTL (reg/v:DI <0> [ ptr ]))
> ++      (DECL_RTL_INCOMING (reg/v:DI x0 [ ptr ]))
> ++    ) ;; param "ptr"
> ++    (insn-chain
> ++      (block 2
> ++      (edge-from entry (flags "FALLTHRU"))
> ++      (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
> ++      (insn 4 (set (reg:DI <0>) (reg:DI x0)))
> ++      (insn 5 (set (reg:DI <1>)
> ++                   (plus:DI (reg:DI <0>) (const_int 768))))
> ++      (insn 6 (set (mem:SI (plus:DI (reg:DI <1>)
> ++                                    (const_int -256)) [1 &data+512 S4 A4])
> ++                   (const_int 0)))
> ++      (insn 7 (set (mem:SI (plus:DI (reg:DI <0>)
> ++                                    (const_int 508)) [1 &data+508 S4 A4])
> ++                   (const_int 0)))
> ++      (edge-to exit (flags "FALLTHRU"))
> ++      ) ;; block 2
> ++    ) ;; insn-chain
> ++  ) ;; function
> ++}
> --
> 2.42.0
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#187768): https://lists.openembedded.org/g/openembedded-core/message/187768
> Mute This Topic: https://lists.openembedded.org/mt/101387993/3620601
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [steve@sakoman.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

Re: [OE-core] [mickledore][PATCH] gcc: backport a fix for ICE caused by CVE-2023-4039.patch

[Ross Burton]

On 18 Sep 2023, at 16:48, Steve Sakoman <steve@sakoman.com> wrote:
> 
> The CVE-2023-4039.patch was also submitted for kirkstone and dunfell.
> Should I attempt to cherry-pick this fix for those branches too?

The bug says that the patch only applies cleaning to GCC 12/13, I’m seeking clarification as to whether that means it’s not relevant for older branches or whether a proper backport will be needed.

Ross

Re: [OE-core] [mickledore][PATCH] gcc: backport a fix for ICE caused by CVE-2023-4039.patch

[Steve Sakoman]

On Mon, Sep 18, 2023 at 6:08 AM Ross Burton <Ross.Burton@arm.com> wrote:
>
> On 18 Sep 2023, at 16:48, Steve Sakoman <steve@sakoman.com> wrote:
> >
> > The CVE-2023-4039.patch was also submitted for kirkstone and dunfell.
> > Should I attempt to cherry-pick this fix for those branches too?
>
> The bug says that the patch only applies cleaning to GCC 12/13, I’m seeking clarification as to whether that means it’s not relevant for older branches or whether a proper backport will be needed.

I did a quick experiment with kirkstone and there were two issues with
cherry-picking:

1. The test suite hunk failed since the test file doesn't exist in gcc 11
2. The first hunk fails to apply since the file name being modified in
gcc 11 is a .c, apparently it was changed to .cc in later releases.
You took this into account in the CVE patch file

Removing the testsuite hunk and hacking the filename allowed the first
hunk to apply (but with fuzz errors).

So I'd say we definitely need a proper backport if the issue is
relevant for older branches.

I'll pull CVE-2023-4039.patch from kirkstone and dunfell until we have
clarification.

Thanks!

Steve

Re: [OE-core] [mickledore][PATCH] gcc: backport a fix for ICE caused by CVE-2023-4039.patch

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 1473 bytes --]

I've intentionally sent it only for mickledore as the gcc version in
kirkstone and dunfell isn't affected by this (at least my reproducer isn't).

On Mon, Sep 18, 2023 at 6:42 PM Steve Sakoman <steve@sakoman.com> wrote:

> On Mon, Sep 18, 2023 at 6:08 AM Ross Burton <Ross.Burton@arm.com> wrote:
> >
> > On 18 Sep 2023, at 16:48, Steve Sakoman <steve@sakoman.com> wrote:
> > >
> > > The CVE-2023-4039.patch was also submitted for kirkstone and dunfell.
> > > Should I attempt to cherry-pick this fix for those branches too?
> >
> > The bug says that the patch only applies cleaning to GCC 12/13, I’m
> seeking clarification as to whether that means it’s not relevant for older
> branches or whether a proper backport will be needed.
>
> I did a quick experiment with kirkstone and there were two issues with
> cherry-picking:
>
> 1. The test suite hunk failed since the test file doesn't exist in gcc 11
> 2. The first hunk fails to apply since the file name being modified in
> gcc 11 is a .c, apparently it was changed to .cc in later releases.
> You took this into account in the CVE patch file
>
> Removing the testsuite hunk and hacking the filename allowed the first
> hunk to apply (but with fuzz errors).
>
> So I'd say we definitely need a proper backport if the issue is
> relevant for older branches.
>
> I'll pull CVE-2023-4039.patch from kirkstone and dunfell until we have
> clarification.
>
> Thanks!
>
> Steve
>

[-- Attachment #2: Type: text/html, Size: 1979 bytes --]

Re: [OE-core] [mickledore][PATCH] gcc: backport a fix for ICE caused by CVE-2023-4039.patch

[Steve Sakoman]

On Mon, Sep 18, 2023 at 7:10 AM Martin Jansa <martin.jansa@gmail.com> wrote:
>
> I've intentionally sent it only for mickledore as the gcc version in kirkstone and dunfell isn't affected by this (at least my reproducer isn't).

So, just to clarify, is it OK to take the CVE-2023-4039.patch for
kirkstone and dunfell?

Steve

>
> On Mon, Sep 18, 2023 at 6:42 PM Steve Sakoman <steve@sakoman.com> wrote:
>>
>> On Mon, Sep 18, 2023 at 6:08 AM Ross Burton <Ross.Burton@arm.com> wrote:
>> >
>> > On 18 Sep 2023, at 16:48, Steve Sakoman <steve@sakoman.com> wrote:
>> > >
>> > > The CVE-2023-4039.patch was also submitted for kirkstone and dunfell.
>> > > Should I attempt to cherry-pick this fix for those branches too?
>> >
>> > The bug says that the patch only applies cleaning to GCC 12/13, I’m seeking clarification as to whether that means it’s not relevant for older branches or whether a proper backport will be needed.
>>
>> I did a quick experiment with kirkstone and there were two issues with
>> cherry-picking:
>>
>> 1. The test suite hunk failed since the test file doesn't exist in gcc 11
>> 2. The first hunk fails to apply since the file name being modified in
>> gcc 11 is a .c, apparently it was changed to .cc in later releases.
>> You took this into account in the CVE patch file
>>
>> Removing the testsuite hunk and hacking the filename allowed the first
>> hunk to apply (but with fuzz errors).
>>
>> So I'd say we definitely need a proper backport if the issue is
>> relevant for older branches.
>>
>> I'll pull CVE-2023-4039.patch from kirkstone and dunfell until we have
>> clarification.
>>
>> Thanks!
>>
>> Steve

Re: [OE-core] [mickledore][PATCH] gcc: backport a fix for ICE caused by CVE-2023-4039.patch

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 438 bytes --]

On Mon, Sep 18, 2023 at 7:21 PM Steve Sakoman <steve@sakoman.com> wrote:

> On Mon, Sep 18, 2023 at 7:10 AM Martin Jansa <martin.jansa@gmail.com>
> wrote:
> >
> > I've intentionally sent it only for mickledore as the gcc version in
> kirkstone and dunfell isn't affected by this (at least my reproducer isn't).
>
> So, just to clarify, is it OK to take the CVE-2023-4039.patch for
> kirkstone and dunfell?
>

I think so.

[-- Attachment #2: Type: text/html, Size: 802 bytes --]

Re: [OE-core] [mickledore][PATCH] gcc: backport a fix for ICE caused by CVE-2023-4039.patch

[Ross Burton]

On 18 Sep 2023, at 18:50, Martin Jansa <martin.jansa@gmail.com> wrote:
> 
> On Mon, Sep 18, 2023 at 7:21 PM Steve Sakoman <steve@sakoman.com> wrote:
> On Mon, Sep 18, 2023 at 7:10 AM Martin Jansa <martin.jansa@gmail.com> wrote:
> >
> > I've intentionally sent it only for mickledore as the gcc version in kirkstone and dunfell isn't affected by this (at least my reproducer isn't).
> 
> So, just to clarify, is it OK to take the CVE-2023-4039.patch for
> kirkstone and dunfell?
> 
> I think so. 

Agreed.  The relevant code changed a fair bit and there’s no evidence right now that it was broken in older releases.

Ross