| Message ID | 20240313142441.180242-2-cupertino.miranda@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] bpf: Fix CO-RE field expression builtins | expand |
On 3/13/24 07:24, Cupertino Miranda wrote: > Although part of all CO-RE relocation data, type based relocations do > not require an access string. > Initial implementation defined it as an empty string. > On the other hand, libbpf when parsing the CO-RE relocations verifies > that those strings would contain "0", otherwise reports an error. > This patch makes GCC compliant with libbpf expectations. OK, thanks. > > gcc/Changelog: > * config/bpf/btfext-out.cc (cpf_core_reloc_add): Correct for new code. > Add assert to validate the string is set. > * config/bpf/core-builtins.cc (cr_final): Make string struct > field as const. > (process_enum_value): Correct for field type change. > (process_type): Set access string to "0". > > gcc/testsuite/ChangeLog: > * gcc.target/bpf/core-builtin-type-based.c: Correct. > * gcc.target/bpf/core-builtin-type-id.c: Correct. > --- > gcc/config/bpf/btfext-out.cc | 5 +++-- > gcc/config/bpf/core-builtins.cc | 10 ++++++---- > gcc/testsuite/gcc.target/bpf/core-builtin-type-based.c | 1 + > gcc/testsuite/gcc.target/bpf/core-builtin-type-id.c | 1 + > 4 files changed, 11 insertions(+), 6 deletions(-) > > diff --git a/gcc/config/bpf/btfext-out.cc b/gcc/config/bpf/btfext-out.cc > index 57c0dc323812..ff1fd0739f1e 100644 > --- a/gcc/config/bpf/btfext-out.cc > +++ b/gcc/config/bpf/btfext-out.cc > @@ -299,8 +299,9 @@ bpf_core_reloc_add (const tree type, const char * section_name, > > /* Buffer the access string in the auxiliary strtab. */ > bpfcr->bpfcr_astr_off = 0; > - if (accessor != NULL) > - bpfcr->bpfcr_astr_off = btf_ext_add_string (accessor); > + gcc_assert (accessor != NULL); > + bpfcr->bpfcr_astr_off = btf_ext_add_string (accessor); > + > bpfcr->bpfcr_type = get_btf_id (ctf_lookup_tree_type (ctfc, type)); > bpfcr->bpfcr_insn_label = label; > bpfcr->bpfcr_kind = kind; > diff --git a/gcc/config/bpf/core-builtins.cc b/gcc/config/bpf/core-builtins.cc > index 4256fea15e49..70b14e48e6e5 100644 > --- a/gcc/config/bpf/core-builtins.cc > +++ b/gcc/config/bpf/core-builtins.cc > @@ -205,7 +205,7 @@ struct cr_local > /* Core Relocation Final data */ > struct cr_final > { > - char *str; > + const char *str; > tree type; > enum btf_core_reloc_kind kind; > }; > @@ -868,8 +868,10 @@ process_enum_value (struct cr_builtins *data) > { > if (TREE_VALUE (l) == expr) > { > - ret.str = (char *) ggc_alloc_atomic ((index / 10) + 1); > - sprintf (ret.str, "%d", index); > + char *tmp = (char *) ggc_alloc_atomic ((index / 10) + 1); > + sprintf (tmp, "%d", index); > + ret.str = (const char *) tmp; > + > break; > } > index++; > @@ -987,7 +989,7 @@ process_type (struct cr_builtins *data) > || data->kind == BPF_RELO_TYPE_MATCHES); > > struct cr_final ret; > - ret.str = NULL; > + ret.str = ggc_strdup ("0"); > ret.type = data->type; > ret.kind = data->kind; > > diff --git a/gcc/testsuite/gcc.target/bpf/core-builtin-type-based.c b/gcc/testsuite/gcc.target/bpf/core-builtin-type-based.c > index 74a8d5a14d9d..9d818133c084 100644 > --- a/gcc/testsuite/gcc.target/bpf/core-builtin-type-based.c > +++ b/gcc/testsuite/gcc.target/bpf/core-builtin-type-based.c > @@ -56,3 +56,4 @@ int foo(void *data) > /* { dg-final { scan-assembler-times "0x8\[\t \]+\[^\n\]*bpfcr_kind" 13 } } BPF_TYPE_EXISTS */ > /* { dg-final { scan-assembler-times "0x9\[\t \]+\[^\n\]*bpfcr_kind" 11 } } BPF_TYPE_SIZE */ > /* { dg-final { scan-assembler-times "0xc\[\t \]+\[^\n\]*bpfcr_kind" 13 } } BPF_TYPE_MATCHES */ > +/* { dg-final { scan-assembler-times "bpfcr_astr_off \[(\"\]+0\[(\"\]+" 37 } } */ > diff --git a/gcc/testsuite/gcc.target/bpf/core-builtin-type-id.c b/gcc/testsuite/gcc.target/bpf/core-builtin-type-id.c > index 4b23288eac08..9576b91bc940 100644 > --- a/gcc/testsuite/gcc.target/bpf/core-builtin-type-id.c > +++ b/gcc/testsuite/gcc.target/bpf/core-builtin-type-id.c > @@ -38,3 +38,4 @@ int foo(void *data) > /* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bpfcr_type" 0 { xfail *-*-* } } } */ > /* { dg-final { scan-assembler-times "0x6\[\t \]+\[^\n\]*bpfcr_kind" 13 } } BPF_TYPE_ID_LOCAL */ > /* { dg-final { scan-assembler-times "0x7\[\t \]+\[^\n\]*bpfcr_kind" 7 } } BPF_TYPE_ID_TARGET */ > +/* { dg-final { scan-assembler-times "bpfcr_astr_off \[(\"\]+0\[(\"\]+" 20 } } */
David Faust writes: > On 3/13/24 07:24, Cupertino Miranda wrote: >> Although part of all CO-RE relocation data, type based relocations do >> not require an access string. >> Initial implementation defined it as an empty string. >> On the other hand, libbpf when parsing the CO-RE relocations verifies >> that those strings would contain "0", otherwise reports an error. >> This patch makes GCC compliant with libbpf expectations. > > OK, thanks. > Pushed! Thanks >> >> gcc/Changelog: >> * config/bpf/btfext-out.cc (cpf_core_reloc_add): Correct for new code. >> Add assert to validate the string is set. >> * config/bpf/core-builtins.cc (cr_final): Make string struct >> field as const. >> (process_enum_value): Correct for field type change. >> (process_type): Set access string to "0". >> >> gcc/testsuite/ChangeLog: >> * gcc.target/bpf/core-builtin-type-based.c: Correct. >> * gcc.target/bpf/core-builtin-type-id.c: Correct. >> --- >> gcc/config/bpf/btfext-out.cc | 5 +++-- >> gcc/config/bpf/core-builtins.cc | 10 ++++++---- >> gcc/testsuite/gcc.target/bpf/core-builtin-type-based.c | 1 + >> gcc/testsuite/gcc.target/bpf/core-builtin-type-id.c | 1 + >> 4 files changed, 11 insertions(+), 6 deletions(-) >> >> diff --git a/gcc/config/bpf/btfext-out.cc b/gcc/config/bpf/btfext-out.cc >> index 57c0dc323812..ff1fd0739f1e 100644 >> --- a/gcc/config/bpf/btfext-out.cc >> +++ b/gcc/config/bpf/btfext-out.cc >> @@ -299,8 +299,9 @@ bpf_core_reloc_add (const tree type, const char * section_name, >> >> /* Buffer the access string in the auxiliary strtab. */ >> bpfcr->bpfcr_astr_off = 0; >> - if (accessor != NULL) >> - bpfcr->bpfcr_astr_off = btf_ext_add_string (accessor); >> + gcc_assert (accessor != NULL); >> + bpfcr->bpfcr_astr_off = btf_ext_add_string (accessor); >> + >> bpfcr->bpfcr_type = get_btf_id (ctf_lookup_tree_type (ctfc, type)); >> bpfcr->bpfcr_insn_label = label; >> bpfcr->bpfcr_kind = kind; >> diff --git a/gcc/config/bpf/core-builtins.cc b/gcc/config/bpf/core-builtins.cc >> index 4256fea15e49..70b14e48e6e5 100644 >> --- a/gcc/config/bpf/core-builtins.cc >> +++ b/gcc/config/bpf/core-builtins.cc >> @@ -205,7 +205,7 @@ struct cr_local >> /* Core Relocation Final data */ >> struct cr_final >> { >> - char *str; >> + const char *str; >> tree type; >> enum btf_core_reloc_kind kind; >> }; >> @@ -868,8 +868,10 @@ process_enum_value (struct cr_builtins *data) >> { >> if (TREE_VALUE (l) == expr) >> { >> - ret.str = (char *) ggc_alloc_atomic ((index / 10) + 1); >> - sprintf (ret.str, "%d", index); >> + char *tmp = (char *) ggc_alloc_atomic ((index / 10) + 1); >> + sprintf (tmp, "%d", index); >> + ret.str = (const char *) tmp; >> + >> break; >> } >> index++; >> @@ -987,7 +989,7 @@ process_type (struct cr_builtins *data) >> || data->kind == BPF_RELO_TYPE_MATCHES); >> >> struct cr_final ret; >> - ret.str = NULL; >> + ret.str = ggc_strdup ("0"); >> ret.type = data->type; >> ret.kind = data->kind; >> >> diff --git a/gcc/testsuite/gcc.target/bpf/core-builtin-type-based.c b/gcc/testsuite/gcc.target/bpf/core-builtin-type-based.c >> index 74a8d5a14d9d..9d818133c084 100644 >> --- a/gcc/testsuite/gcc.target/bpf/core-builtin-type-based.c >> +++ b/gcc/testsuite/gcc.target/bpf/core-builtin-type-based.c >> @@ -56,3 +56,4 @@ int foo(void *data) >> /* { dg-final { scan-assembler-times "0x8\[\t \]+\[^\n\]*bpfcr_kind" 13 } } BPF_TYPE_EXISTS */ >> /* { dg-final { scan-assembler-times "0x9\[\t \]+\[^\n\]*bpfcr_kind" 11 } } BPF_TYPE_SIZE */ >> /* { dg-final { scan-assembler-times "0xc\[\t \]+\[^\n\]*bpfcr_kind" 13 } } BPF_TYPE_MATCHES */ >> +/* { dg-final { scan-assembler-times "bpfcr_astr_off \[(\"\]+0\[(\"\]+" 37 } } */ >> diff --git a/gcc/testsuite/gcc.target/bpf/core-builtin-type-id.c b/gcc/testsuite/gcc.target/bpf/core-builtin-type-id.c >> index 4b23288eac08..9576b91bc940 100644 >> --- a/gcc/testsuite/gcc.target/bpf/core-builtin-type-id.c >> +++ b/gcc/testsuite/gcc.target/bpf/core-builtin-type-id.c >> @@ -38,3 +38,4 @@ int foo(void *data) >> /* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bpfcr_type" 0 { xfail *-*-* } } } */ >> /* { dg-final { scan-assembler-times "0x6\[\t \]+\[^\n\]*bpfcr_kind" 13 } } BPF_TYPE_ID_LOCAL */ >> /* { dg-final { scan-assembler-times "0x7\[\t \]+\[^\n\]*bpfcr_kind" 7 } } BPF_TYPE_ID_TARGET */ >> +/* { dg-final { scan-assembler-times "bpfcr_astr_off \[(\"\]+0\[(\"\]+" 20 } } */
diff --git a/gcc/config/bpf/btfext-out.cc b/gcc/config/bpf/btfext-out.cc index 57c0dc323812..ff1fd0739f1e 100644 --- a/gcc/config/bpf/btfext-out.cc +++ b/gcc/config/bpf/btfext-out.cc @@ -299,8 +299,9 @@ bpf_core_reloc_add (const tree type, const char * section_name, /* Buffer the access string in the auxiliary strtab. */ bpfcr->bpfcr_astr_off = 0; - if (accessor != NULL) - bpfcr->bpfcr_astr_off = btf_ext_add_string (accessor); + gcc_assert (accessor != NULL); + bpfcr->bpfcr_astr_off = btf_ext_add_string (accessor); + bpfcr->bpfcr_type = get_btf_id (ctf_lookup_tree_type (ctfc, type)); bpfcr->bpfcr_insn_label = label; bpfcr->bpfcr_kind = kind; diff --git a/gcc/config/bpf/core-builtins.cc b/gcc/config/bpf/core-builtins.cc index 4256fea15e49..70b14e48e6e5 100644 --- a/gcc/config/bpf/core-builtins.cc +++ b/gcc/config/bpf/core-builtins.cc @@ -205,7 +205,7 @@ struct cr_local /* Core Relocation Final data */ struct cr_final { - char *str; + const char *str; tree type; enum btf_core_reloc_kind kind; }; @@ -868,8 +868,10 @@ process_enum_value (struct cr_builtins *data) { if (TREE_VALUE (l) == expr) { - ret.str = (char *) ggc_alloc_atomic ((index / 10) + 1); - sprintf (ret.str, "%d", index); + char *tmp = (char *) ggc_alloc_atomic ((index / 10) + 1); + sprintf (tmp, "%d", index); + ret.str = (const char *) tmp; + break; } index++; @@ -987,7 +989,7 @@ process_type (struct cr_builtins *data) || data->kind == BPF_RELO_TYPE_MATCHES); struct cr_final ret; - ret.str = NULL; + ret.str = ggc_strdup ("0"); ret.type = data->type; ret.kind = data->kind; diff --git a/gcc/testsuite/gcc.target/bpf/core-builtin-type-based.c b/gcc/testsuite/gcc.target/bpf/core-builtin-type-based.c index 74a8d5a14d9d..9d818133c084 100644 --- a/gcc/testsuite/gcc.target/bpf/core-builtin-type-based.c +++ b/gcc/testsuite/gcc.target/bpf/core-builtin-type-based.c @@ -56,3 +56,4 @@ int foo(void *data) /* { dg-final { scan-assembler-times "0x8\[\t \]+\[^\n\]*bpfcr_kind" 13 } } BPF_TYPE_EXISTS */ /* { dg-final { scan-assembler-times "0x9\[\t \]+\[^\n\]*bpfcr_kind" 11 } } BPF_TYPE_SIZE */ /* { dg-final { scan-assembler-times "0xc\[\t \]+\[^\n\]*bpfcr_kind" 13 } } BPF_TYPE_MATCHES */ +/* { dg-final { scan-assembler-times "bpfcr_astr_off \[(\"\]+0\[(\"\]+" 37 } } */ diff --git a/gcc/testsuite/gcc.target/bpf/core-builtin-type-id.c b/gcc/testsuite/gcc.target/bpf/core-builtin-type-id.c index 4b23288eac08..9576b91bc940 100644 --- a/gcc/testsuite/gcc.target/bpf/core-builtin-type-id.c +++ b/gcc/testsuite/gcc.target/bpf/core-builtin-type-id.c @@ -38,3 +38,4 @@ int foo(void *data) /* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bpfcr_type" 0 { xfail *-*-* } } } */ /* { dg-final { scan-assembler-times "0x6\[\t \]+\[^\n\]*bpfcr_kind" 13 } } BPF_TYPE_ID_LOCAL */ /* { dg-final { scan-assembler-times "0x7\[\t \]+\[^\n\]*bpfcr_kind" 7 } } BPF_TYPE_ID_TARGET */ +/* { dg-final { scan-assembler-times "bpfcr_astr_off \[(\"\]+0\[(\"\]+" 20 } } */