Re: [PATCH] bpf: Split expressions for proper CO-RE code generation

[Cupertino Miranda]

On 28-10-2025 21:26, David Faust wrote:

On 10/24/25 10:01, Cupertino Miranda wrote:

This patch corrects CO-RE generation for the cases where an expression
starts with a non-CO-RE access, but in the middle it requires to
generate CO-RE to correctly compute the access location.

It fixes it by splitting the expression into their CO-RE and non-CO-RE
counterparts. It performs this by walking gimple expressions, and for
each field access to which its type is a struct or union, it verifies if
both the types for the base and field are attributed similarly.
Otherwise, it splits the expression at this location by creating a
temporary variable and performing any required pointer conversions.
This smaller expressions are converted to CO-RE in the subsequent
gimple walker.

There is no way in GCC to distinguish nested struct/union definitions
from non-nested ones.
This patch simplifies code and enforces that all preserve_access_index
structs/unions would be attributed explicity.
Previous approach was error prone as it would extend CO-RE accesses to
structures which would not be attributed.

One little nit/suggestion inline below.

OK either way.
Thanks!

gcc/ChangeLog:
        PR target/120241
        * config/bpf/core-builtins.cc
        (is_attr_preserve_access): Correct for pointer types.
        (maybe_get_base_for_field_expr, core_access_index_map,
        core_access_clean, core_is_access_index, core_mark_as_access_index):
        Remove.
        (make_gimple_core_safe_access_index): Remove call to
        core_is_access_index.
        (core_should_split_expr, gimple_core_early_split_expr): Add
        function to split expressions in CO-RE and non-CO-RE
        expressions.
        (execute_lower_bpf_core): Adapt to new code.

gcc/testsuite/ChangeLog:
        * gcc.target/bpf/core-attr-3.c: Add attribute.
        * gcc.target/bpf/core-attr-4.c: Add attribute.
        * gcc.target/bpf/core-attr-5.c: Add attribute.
        * gcc.target/bpf/core-attr-6.c: Add attribute.
        * gcc.target/bpf/core-attr-7.c: New test.
        * gcc.target/bpf/core-attr-calls.c: Adapt.
---
  gcc/config/bpf/core-builtins.cc               | 143 +++++++++---------
  gcc/testsuite/gcc.target/bpf/core-attr-3.c    |   4 +-
  gcc/testsuite/gcc.target/bpf/core-attr-4.c    |   4 +-
  gcc/testsuite/gcc.target/bpf/core-attr-5.c    |   2 +-
  gcc/testsuite/gcc.target/bpf/core-attr-6.c    |   4 +-
  gcc/testsuite/gcc.target/bpf/core-attr-7.c    | 105 +++++++++++++
  .../gcc.target/bpf/core-attr-calls.c          |   4 +-
  7 files changed, 184 insertions(+), 82 deletions(-)
  create mode 100644 gcc/testsuite/gcc.target/bpf/core-attr-7.c

diff --git a/gcc/config/bpf/core-builtins.cc b/gcc/config/bpf/core-builtins.cc
index 152da94a976..64479be537c 100644
--- a/gcc/config/bpf/core-builtins.cc
+++ b/gcc/config/bpf/core-builtins.cc
@@ -324,8 +324,13 @@ is_attr_preserve_access (tree t)
                             TYPE_ATTRIBUTES (TREE_TYPE (base)));

if (TREE_CODE (base) == MEM_REF)

-    return lookup_attribute ("preserve_access_index",
-                            TYPE_ATTRIBUTES (TREE_TYPE (base)));
+    {
+      tree type = TREE_TYPE (base);
+      if (POINTER_TYPE_P (type))
+       type = TREE_TYPE (type);
+      return lookup_attribute ("preserve_access_index",
+                              TYPE_ATTRIBUTES (type));
+    }

if (TREE_CODE (t) == COMPONENT_REF)

      {
@@ -1710,68 +1715,6 @@ bpf_output_core_reloc (rtx *operands, int nr_ops)
      }
  }

-static tree

-maybe_get_base_for_field_expr (tree expr)
-{
-  poly_int64 bitsize, bitpos;
-  tree var_off;
-  machine_mode mode;
-  int sign, reverse, vol;
-
-  if (expr == NULL_TREE)
-    return NULL_TREE;
-
-  return get_inner_reference (expr, &bitsize, &bitpos, &var_off, &mode,
-                             &sign, &reverse, &vol);
-}
-
-/* Access functions to mark sub expressions as attributed with
-   __preserve_access_index.
-   This is required since in gimple format, in order to convert an expression 
as
-   CO-RE safe, we must create multiple gimple statements.
-   Also, only the type of the base of the expression might be attributed with
-   __preserve_access_index.  Nevertheless all the consecutive accesses to this
-   attributed node should also be converted to CO-RE safe.
-   Any LHS assigned values with CO-RE converted expressions are marked and
-   any uses of these values are later checked for further convertion.
-   The core_access_index_map functions allow to mark this nodes for later
-   convertion to CO-RE.
-   This mechanism are used by make_gimple_core_safe_access_index.  */
-
-static GTY(()) hash_map<tree, tree> *core_access_index_map = NULL;
-
-static void
-core_access_clean (void)
-{
-  if (core_access_index_map == NULL)
-    core_access_index_map = hash_map<tree, tree>::create_ggc (10);
-  core_access_index_map->empty ();
-}
-
-static bool
-core_is_access_index (tree expr)
-{
-  if (TREE_CODE (expr) == MEM_REF
-      || TREE_CODE (expr) == INDIRECT_REF)
-    expr = TREE_OPERAND (expr, 0);
-
-  tree *def = core_access_index_map->get (expr);
-  if (def)
-    return true;
-  return false;
-}
-
-static void
-core_mark_as_access_index (tree expr)
-{
-  if (TREE_CODE (expr) == MEM_REF
-      || TREE_CODE (expr) == INDIRECT_REF)
-    expr = TREE_OPERAND (expr, 0);
-
-  if (core_access_index_map->get (expr) == NULL)
-    core_access_index_map->put (expr, NULL_TREE);
-}
-
  /* This function is an adaptation of make_core_safe_access_index but to be 
used
     in gimple format trees.  It is used by execute_lower_bpf_core, when
     traversing the gimple tree looking for nodes that would have its type
@@ -1792,8 +1735,7 @@ make_gimple_core_safe_access_index (tree *tp,
      patch = &(TREE_OPERAND (*tp, 0));
    tree orig_type = TREE_TYPE (*patch);

- if ((is_attr_preserve_access (*patch)

-      || core_is_access_index (maybe_get_base_for_field_expr (*patch)))
+  if (is_attr_preserve_access (*patch)
        && (n = compute_field_expr (*patch, NULL, &valid, NULL)) > 0
        && valid == true)
      {
@@ -1822,13 +1764,66 @@ make_gimple_core_safe_access_index (tree *tp,

wi->changed = true;

        *walk_subtrees = false;
+    }
+  return NULL_TREE;
+}
+
+static bool
+core_should_split_expr (tree t)
+{
+  tree type = TREE_TYPE (t);
+  if (TREE_CODE (type) != RECORD_TYPE
+      && TREE_CODE (type) != UNION_TYPE)

if (!RECORD_OR_UNION_TYPE_P (type))  ?

Thanks for the suggestion. It is much better!>> +    return false;

+
+  if (TREE_CODE (t) == COMPONENT_REF)
+    {
+      tree base = TREE_OPERAND (t, 0);
+
+      bool base_is_attr = lookup_attribute ("preserve_access_index",
+                            TYPE_ATTRIBUTES (TREE_TYPE (base)));
+      bool cur_is_attr = lookup_attribute ("preserve_access_index",
+                            TYPE_ATTRIBUTES (TREE_TYPE (t)));

- tree lhs;

-      if (!wi->is_lhs
-         && gimple_code (wi->stmt) != GIMPLE_CALL
-         && (lhs = gimple_get_lhs (wi->stmt)) != NULL_TREE)
-       core_mark_as_access_index (lhs);
+      if (base_is_attr != cur_is_attr)
+       return true;
      }
+  return false;
+}
+
+static tree
+gimple_core_early_split_expr (tree *tp,
+                             int *walk_subtrees ATTRIBUTE_UNUSED,
+                             void *data)
+{
+  struct walk_stmt_info *wi = (struct walk_stmt_info *) data;
+
+  if (core_should_split_expr (*tp))
+    {
+      gimple_seq before = NULL;
+      push_gimplify_context ();
+
+      tree base = TREE_OPERAND (*tp, 0);
+      walk_tree (&base, gimple_core_early_split_expr, data, NULL);
+      TREE_OPERAND (*tp, 0) = base;
+
+      if (POINTER_TYPE_P (TREE_TYPE (*tp)))
+       {
+         gimplify_expr (tp, &before, NULL, is_gimple_addressable, fb_lvalue);
+       }
+      else
+       {
+         tree tmp = fold_build1 (ADDR_EXPR,
+                                 build_pointer_type (TREE_TYPE (*tp)), *tp);
+         gimplify_expr (&tmp, &before, NULL, is_gimple_val, fb_rvalue);
+         *tp = fold_build2 (MEM_REF, TREE_TYPE (tmp), tmp,
+                            build_int_cst (ptr_type_node, 0));
+       }
+
+      gsi_insert_seq_before (&(wi->gsi), before, GSI_SAME_STMT);
+      pop_gimplify_context (NULL);
+    }
+
    return NULL_TREE;
  }

@@ -1851,11 +1846,13 @@ execute_lower_bpf_core (void)

    gimple_seq body = gimple_body (current_function_decl);

struct walk_stmt_info wi;

-  core_access_clean ();
-
    memset (&wi, 0, sizeof (wi));
    wi.info = NULL;

+ /* Early split to guarantee base of expression is a preserve_access_index

+     structure.  */
+  walk_gimple_seq_mod (&body, NULL, gimple_core_early_split_expr, &wi);
+
    /* Split preserve_access_index expressions when needed.  */
    walk_gimple_seq_mod (&body, NULL, make_gimple_core_safe_access_index, &wi);
    return 0;
diff --git a/gcc/testsuite/gcc.target/bpf/core-attr-3.c 
b/gcc/testsuite/gcc.target/bpf/core-attr-3.c
index 12354fc6f86..58c27fd43bb 100644
--- a/gcc/testsuite/gcc.target/bpf/core-attr-3.c
+++ b/gcc/testsuite/gcc.target/bpf/core-attr-3.c
@@ -11,14 +11,14 @@
  struct O {
    int e;
    int f;
-};
+} __attribute__((preserve_access_index));

struct S {

    int a;
    struct {
      int b;
      int c;
-  } inner;
+  } __attribute__((preserve_access_index)) inner;
    struct O other;
  } __attribute__((preserve_access_index));

diff --git a/gcc/testsuite/gcc.target/bpf/core-attr-4.c b/gcc/testsuite/gcc.target/bpf/core-attr-4.c

index 6f025f42f3e..c001b5b76ef 100644
--- a/gcc/testsuite/gcc.target/bpf/core-attr-4.c
+++ b/gcc/testsuite/gcc.target/bpf/core-attr-4.c
@@ -13,8 +13,8 @@ struct T {
        int d;
        int e[4];
        int f;
-    } v;
-  } u;
+    } __attribute__((preserve_access_index)) v;
+  } __attribute__((preserve_access_index)) u;
  } __attribute__((preserve_access_index));

diff --git a/gcc/testsuite/gcc.target/bpf/core-attr-5.c b/gcc/testsuite/gcc.target/bpf/core-attr-5.c

index 81e25fa85de..4d443d88b0a 100644
--- a/gcc/testsuite/gcc.target/bpf/core-attr-5.c
+++ b/gcc/testsuite/gcc.target/bpf/core-attr-5.c
@@ -11,7 +11,7 @@ struct U {
      int e[4];
      int f;
      int *g;
-  } v;
+  } __attribute__((preserve_access_index)) v;
  } __attribute__((preserve_access_index));

struct T {

diff --git a/gcc/testsuite/gcc.target/bpf/core-attr-6.c 
b/gcc/testsuite/gcc.target/bpf/core-attr-6.c
index 25215b5ae37..d43825ea97c 100644
--- a/gcc/testsuite/gcc.target/bpf/core-attr-6.c
+++ b/gcc/testsuite/gcc.target/bpf/core-attr-6.c
@@ -11,8 +11,8 @@ struct U {
      int e[4];
      int f;
      int *g;
-  } v;
-} u;
+  } __attribute__((preserve_access_index)) v;
+} __attribute__((preserve_access_index)) u;

struct T {

    int a;
diff --git a/gcc/testsuite/gcc.target/bpf/core-attr-7.c 
b/gcc/testsuite/gcc.target/bpf/core-attr-7.c
new file mode 100644
index 00000000000..5032fa8dba9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/bpf/core-attr-7.c
@@ -0,0 +1,105 @@
+/* Test for BPF CO-RE __attribute__((preserve_access_index)) with accesses on
+   LHS and both LHS and RHS of assignment.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O2 -dA -gbtf -mco-re -masm=normal" } */
+
+struct other
+{
+  char c;
+  int i;
+};
+
+struct inner_attr1
+{
+  int i1;
+  int i2;
+} __attribute__((preserve_access_index));
+
+struct inner_noattr
+{
+  int i1;
+  int i2;
+};
+
+union A_noattr
+{
+  struct inner_attr1 inner_attr;
+  struct inner_noattr inner_no_attr;
+  struct inner_noattr *inner_p;
+};
+union A_attr
+{
+  struct inner_attr1 inner_attr;
+  struct inner_noattr inner_no_attr;
+  struct inner_noattr *inner_p;
+} __attribute__((preserve_access_index));
+
+
+struct outer_noattr
+{
+  struct other *other;
+  struct inner_attr
+  {
+    int i1;
+    int i2;
+  } __attribute__((preserve_access_index)) inner;
+  struct inner_noattr inner_no_attr;
+  struct inner_attr1 *inner_p;
+  union A_attr a_attr;
+  union A_noattr a_noattr;
+};
+
+struct outer_attr
+{
+  struct other *other;
+  struct inner_attr
+  {
+    int i1;
+    int i2;
+  } __attribute__((preserve_access_index)) inner;
+
+  struct inner_noattr inner_no_attr;
+  struct inner_attr1 *inner_p;
+  union A_attr a_attr;
+  union A_noattr a_noattr;
+} __attribute__((preserve_access_index));
+
+
+void
+func (struct outer_noattr *o, struct outer_attr *o_attr)
+{
+  o->inner.i2 = 7;
+  o->inner_p->i2 = 8;
+  o->inner_no_attr.i2 = 9;
+
+  o_attr->inner_no_attr.i2 = 10;
+
+  o->a_noattr.inner_no_attr.i1 = 1;
+  o->a_attr.inner_no_attr.i1 = 2;
+  o->a_noattr.inner_attr.i1 = 3;
+  o_attr->a_attr.inner_attr.i1 = 4;
+  o_attr->a_noattr.inner_attr.i1 = 5;
+
+  /* This would still force everything as being attributed, although none of
+     the structs has the attribute.  */
+  __builtin_preserve_access_index (({ o->a_noattr.inner_p->i1 = 20; }));
+}
+
+/* { dg-final { scan-assembler-times "ascii \"0:1.0\"\[\t 
\]+\[^\n\]*btf_aux_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"0:0.0\"\[\t 
\]+\[^\n\]*btf_aux_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"0:2.0\"\[\t 
\]+\[^\n\]*btf_aux_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"0:5.0\"\[\t 
\]+\[^\n\]*btf_aux_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"0:4:0:0.0\"\[\t 
\]+\[^\n\]*btf_aux_string" 1 } } */
+/* { dg-final { scan-assembler-times "ascii \"0:5:2.0\"\[\t 
\]+\[^\n\]*btf_aux_string" 1 } } */
+
+/* { dg-final { scan-assembler-times "bpfcr_astr_off \\(\"0:0\"\\)" 3 } } */
+/* { dg-final { scan-assembler-times "bpfcr_astr_off \\(\"0:1\"\\)" 3 } } */
+/* { dg-final { scan-assembler-times "bpfcr_astr_off \\(\"0:2\"\\)" 1 } } */
+/* { dg-final { scan-assembler-times "bpfcr_astr_off \\(\"0:5\"\\)" 1 } } */
+/* { dg-final { scan-assembler-times "bpfcr_astr_off \\(\"0:4:0:0\"\\)" 1 } } 
*/
+/* { dg-final { scan-assembler-times "bpfcr_astr_off \\(\"0:5:2\"\\)" 1 } } */
+/* { dg-final { scan-assembler-times "bpfcr_type \\(struct inner_attr\\)" 1 } 
} */
+/* { dg-final { scan-assembler-times "bpfcr_type \\(struct inner_attr1\\)" 3 } 
} */
+/* { dg-final { scan-assembler-times "bpfcr_type \\(struct outer_attr\\)" 3 } 
} */
+/* { dg-final { scan-assembler-times "bpfcr_type \\(union A_attr\\)" 1 } } */
diff --git a/gcc/testsuite/gcc.target/bpf/core-attr-calls.c 
b/gcc/testsuite/gcc.target/bpf/core-attr-calls.c
index 87290c5c211..27b08af1bb7 100644
--- a/gcc/testsuite/gcc.target/bpf/core-attr-calls.c
+++ b/gcc/testsuite/gcc.target/bpf/core-attr-calls.c
@@ -37,13 +37,13 @@ func (struct T *t, int i)
    /* 0:3  */
    get_other_u(t->ptr_u)->c = 43;

- /* 0:2:1 */

+  /* 0:2  */
    get_other_v(&t->u.v)->d = 44;
  }

/* { dg-final { scan-assembler-times "bpfcr_astr_off \\(\"0:3\"\\)" 2 } } */

  /* { dg-final { scan-assembler-times "bpfcr_astr_off \\(\"0:0\"\\)" 1 } } */
-/* { dg-final { scan-assembler-times "bpfcr_astr_off \\(\"0:2:1\"\\)" 1 } } */
+/* { dg-final { scan-assembler-times "bpfcr_astr_off \\(\"0:2\"\\)" 1 } } */
  /* { dg-final { scan-assembler-times "bpfcr_type \\(struct T\\)" 3 } } */
  /* { dg-final { scan-assembler-times "bpfcr_type \\(struct U\\)" 1 } } */