[v2] bpf: Split expressions for proper CO-RE code generation
Commit Message
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.
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 | 181 +++++++++++-------
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 | 120 ++++++++++++
.../gcc.target/bpf/core-attr-calls.c | 4 +-
7 files changed, 236 insertions(+), 83 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/bpf/core-attr-7.c
Comments
Hi Cupertino,
On 1/9/26 11:20, 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.
Just to check since it isn't mentioned: this has been tested right? :)
Have you also compiled and run the kernel bpf selftests with this
patch and checked for regressions?
A couple of comments inline below, but generally looks good.
>
> 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 | 181 +++++++++++-------
> 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 | 120 ++++++++++++
> .../gcc.target/bpf/core-attr-calls.c | 4 +-
> 7 files changed, 236 insertions(+), 83 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 152da94a9761..36c477db450b 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,90 @@ make_gimple_core_safe_access_index (tree *tp,
>
> wi->changed = true;
> *walk_subtrees = false;
> + }
> + return NULL_TREE;
> +}
>
> - 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);
> +/* Structure to pass data for the walkers in execute_lower_bpf_core. */
> +struct lower_bpf_core_data {
> + bool is_root_expression;
> +};
> +
> +static bool
> +core_should_split_expr (tree t, bool is_root)
> +{
> + tree type = TREE_TYPE (t);
> + if (TREE_CODE (type) != RECORD_TYPE
> + && TREE_CODE (type) != UNION_TYPE)
> + 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)));
> +
> + if (is_root == true && cur_is_attr == false)
> + return false;
> +
> + if (base_is_attr != cur_is_attr)
> + return true;
> + }
> + return false;
> +}
It would be good to add some function level comments documenting
core_should_split_expr and gimple_core_early_split_expr to explain
what they are doing and the reasoning for it. Similar to the
deleted block comment above.
> +
> +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;
> + struct lower_bpf_core_data *p_data = (struct lower_bpf_core_data *) wi->info;
> +
> + if (core_should_split_expr (*tp, p_data->is_root_expression))
> + {
> + gimple_seq before = NULL;
> + push_gimplify_context ();
> +
> + bool tmp = p_data->is_root_expression;
> + p_data->is_root_expression = false;
> + tree base = TREE_OPERAND (*tp, 0);
> + walk_tree (&base, gimple_core_early_split_expr, data, NULL);
> + TREE_OPERAND (*tp, 0) = base;
> + p_data->is_root_expression = tmp;
> +
> + 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);
> +
> + p_data->is_root_expression = false;
> + return NULL_TREE;
> }
> + if (TREE_CODE (*tp) == COMPONENT_REF)
> + {
> + bool tmp = p_data->is_root_expression;
> + p_data->is_root_expression = false;
> + tree base = TREE_OPERAND (*tp, 0);
> + walk_tree (&base, gimple_core_early_split_expr, data, NULL);
> + TREE_OPERAND (*tp, 0) = base;
> + p_data->is_root_expression = tmp;
> + }
> +
> return NULL_TREE;
> }
>
> @@ -1841,6 +1860,16 @@ make_gimple_core_safe_access_index (tree *tp,
> case it cannot be represented CO-RE safeguarded by a single CO-RE
> relocation. */
>
> +static tree
> +reset_access_expression_root (gimple_stmt_iterator *gsi ATTRIBUTE_UNUSED,
> + bool *handle_ops, struct walk_stmt_info *wi)
> +{
> + struct lower_bpf_core_data *p_data = (struct lower_bpf_core_data *) wi->info;
> + p_data->is_root_expression = true;
> + *handle_ops = false;
> + return NULL_TREE;
> +}
> +
> static unsigned int
> execute_lower_bpf_core (void)
> {
> @@ -1850,11 +1879,15 @@ execute_lower_bpf_core (void)
>
> gimple_seq body = gimple_body (current_function_decl);
>
> + struct lower_bpf_core_data data = { true };
> struct walk_stmt_info wi;
> - core_access_clean ();
> -
> memset (&wi, 0, sizeof (wi));
> - wi.info = NULL;
> + wi.info = &data;
> +
> + /* 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);
Let's delete this line rather than leaving it commented out.
> + walk_gimple_seq_mod (&body, reset_access_expression_root, 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);
> diff --git a/gcc/testsuite/gcc.target/bpf/core-attr-3.c b/gcc/testsuite/gcc.target/bpf/core-attr-3.c
> index 12354fc6f86d..58c27fd43bb4 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 6f025f42f3ee..c001b5b76ef9 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 81e25fa85de7..4d443d88b0ae 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 25215b5ae376..d43825ea97c3 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 000000000000..8547f4017e92
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/bpf/core-attr-7.c
> @@ -0,0 +1,120 @@
> +/* 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));
> +
> +extern void use_value(int);
> +
> +void
> +func (struct outer_noattr *o, struct outer_attr *o_attr)
> +{
> + /* 0:1 */
> + o->inner.i2 = 7;
> + /* 0:1 */
> + o->inner_p->i2 = 8;
> + /* nothing */
> + o->inner_no_attr.i2 = 9;
> +
> + /* 0:2 */
> + o_attr->inner_no_attr.i2 = 10;
> +
> + /* nothing */
> + o->a_noattr.inner_no_attr.i1 = 1;
> + /* 0:1 */
> + o->a_attr.inner_no_attr.i1 = 2;
> + /* 0:0 */
> + o->a_noattr.inner_attr.i1 = 3;
> + /* 0:4:0:0 */
> + o_attr->a_attr.inner_attr.i1 = 4;
> + /* 0:5 0:0 */
> + o_attr->a_noattr.inner_attr.i1 = 5;
> +
> + /* This would still force everything as being attributed, although none of
> + the structs has the attribute. */
> + /* 0:5:2 0:0 */
> + __builtin_preserve_access_index (({ o->a_noattr.inner_p->i1 = 20; }));
> +
> + /* 0:2 */
> + struct inner_noattr d = o_attr->inner_no_attr;
> + use_value(d.i2);
> +}
> +
> +/* { 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\"\\)" 2 } } */
> +/* { 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\\)" 4 } } */
> +/* { 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 87290c5c2116..27b08af1bb79 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 } } */
>
@@ -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,90 @@ make_gimple_core_safe_access_index (tree *tp,
wi->changed = true;
*walk_subtrees = false;
+ }
+ return NULL_TREE;
+}
- 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);
+/* Structure to pass data for the walkers in execute_lower_bpf_core. */
+struct lower_bpf_core_data {
+ bool is_root_expression;
+};
+
+static bool
+core_should_split_expr (tree t, bool is_root)
+{
+ tree type = TREE_TYPE (t);
+ if (TREE_CODE (type) != RECORD_TYPE
+ && TREE_CODE (type) != UNION_TYPE)
+ 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)));
+
+ if (is_root == true && cur_is_attr == false)
+ return false;
+
+ 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;
+ struct lower_bpf_core_data *p_data = (struct lower_bpf_core_data *) wi->info;
+
+ if (core_should_split_expr (*tp, p_data->is_root_expression))
+ {
+ gimple_seq before = NULL;
+ push_gimplify_context ();
+
+ bool tmp = p_data->is_root_expression;
+ p_data->is_root_expression = false;
+ tree base = TREE_OPERAND (*tp, 0);
+ walk_tree (&base, gimple_core_early_split_expr, data, NULL);
+ TREE_OPERAND (*tp, 0) = base;
+ p_data->is_root_expression = tmp;
+
+ 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);
+
+ p_data->is_root_expression = false;
+ return NULL_TREE;
}
+ if (TREE_CODE (*tp) == COMPONENT_REF)
+ {
+ bool tmp = p_data->is_root_expression;
+ p_data->is_root_expression = false;
+ tree base = TREE_OPERAND (*tp, 0);
+ walk_tree (&base, gimple_core_early_split_expr, data, NULL);
+ TREE_OPERAND (*tp, 0) = base;
+ p_data->is_root_expression = tmp;
+ }
+
return NULL_TREE;
}
@@ -1841,6 +1860,16 @@ make_gimple_core_safe_access_index (tree *tp,
case it cannot be represented CO-RE safeguarded by a single CO-RE
relocation. */
+static tree
+reset_access_expression_root (gimple_stmt_iterator *gsi ATTRIBUTE_UNUSED,
+ bool *handle_ops, struct walk_stmt_info *wi)
+{
+ struct lower_bpf_core_data *p_data = (struct lower_bpf_core_data *) wi->info;
+ p_data->is_root_expression = true;
+ *handle_ops = false;
+ return NULL_TREE;
+}
+
static unsigned int
execute_lower_bpf_core (void)
{
@@ -1850,11 +1879,15 @@ execute_lower_bpf_core (void)
gimple_seq body = gimple_body (current_function_decl);
+ struct lower_bpf_core_data data = { true };
struct walk_stmt_info wi;
- core_access_clean ();
-
memset (&wi, 0, sizeof (wi));
- wi.info = NULL;
+ wi.info = &data;
+
+ /* 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);
+ walk_gimple_seq_mod (&body, reset_access_expression_root, 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);
@@ -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));
@@ -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));
@@ -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 {
@@ -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;
new file mode 100644
@@ -0,0 +1,120 @@
+/* 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));
+
+extern void use_value(int);
+
+void
+func (struct outer_noattr *o, struct outer_attr *o_attr)
+{
+ /* 0:1 */
+ o->inner.i2 = 7;
+ /* 0:1 */
+ o->inner_p->i2 = 8;
+ /* nothing */
+ o->inner_no_attr.i2 = 9;
+
+ /* 0:2 */
+ o_attr->inner_no_attr.i2 = 10;
+
+ /* nothing */
+ o->a_noattr.inner_no_attr.i1 = 1;
+ /* 0:1 */
+ o->a_attr.inner_no_attr.i1 = 2;
+ /* 0:0 */
+ o->a_noattr.inner_attr.i1 = 3;
+ /* 0:4:0:0 */
+ o_attr->a_attr.inner_attr.i1 = 4;
+ /* 0:5 0:0 */
+ o_attr->a_noattr.inner_attr.i1 = 5;
+
+ /* This would still force everything as being attributed, although none of
+ the structs has the attribute. */
+ /* 0:5:2 0:0 */
+ __builtin_preserve_access_index (({ o->a_noattr.inner_p->i1 = 20; }));
+
+ /* 0:2 */
+ struct inner_noattr d = o_attr->inner_no_attr;
+ use_value(d.i2);
+}
+
+/* { 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\"\\)" 2 } } */
+/* { 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\\)" 4 } } */
+/* { dg-final { scan-assembler-times "bpfcr_type \\(union A_attr\\)" 1 } } */
@@ -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 } } */