From 0f801e0b6cc9f67c9a8983127e23161f6025c5b6 Mon Sep 17 00:00:00 2001 From: Tamar Christina Date: Tue, 27 Oct 2020 16:30:31 +0000 Subject: [PATCH] AArch64: Fix overflow in memcopy expansion on aarch64. Currently the inline memcpy expansion code for AArch64 is using a signed int to hold the number of elements to copy. When you giver give it a value larger than INT_MAX it will overflow. The overflow causes the maximum number of instructions we want to expand to check to fail since this assumes an unsigned number. This patch changes the maximum isns arithmetic to be unsigned HOST_WIDE_INT. note that the calculation *must* remained signed as the memcopy issues overlapping unaligned copies. This means the pointer must be moved back and so you need signed arithmetic. gcc/ChangeLog: PR target/97535 * config/aarch64/aarch64.c (aarch64_expand_cpymem): Use unsigned arithmetic in check. gcc/testsuite/ChangeLog: PR target/97535 * gcc.target/aarch64/pr97535.c: New test. --- gcc/config/aarch64/aarch64.c | 13 +++++++++---- gcc/testsuite/gcc.target/aarch64/pr97535.c | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gcc.target/aarch64/pr97535.c diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index a8cc545c370..35d6f2e2f01 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -21299,6 +21299,8 @@ aarch64_copy_one_block_and_progress_pointers (rtx *src, rtx *dst, bool aarch64_expand_cpymem (rtx *operands) { + /* These need to be signed as we need to perform arithmetic on n as + signed operations. */ int n, mode_bits; rtx dst = operands[0]; rtx src = operands[1]; @@ -21309,21 +21311,24 @@ aarch64_expand_cpymem (rtx *operands) /* When optimizing for size, give a better estimate of the length of a memcpy call, but use the default otherwise. Moves larger than 8 bytes will always require an even number of instructions to do now. And each - operation requires both a load+store, so devide the max number by 2. */ - int max_num_moves = (speed_p ? 16 : AARCH64_CALL_RATIO) / 2; + operation requires both a load+store, so divide the max number by 2. */ + unsigned int max_num_moves = (speed_p ? 16 : AARCH64_CALL_RATIO) / 2; /* We can't do anything smart if the amount to copy is not constant. */ if (!CONST_INT_P (operands[2])) return false; - n = INTVAL (operands[2]); + unsigned HOST_WIDE_INT tmp = INTVAL (operands[2]); /* Try to keep the number of instructions low. For all cases we will do at most two moves for the residual amount, since we'll always overlap the remainder. */ - if (((n / 16) + (n % 16 ? 2 : 0)) > max_num_moves) + if (((tmp / 16) + (tmp % 16 ? 2 : 0)) > max_num_moves) return false; + /* At this point tmp is known to have to fit inside an int. */ + n = tmp; + base = copy_to_mode_reg (Pmode, XEXP (dst, 0)); dst = adjust_automodify_address (dst, VOIDmode, base, 0); diff --git a/gcc/testsuite/gcc.target/aarch64/pr97535.c b/gcc/testsuite/gcc.target/aarch64/pr97535.c new file mode 100644 index 00000000000..6f83b3f5714 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/pr97535.c @@ -0,0 +1,16 @@ +/* { dg-do compile } */ + +#include + +#define SIZE 2181038080 + +extern char raw_buffer[SIZE]; + +void setRaw(const void *raw) +{ + memcpy(raw_buffer, raw, SIZE); +} + +/* At any optimization level this should be a function call + and not inlined. */ +/* { dg-final { scan-assembler "bl\tmemcpy" } } */ -- 2.30.2