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).