rtl-ssa: Fix move range canonicalisation [PR115929]
Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 |
success
|
Test passed
|
linaro-tcwg-bot/tcwg_gcc_check--master-arm |
success
|
Test passed
|
Commit Message
In this PR, canonicalize_move_range walked off the end of a list
and triggered a null dereference. There are multiple ways of fixing
that, but I think the approach taken in the patch should be
relatively efficient.
Tested on aarch64-linux-gnu & x86_64-linux-gnu. OK to install?
Richard
gcc/
PR rtl-optimization/115929
* rtl-ssa/movement.h (canonicalize_move_range): Check for null prev
and next insns and create an invalid move range for them.
gcc/testsuite/
PR rtl-optimization/115929
* gcc.dg/torture/pr115929-2.c: New test.
---
gcc/rtl-ssa/movement.h | 20 ++++++++++++++++++--
gcc/testsuite/gcc.dg/torture/pr115929-2.c | 22 ++++++++++++++++++++++
2 files changed, 40 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/torture/pr115929-2.c
Comments
On Tue, Jul 16, 2024 at 4:30 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> In this PR, canonicalize_move_range walked off the end of a list
> and triggered a null dereference. There are multiple ways of fixing
> that, but I think the approach taken in the patch should be
> relatively efficient.
>
> Tested on aarch64-linux-gnu & x86_64-linux-gnu. OK to install?
OK.
I think you own RTL-SSA, but I'm missing a maintainers entry for
rtl-ssa in MAINTAINERS. Jeff, can you poke the appropriate people
to get the ball rolling on that?
Thanks,
Richard.
> Richard
>
>
> gcc/
> PR rtl-optimization/115929
> * rtl-ssa/movement.h (canonicalize_move_range): Check for null prev
> and next insns and create an invalid move range for them.
>
> gcc/testsuite/
> PR rtl-optimization/115929
> * gcc.dg/torture/pr115929-2.c: New test.
> ---
> gcc/rtl-ssa/movement.h | 20 ++++++++++++++++++--
> gcc/testsuite/gcc.dg/torture/pr115929-2.c | 22 ++++++++++++++++++++++
> 2 files changed, 40 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/torture/pr115929-2.c
>
> diff --git a/gcc/rtl-ssa/movement.h b/gcc/rtl-ssa/movement.h
> index 17d31e0b5cb..ea1f788df49 100644
> --- a/gcc/rtl-ssa/movement.h
> +++ b/gcc/rtl-ssa/movement.h
> @@ -76,9 +76,25 @@ inline bool
> canonicalize_move_range (insn_range_info &move_range, insn_info *insn)
> {
> while (move_range.first != insn && !can_insert_after (move_range.first))
> - move_range.first = move_range.first->next_nondebug_insn ();
> + if (auto *next = move_range.first->next_nondebug_insn ())
> + move_range.first = next;
> + else
> + {
> + // Invalidate the range. prev_nondebug_insn is always nonnull
> + // if next_nondebug_insn is null.
> + move_range.last = move_range.first->prev_nondebug_insn ();
> + return false;
> + }
> while (move_range.last != insn && !can_insert_after (move_range.last))
> - move_range.last = move_range.last->prev_nondebug_insn ();
> + if (auto *prev = move_range.last->prev_nondebug_insn ())
> + move_range.last = prev;
> + else
> + {
> + // Invalidate the range. next_nondebug_insn is always nonnull
> + // if prev_nondebug_insn is null.
> + move_range.first = move_range.last->next_nondebug_insn ();
> + return false;
> + }
> return bool (move_range);
> }
>
> diff --git a/gcc/testsuite/gcc.dg/torture/pr115929-2.c b/gcc/testsuite/gcc.dg/torture/pr115929-2.c
> new file mode 100644
> index 00000000000..c8473a74da6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr115929-2.c
> @@ -0,0 +1,22 @@
> +/* { dg-additional-options "-fschedule-insns" } */
> +
> +int a, b, c, d, e, f;
> +int main() {
> + if (e && f)
> + while (1)
> + while (a)
> + a = 0;
> + if (c) {
> + if (b)
> + goto g;
> + int h = a;
> + i:
> + b = ~((b ^ h) | 1 % b);
> + if (a)
> + g:
> + b = 0;
> + }
> + if (d)
> + goto i;
> + return 0;
> +}
> --
> 2.25.1
>
@@ -76,9 +76,25 @@ inline bool
canonicalize_move_range (insn_range_info &move_range, insn_info *insn)
{
while (move_range.first != insn && !can_insert_after (move_range.first))
- move_range.first = move_range.first->next_nondebug_insn ();
+ if (auto *next = move_range.first->next_nondebug_insn ())
+ move_range.first = next;
+ else
+ {
+ // Invalidate the range. prev_nondebug_insn is always nonnull
+ // if next_nondebug_insn is null.
+ move_range.last = move_range.first->prev_nondebug_insn ();
+ return false;
+ }
while (move_range.last != insn && !can_insert_after (move_range.last))
- move_range.last = move_range.last->prev_nondebug_insn ();
+ if (auto *prev = move_range.last->prev_nondebug_insn ())
+ move_range.last = prev;
+ else
+ {
+ // Invalidate the range. next_nondebug_insn is always nonnull
+ // if prev_nondebug_insn is null.
+ move_range.first = move_range.last->next_nondebug_insn ();
+ return false;
+ }
return bool (move_range);
}
new file mode 100644
@@ -0,0 +1,22 @@
+/* { dg-additional-options "-fschedule-insns" } */
+
+int a, b, c, d, e, f;
+int main() {
+ if (e && f)
+ while (1)
+ while (a)
+ a = 0;
+ if (c) {
+ if (b)
+ goto g;
+ int h = a;
+ i:
+ b = ~((b ^ h) | 1 % b);
+ if (a)
+ g:
+ b = 0;
+ }
+ if (d)
+ goto i;
+ return 0;
+}