> This patch corrects bugs within the CO-RE builtin field expression
> related builtins.
> The following bugs were identified and corrected based on the expected
> results of bpf-next selftests testsuite.
> It addresses the following problems:
> - Expressions with pointer dereferencing now point to the BTF structure
> type, instead of the structure pointer type.
> - Pointer addition to structure root is now identified and constructed
> in CO-RE relocations as if it is an array access. For example,
> "&(s+2)->b" generates "2:1" as an access string where "2" is
> refering to the access for "s+2".
>
> gcc/ChangeLog:
> * config/bpf/core-builtins.cc (core_field_info): Add
> support for POINTER_PLUS_EXPR in the root of the field expression.
> (bpf_core_get_index): Likewise.
> (pack_field_expr): Make the BTF type to point to the structure
> related node, instead of its pointer type.
> (make_core_safe_access_index): Correct to new code.
>
> gcc/testsuite/ChangeLog:
> * gcc.target/bpf/core-attr-5.c: Correct.
> * gcc.target/bpf/core-attr-6.c: Likewise.
> * gcc.target/bpf/core-attr-struct-as-array.c: Add test case for
> pointer arithmetics as array access use case.
> ---
> gcc/config/bpf/core-builtins.cc | 54 +++++++++++++++----
> gcc/testsuite/gcc.target/bpf/core-attr-5.c | 4 +-
> gcc/testsuite/gcc.target/bpf/core-attr-6.c | 4 +-
> .../bpf/core-attr-struct-as-array.c | 35 ++++++++++++
> 4 files changed, 82 insertions(+), 15 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/bpf/core-attr-struct-as-array.c
>
> diff --git a/gcc/config/bpf/core-builtins.cc b/gcc/config/bpf/core-builtins.cc
> index 8d8c54c1fb3d..4256fea15e49 100644
> --- a/gcc/config/bpf/core-builtins.cc
> +++ b/gcc/config/bpf/core-builtins.cc
> @@ -388,8 +388,8 @@ core_field_info (tree src, enum btf_core_reloc_kind kind)
>
> src = root_for_core_field_info (src);
>
> - get_inner_reference (src, &bitsize, &bitpos, &var_off, &mode, &unsignedp,
> - &reversep, &volatilep);
> + tree root = get_inner_reference (src, &bitsize, &bitpos, &var_off, &mode,
> + &unsignedp, &reversep, &volatilep);
>
> /* Note: Use DECL_BIT_FIELD_TYPE rather than DECL_BIT_FIELD here, because
> it
> remembers whether the field in question was originally declared as a
> @@ -414,6 +414,23 @@ core_field_info (tree src, enum btf_core_reloc_kind kind)
> {
> case BPF_RELO_FIELD_BYTE_OFFSET:
> {
> + result = 0;
> + if (var_off == NULL_TREE
> + && TREE_CODE (root) == INDIRECT_REF
> + && TREE_CODE (TREE_OPERAND (root, 0)) == POINTER_PLUS_EXPR)
> + {
> + tree node = TREE_OPERAND (root, 0);
> + tree offset = TREE_OPERAND (node, 1);
> + tree type = TREE_TYPE (TREE_OPERAND (node, 0));
> + type = TREE_TYPE (type);
> +
> + gcc_assert (TREE_CODE (offset) == INTEGER_CST && tree_fits_shwi_p
> (offset)
> + && COMPLETE_TYPE_P (type) && tree_fits_shwi_p (TYPE_SIZE
> (type)));
What if an expression with a non-constant offset (something like s+foo)
is passed to the builtin? Wouldn't it be better to error there instead
of ICEing?
> +
> + HOST_WIDE_INT offset_i = tree_to_shwi (offset);
> + result += offset_i;
> + }
> +
> type = unsigned_type_node;
> if (var_off != NULL_TREE)
> {
> @@ -422,9 +439,9 @@ core_field_info (tree src, enum btf_core_reloc_kind kind)
> }
>
> if (bitfieldp)
> - result = start_bitpos / 8;
> + result += start_bitpos / 8;
> else
> - result = bitpos / 8;
> + result += bitpos / 8;
> }
> break;
>
> @@ -552,6 +569,7 @@ bpf_core_get_index (const tree node, bool *valid)
> {
> tree offset = TREE_OPERAND (node, 1);
> tree type = TREE_TYPE (TREE_OPERAND (node, 0));
> + type = TREE_TYPE (type);
>
> if (TREE_CODE (offset) == INTEGER_CST && tree_fits_shwi_p (offset)
> && COMPLETE_TYPE_P (type) && tree_fits_shwi_p (TYPE_SIZE (type)))
> @@ -627,14 +645,18 @@ compute_field_expr (tree node, unsigned int *accessors,
>
> switch (TREE_CODE (node))
> {
> - case ADDR_EXPR:
> - return 0;
> case INDIRECT_REF:
> - accessors[0] = 0;
> - return 1;
> - case POINTER_PLUS_EXPR:
> - accessors[0] = bpf_core_get_index (node, valid);
> - return 1;
> + if (TREE_CODE (node = TREE_OPERAND (node, 0)) == POINTER_PLUS_EXPR)
> + {
> + accessors[0] = bpf_core_get_index (node, valid);
> + *access_node = TREE_OPERAND (node, 0);
> + return 1;
> + }
> + else
> + {
> + accessors[0] = 0;
> + return 1;
> + }
> case COMPONENT_REF:
> n = compute_field_expr (TREE_OPERAND (node, 0), accessors,
> valid,
> @@ -660,6 +682,7 @@ compute_field_expr (tree node, unsigned int *accessors,
> access_node, false);
> return n;
>
> + case ADDR_EXPR:
> case CALL_EXPR:
> case SSA_NAME:
> case VAR_DECL:
> @@ -688,6 +711,9 @@ pack_field_expr (tree *args,
> tree access_node = NULL_TREE;
> tree type = NULL_TREE;
>
> + if (TREE_CODE (root) == ADDR_EXPR)
> + root = TREE_OPERAND (root, 0);
> +
> ret.reloc_decision = REPLACE_CREATE_RELOCATION;
>
> unsigned int accessors[100];
> @@ -695,6 +721,8 @@ pack_field_expr (tree *args,
> compute_field_expr (root, accessors, &valid, &access_node, false);
>
> type = TREE_TYPE (access_node);
> + if (POINTER_TYPE_P (type))
> + type = TREE_TYPE (type);
>
> if (valid == true)
> {
> @@ -1351,6 +1379,8 @@ make_core_safe_access_index (tree expr, bool *changed,
> bool entry = true)
> if (base == NULL_TREE || base == expr)
> return expr;
>
> + base = expr;
> +
> tree ret = NULL_TREE;
> int n;
> bool valid = true;
> @@ -1365,6 +1395,8 @@ make_core_safe_access_index (tree expr, bool *changed,
> bool entry = true)
> {
> if (TREE_CODE (access_node) == INDIRECT_REF)
> base = TREE_OPERAND (access_node, 0);
> + else
> + base = access_node;
>
> bool local_changed = false;
> ret = make_core_safe_access_index (base, &local_changed, false);
> diff --git a/gcc/testsuite/gcc.target/bpf/core-attr-5.c
> b/gcc/testsuite/gcc.target/bpf/core-attr-5.c
> index e71901d0d4d1..90734dab3a29 100644
> --- a/gcc/testsuite/gcc.target/bpf/core-attr-5.c
> +++ b/gcc/testsuite/gcc.target/bpf/core-attr-5.c
> @@ -63,5 +63,5 @@ func (struct T *t, int i)
> /* { dg-final { scan-assembler-times "bpfcr_astr_off \\(\"0:1:2\"\\)" 1 } }
> */
> /* { dg-final { scan-assembler-times "bpfcr_astr_off \\(\"0:1:1:1\"\\)" 1 }
> } */
> /* { dg-final { scan-assembler-times "bpfcr_astr_off \\(\"0:0\"\\)" 1 } } */
> -/* { dg-final { scan-assembler-times "bpfcr_type \\(struct T \\*\\)" 4 } } */
> -/* { dg-final { scan-assembler-times "bpfcr_type \\(struct U \\*\\)" 4 {
> xfail *-*-* } } } */
> +/* { dg-final { scan-assembler-times "bpfcr_type \\(struct T\\)" 4 } } */
> +/* { dg-final { scan-assembler-times "bpfcr_type \\(struct U\\)" 4 { xfail
> *-*-* } } } */
> diff --git a/gcc/testsuite/gcc.target/bpf/core-attr-6.c
> b/gcc/testsuite/gcc.target/bpf/core-attr-6.c
> index 34a4c367e528..d0c5371b86e0 100644
> --- a/gcc/testsuite/gcc.target/bpf/core-attr-6.c
> +++ b/gcc/testsuite/gcc.target/bpf/core-attr-6.c
> @@ -45,6 +45,6 @@ func (struct T *t, int i)
> /* { dg-final { scan-assembler-times "bpfcr_astr_off \\(\"0:3\"\\)" 2 } } */
> /* { dg-final { scan-assembler-times "bpfcr_astr_off \\(\"0:1:2\"\\)" 1 } }
> */
> /* { dg-final { scan-assembler-times "bpfcr_astr_off \\(\"0:0\"\\)" 1 } } */
> -/* { dg-final { scan-assembler-times "bpfcr_type \\(struct T \\*\\)" 3 } } */
> -/* { dg-final { scan-assembler-times "bpfcr_type \\(struct U \\*\\)" 2 } } */
> +/* { dg-final { scan-assembler-times "bpfcr_type \\(struct T\\)" 3 } } */
> +/* { dg-final { scan-assembler-times "bpfcr_type \\(struct U\\)" 2 } } */
>
> diff --git a/gcc/testsuite/gcc.target/bpf/core-attr-struct-as-array.c
> b/gcc/testsuite/gcc.target/bpf/core-attr-struct-as-array.c
> new file mode 100644
> index 000000000000..3f6eb9cb97f8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/bpf/core-attr-struct-as-array.c
> @@ -0,0 +1,35 @@
> +/* Basic test for struct __attribute__((preserve_access_index))
> + for BPF CO-RE support. */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-O0 -dA -gbtf -mco-re" } */
> +
> +struct S {
> + int a;
> + int b;
> + int c;
> +} __attribute__((preserve_access_index));
> +
> +void
> +func (struct S * s)
> +{
> + /* This test is marked as XFAIL since for the time being the CO-RE
> + implementation is not able to disambiguate between a point manipulation
> + and a CO-RE access when using preserve_access_index attribute. The
> + current implemetantion is incorrect if we consider that STRUCT S might
> + have different size within the kernel.
> + This example demonstrates how the implementation of
> preserve_access_index
> + as an attribute of the type is flagile. */
> +
> + /* 2:2 */
> + int *x = &((s+2)->c);
> + *x = 4;
> +
> + /* 2:1 */
> + int *y = __builtin_preserve_access_index (&((s+2)->b));
> + *y = 2;
> +}
> +
> +/* { dg-final { scan-assembler-times "ascii \"2:2.0\"\[\t
> \]+\[^\n\]*btf_aux_string" 1 { xfail *-*-* } } } */
> +/* { dg-final { scan-assembler-times "ascii \"2:1.0\"\[\t
> \]+\[^\n\]*btf_aux_string" 1 } } */
> +/* { dg-final { scan-assembler-times "bpfcr_type" 2 } } */
