From 886964a78aa89ef3c15e69a7b4b96c55d51ea812 Mon Sep 17 00:00:00 2001 From: Tamar Christina Date: Tue, 17 Nov 2020 10:19:20 +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. The type can stay 32-bits since the number of instructions we are allowed to expand to are at most 8 which is far below what you could fit in an unsigned 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 df973165165e..17ccfa2b1011 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -21110,6 +21110,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]; @@ -21120,21 +21122,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 000000000000..6f83b3f57141 --- /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.47.2