public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 1/3] bpf: Fix CO-RE field expression builtins
@ 2024-03-13 14:24 Cupertino Miranda
  2024-03-13 14:24 ` [PATCH 2/3] bpf: Fix access string default for CO-RE type based relocations Cupertino Miranda
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Cupertino Miranda @ 2024-03-13 14:24 UTC (permalink / raw)
  To: gcc-patches; +Cc: jose.marchesi, david.faust, elena.zannoni, Cupertino Miranda

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)));
+
+	    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 } } */
-- 
2.30.2


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-03-20 20:33 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-13 14:24 [PATCH 1/3] bpf: Fix CO-RE field expression builtins Cupertino Miranda
2024-03-13 14:24 ` [PATCH 2/3] bpf: Fix access string default for CO-RE type based relocations Cupertino Miranda
2024-03-19 16:57   ` David Faust
2024-03-20 20:31     ` Cupertino Miranda
2024-03-13 14:24 ` [PATCH 3/3] bpf: Corrected index computation when present with unnamed struct fields Cupertino Miranda
2024-03-19 17:07   ` David Faust
2024-03-20 20:32     ` Cupertino Miranda
2024-03-14  8:54 ` [PATCH 1/3] bpf: Fix CO-RE field expression builtins Jose E. Marchesi
2024-03-14 11:07   ` Cupertino Miranda
2024-03-14 13:23     ` Jose E. Marchesi
2024-03-14 13:44       ` Jose E. Marchesi
2024-03-20 20:31         ` Cupertino Miranda

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