Hi David,

On 30-09-2024 18:24, David Faust wrote:


On 9/27/24 09:49, Cupertino Miranda wrote:
Based on observation within bpf-next selftests and comparisson of GCC
and clang compiled code, the BPF loader expects all CO-RE relocations to
point to BTF non const type nodes.
---
  gcc/btfout.cc                                 |  2 +-
  gcc/config/bpf/btfext-out.cc                  |  6 ++++
  gcc/ctfc.h                                    |  2 ++
  .../gcc.target/bpf/core-attr-const.c          | 32 +++++++++++++++++++
  4 files changed, 41 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/gcc.target/bpf/core-attr-const.c

diff --git a/gcc/btfout.cc b/gcc/btfout.cc
index 8b91bde8798..24f62ec1a52 100644
--- a/gcc/btfout.cc
+++ b/gcc/btfout.cc
@@ -167,7 +167,7 @@ get_btf_kind (uint32_t ctf_kind)
/* Convenience wrapper around get_btf_kind for the common case. */ -static uint32_t
+uint32_t
  btf_dtd_kind (ctf_dtdef_ref dtd)
  {
    if (!dtd)
diff --git a/gcc/config/bpf/btfext-out.cc b/gcc/config/bpf/btfext-out.cc
index 095c35b894b..655da23066d 100644
--- a/gcc/config/bpf/btfext-out.cc
+++ b/gcc/config/bpf/btfext-out.cc
@@ -320,6 +320,12 @@ bpf_core_reloc_add (const tree type, const char * 
section_name,
    ctf_container_ref ctfc = ctf_get_tu_ctfc ();
    ctf_dtdef_ref dtd = ctf_lookup_tree_type (ctfc, type);
+ /* Make sure CO-RE type is never the const version. */
+  if (btf_dtd_kind (dtd) == BTF_KIND_CONST

Hm, what about volatile and restrict? I would guess they are treated in
the same way as const by the kernel BPF loader, so probably we will have
to handle them here also. Please check.
Thanks for pointing this out. Indeed volatile had the same problem in GCC.
However, restrict seems to be rather a decl qualifier and did not seem to have any impact on the type used in either clang of gcc, i.e. in both the result code was already similar adding restrict did not change the CO-RE relocation definition.

Will send v2 shortly.

+      && kind >= BPF_RELO_FIELD_BYTE_OFFSET
+      && kind <= BPF_RELO_FIELD_RSHIFT_U64)
+    dtd = dtd->ref_type;
+
    /* Buffer the access string in the auxiliary strtab.  */
    bpfcr->bpfcr_astr_off = 0;
    gcc_assert (accessor != NULL);
diff --git a/gcc/ctfc.h b/gcc/ctfc.h
index 41e1169f271..e5967f590f9 100644
--- a/gcc/ctfc.h
+++ b/gcc/ctfc.h
@@ -465,4 +465,6 @@ extern void btf_mark_type_used (tree);
  extern int ctfc_get_dtd_srcloc (ctf_dtdef_ref, ctf_srcloc_ref);
  extern int ctfc_get_dvd_srcloc (ctf_dvdef_ref, ctf_srcloc_ref);
+extern uint32_t btf_dtd_kind (ctf_dtdef_ref dtd);
+
  #endif /* GCC_CTFC_H */
diff --git a/gcc/testsuite/gcc.target/bpf/core-attr-const.c 
b/gcc/testsuite/gcc.target/bpf/core-attr-const.c
new file mode 100644
index 00000000000..34a4a9cc5e8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/bpf/core-attr-const.c
@@ -0,0 +1,32 @@
+/* Test to make sure CO-RE access relocs point to non const versions of the
+   type.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O0 -dA -gbtf -mco-re -masm=normal" } */
+
+struct S {
+  int a;
+  int b;
+  int c;
+} __attribute__((preserve_access_index));
+
+void
+func (struct S * s)
+{
+  int *x;
+  int *y;
+  const struct S *cs = s;
+
+  /* 0:2 */
+  x = &(s->c);
+
+  /* 0:2 */
+  y = (int *) &(cs->c);
+
+  *x = 4;
+  *y = 4;
+}
+
+/* Both const and non const struct type should have the same bpfcr_type. */
+/* { dg-final { scan-assembler-times "0x1\t# bpfcr_type \\(struct S\\)" 1 } } 
*/
+/* { dg-final { scan-assembler-times "0x1\t# bpfcr_type \\(const struct S\\)" 
1 } } */

Reply via email to