Hi Andrea,
> -----Original Message-----
> From: Andrea Corallo <andrea.cora...@arm.com>
> Sent: 14 April 2020 11:01
> To: gcc-patches@gcc.gnu.org
> Cc: Kyrylo Tkachov <kyrylo.tkac...@arm.com>; nd <n...@arm.com>;
> Christophe Lyon <christophe.l...@linaro.org>
> Subject: [PATCH]aarch64: falkor-tag-collision-avoidance.c fix valid_src_p for
> use of uninitialized value
>
> Hi all,
>
> Testing the backport of pr94530 fix (af19e4d0e23e) to GCC 9 I realized
> this is not entirely correct.
>
> aarch64_classify_address sets the base field of struct
> aarch64_address_info for all aarch64_address_type with the exception
> of ADDRESS_SYMBOLIC. In this case we would access an uninitialized
> value. The attached patch fixes the issue.
>
> Bootstraped on aarch64 and regressioned. Okay for trunk?
>
> I've also tested af19e4d0e23e + the current patch rebased on on top of
> the gcc-9 branch. Okay to backport?
>
> Andrea
>
> PS I'm wondering if would be good also to always set the addr field in
> aarch64_classify_address for safeness.
>
> gcc/ChangeLog
>
> 2020-??-?? Andrea Corallo <andrea.cora...@arm.com>
>
> * config/aarch64/falkor-tag-collision-avoidance.c
> (valid_src_p): Check for aarch64_address_info type before
> accessing base field.
>From 4af5bb32cd88bb4e65591207839fb0e276b2eb23 Mon Sep 17 00:00:00 2001
From: Andrea Corallo <andrea.cora...@arm.com>
Date: Thu, 9 Apr 2020 15:34:50 +0100
Subject: [PATCH] pr94530 2
---
gcc/config/aarch64/falkor-tag-collision-avoidance.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/config/aarch64/falkor-tag-collision-avoidance.c
b/gcc/config/aarch64/falkor-tag-collision-avoidance.c
index f850153fae02..fb56ff66bfab 100644
--- a/gcc/config/aarch64/falkor-tag-collision-avoidance.c
+++ b/gcc/config/aarch64/falkor-tag-collision-avoidance.c
@@ -538,7 +538,7 @@ valid_src_p (rtx src, rtx_insn *insn, struct loop *loop,
bool *pre_post,
if (!aarch64_classify_address (&addr, XEXP (x, 0), mode, true))
return false;
- if (!REG_P (addr.base))
+ if (addr.type == ADDRESS_SYMBOLIC || !REG_P (addr.base))
return false;
Hmm, given that the code below only handles the ADDRESS_REG_* forms, how about
we get defensive here and check that addr.type is not one of ADDRESS_REG_*
instead?
That way, if aarch64_classify_address is extended in the future with new
addr.type values that leave addr.base in the wrong forms, this code will not
need to be updated.
Thanks,
Kyrill
unsigned regno = REGNO (addr.base);
--
2.17.1
