public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc(refs/vendors/ARM/heads/morello)] morello: Fix handling of capability integer_csts with nonzero metadata
@ 2023-06-01 12:46 Alex Coplan
0 siblings, 0 replies; only message in thread
From: Alex Coplan @ 2023-06-01 12:46 UTC (permalink / raw)
To: gcc-cvs
https://gcc.gnu.org/g:aef12b3e317cb7864402509eeb0a5b817f10b54e
commit aef12b3e317cb7864402509eeb0a5b817f10b54e
Author: Alex Coplan <alex.coplan@arm.com>
Date: Thu Apr 6 15:38:57 2023 +0100
morello: Fix handling of capability integer_csts with nonzero metadata
This patch fixes https://bugs.linaro.org/show_bug.cgi?id=5954 .
For the testcase cap-repr-1.c added with the patch, at -O3 FRE / VN is clever
enough to piece together the 16 individual stores to p1 and replace p1 with an
integer_cst of capablity pointer_type with nonzero metadata bits:
<integer_cst 0x7ffff6eee280 type <pointer_type 0x7ffff7018000>
constant 0x100050000000000000000>
This wasn't immediately apparent from the FRE dump, however, since
dump_generic_node currently only prints TREE_INT_CST_LOW (the low
HOST_WIDE_INT bits) for an INTEGER_CST of POINTER_TYPE. With Morello's
128-bit pointers, pointer INTEGER_CSTs may in fact have
TREE_INT_CST_NUNITS > 1. This patch adjusts the dumping to handle this
case and dump such large pointer constants in hex format.
After fixing the tree dumps, it became apparent that the expand code
wasn't handling integer_csts with nonzero metadata properly.
Currently we just ignore the upper bits and emit a null-derived
capability in all cases.
We adjust the expand code to check if any metadata bits are set
in capability integer_csts and, if so, handle them by forcing
the constant to memory in TImode. We then adjust the constant
pool load back into CADImode.
Doing this revealed a latent bug in fold_build_replace_address_value
where for an intcap c:
fold_build_replace_address_value (c,
build_int_cst (noncapability_type (c), -1))
would give:
<integer_cst 0x7ffff7109948 type <intcap_type 0x7ffff70182a0 __intcap>
constant -1>
i.e. 128 1s, but this should in fact have 64 0s (representing the
metadata) followed by 64 1s (for the address value of -1).
So we also fix that bug with this patch.
Diff:
---
gcc/expr.c | 14 +++-
gcc/fold-const.c | 3 +-
.../gcc.target/aarch64/morello/cap-repr-1.c | 20 ++++++
.../gcc.target/aarch64/morello/cap-repr-2.c | 21 ++++++
gcc/tree-pretty-print.c | 1 +
gcc/tree.c | 76 ++++++++++++++++++++++
gcc/tree.h | 4 ++
7 files changed, 137 insertions(+), 2 deletions(-)
diff --git a/gcc/expr.c b/gcc/expr.c
index d778547c8de..6dd3a79b8be 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -10752,8 +10752,20 @@ expand_expr_real_1 (tree exp, rtx target, machine_mode tmode,
GET_MODE_PRECISION (TYPE_MODE (type)), we need to extend from
the former to the latter according to the signedness of the
type. */
+ scalar_int_mode int_mode;
+ if (capability_type_p (type) && cap_cst_metadatap (exp))
+ {
+ /* Capability constants with nonzero metadata bits need special
+ treatment and must be loaded from the constant pool. */
+ int_mode = int_mode_for_mode (TYPE_MODE (type)).require ();
+ const auto prec = GET_MODE_PRECISION (int_mode);
+ temp = immed_wide_int_const (wi::to_wide (exp, prec), int_mode);
+ temp = force_const_mem (int_mode, temp);
+ temp = adjust_address (temp, TYPE_MODE (type), 0);
+ return temp;
+ }
tree noncap_type = noncapability_type (type);
- scalar_int_mode int_mode = SCALAR_INT_TYPE_MODE (noncap_type);
+ int_mode = SCALAR_INT_TYPE_MODE (noncap_type);
temp = immed_wide_int_const
(wi::to_wide (exp, GET_MODE_PRECISION (int_mode)), int_mode);
if (capability_type_p (type))
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index e551cc7c216..be4010b5376 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -13670,7 +13670,8 @@ fold_replace_address_value_1 (location_t loc, tree c, tree cv)
of the number, with the rest being unset. */
wide_int mask = wi::mask (noncap_prec, 0, cap_prec);
wide_int masked_cap = wi::bit_and_not (cap_orig, mask);
- wide_int res = wi::bit_or (masked_cap, new_value);
+ wide_int masked_cv = wi::bit_and (new_value, mask);
+ wide_int res = wi::bit_or (masked_cap, masked_cv);
/* Replacing address value. Only want something marked as overflow if
the original value was overflowed. No way to cause an overflow here.
*/
diff --git a/gcc/testsuite/gcc.target/aarch64/morello/cap-repr-1.c b/gcc/testsuite/gcc.target/aarch64/morello/cap-repr-1.c
new file mode 100644
index 00000000000..f44d67aebcd
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/morello/cap-repr-1.c
@@ -0,0 +1,20 @@
+/* { dg-do run } */
+/* { dg-require-effective-target cheri_capability_any } */
+
+int main()
+{
+ unsigned char nullrepr0[sizeof(void * __capability)]
+ = {0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0};
+ unsigned char nullrepr1[sizeof(void * __capability)]
+ = {0,0,0,0,0,0,0,0,5,0,1,0,0,0,0,0};
+ void * __capability p0, * __capability p1;
+
+ for (unsigned i=0;i<sizeof(void * __capability);i++)
+ {
+ ((unsigned char*)&p0)[i]=nullrepr0[i];
+ ((unsigned char*)&p1)[i]=nullrepr1[i];
+ }
+
+ if (__builtin_cheri_equal_exact (p0,p1))
+ __builtin_abort ();
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/morello/cap-repr-2.c b/gcc/testsuite/gcc.target/aarch64/morello/cap-repr-2.c
new file mode 100644
index 00000000000..f9895f3792b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/morello/cap-repr-2.c
@@ -0,0 +1,21 @@
+/* { dg-do run } */
+/* { dg-require-effective-target cheri_capability_any } */
+
+static const unsigned char repr[sizeof (void * __capability)]
+ = {0,0,0,0,0,0,0,0,5,0,1,0,0,0,0,0};
+
+__attribute__((noipa))
+void check (void * __capability p)
+{
+ if (__builtin_memcmp (&p, repr, sizeof (p)) != 0)
+ __builtin_abort ();
+}
+
+int main()
+{
+ void * __capability p;
+ for (unsigned i = 0; i < sizeof (void * __capability); i++)
+ ((unsigned char*)&p)[i] = repr[i];
+
+ check (p);
+}
diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index 6f367d76136..17ef078b2c3 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -1995,6 +1995,7 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
pp_string (pp, ") ");
}
if (TREE_CODE (TREE_TYPE (node)) == POINTER_TYPE
+ && TREE_INT_CST_NUNITS (node) == 1
&& ! (flags & TDF_GIMPLE))
{
/* In the case of a pointer, one may want to divide by the
diff --git a/gcc/tree.c b/gcc/tree.c
index 90393f110f7..f9934d355df 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -2686,6 +2686,21 @@ integer_all_onesp (const_tree expr)
== wi::to_wide (expr));
}
+/* Test if CAP_CST has any nonzero metadata bits. */
+
+bool
+cap_cst_metadatap (const_tree cap_cst)
+{
+ gcc_assert (TREE_CODE (cap_cst) == INTEGER_CST
+ && capability_type_p (TREE_TYPE (cap_cst)));
+
+ tree cap_type = TREE_TYPE (cap_cst);
+ unsigned HOST_WIDE_INT cap_prec = TYPE_CAP_PRECISION (cap_type);
+ unsigned HOST_WIDE_INT noncap_prec = TYPE_NONCAP_PRECISION (cap_type);
+ wide_int w = wi::to_wide (cap_cst, cap_prec);
+ return wi::min_precision (w, UNSIGNED) > noncap_prec;
+}
+
/* Return 1 if EXPR is either INTEGER_ALL_ONESP, or a constant capability
containing all 1's in the bits of its value and all 0 in the bits of its
metadata.
@@ -16588,6 +16603,66 @@ test_maybe_cap_all_onesp (void)
}
}
+static void
+test_cap_cst_metadatap (void)
+{
+ tree uintcap = uintcap_type_node;
+ if (!uintcap)
+ uintcap = build_intcap_type_for_mode (CADImode, 1);
+ ASSERT_TRUE (uintcap != NULL_TREE);
+
+ tree intcap = intcap_type_node;
+ if (!intcap)
+ intcap = build_intcap_type_for_mode (CADImode, 0);
+ ASSERT_TRUE (intcap != NULL_TREE);
+
+ tree types[] = {
+ force_build_capability_pointer_type (void_type_node),
+ uintcap,
+ intcap
+ };
+ tree t;
+ for (auto type : types)
+ {
+ tree noncap_type = noncapability_type (type);
+
+ t = build_zero_cst (type);
+ ASSERT_FALSE (cap_cst_metadatap (t));
+
+ tree cv = build_int_cst (noncap_type, 42);
+ t = fold_build_replace_address_value (t, cv);
+ ASSERT_FALSE (cap_cst_metadatap (t));
+
+ cv = build_int_cst (noncap_type, -1);
+ t = fold_build_replace_address_value (t, cv);
+ ASSERT_FALSE (cap_cst_metadatap (t));
+
+ const unsigned HOST_WIDE_INT cap_prec = TYPE_CAP_PRECISION (type);
+ const unsigned HOST_WIDE_INT noncap_prec = TYPE_NONCAP_PRECISION (type);
+
+ if (cap_prec > noncap_prec)
+ {
+ /* Pick a bit in the middle of the metadata to set. */
+ const auto bit_to_set = (cap_prec + noncap_prec) / 2;
+
+ auto bit = wi::uhwi (1, cap_prec);
+ auto amt = wi::uhwi (bit_to_set - 1, cap_prec);
+ auto res = wi::bit_or (wi::zero (cap_prec), wi::lshift (bit, amt));
+ t = wide_int_to_tree (type, res);
+ ASSERT_TRUE (cap_cst_metadatap (t));
+
+ /* Changing the address value shouldn't matter. */
+ cv = build_int_cst (noncap_type, 42);
+ t = fold_build_replace_address_value (t, cv);
+ ASSERT_TRUE (cap_cst_metadatap (t));
+
+ cv = build_int_cst (noncap_type, -1);
+ t = fold_build_replace_address_value (t, cv);
+ ASSERT_TRUE (cap_cst_metadatap (t));
+ }
+ }
+}
+
/* Run all of the selftests within this file. */
void
@@ -16596,6 +16671,7 @@ tree_c_tests ()
test_fold_build_replace_address_value ();
test_capability_type_manipulation ();
test_maybe_cap_all_onesp ();
+ test_cap_cst_metadatap ();
test_integer_constants ();
test_identifiers ();
test_labels ();
diff --git a/gcc/tree.h b/gcc/tree.h
index 3ca1956cf2a..08c03e134ae 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -4892,6 +4892,10 @@ extern bool integer_all_onesp (const_tree);
extern bool maybe_cap_all_onesp (const_tree);
+/* Test if CAP_CST has any nonzero metadata bits. */
+
+extern bool cap_cst_metadatap (const_tree);
+
/* integer_minus_onep (tree x) is nonzero if X is an integer constant of
value -1. */
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2023-06-01 12:46 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-01 12:46 [gcc(refs/vendors/ARM/heads/morello)] morello: Fix handling of capability integer_csts with nonzero metadata Alex Coplan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).