On Wed, Nov 2, 2022 at 8:35 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> Tests to verify the following behavior of `btf_dedup_resolve_fwds`:
> - remapping for struct forward declarations;
> - remapping for union forward declarations;
> - no remapping if forward declaration kind does not match similarly
> named struct or union declaration;
> - no remapping if forward declaration name is ambiguous;
> - base ids are considered for fwd resolution in split btf scenario.
>
> Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
> Reviewed-by: Alan Maguire <alan.maguire@xxxxxxxxxx>
> ---
> tools/testing/selftests/bpf/prog_tests/btf.c | 152 ++++++++++++++++++
> .../bpf/prog_tests/btf_dedup_split.c | 45 ++++--
> 2 files changed, 182 insertions(+), 15 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
> index 127b8caa3dc1..f14020d51ab9 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf.c
> @@ -7598,6 +7598,158 @@ static struct btf_dedup_test dedup_tests[] = {
> BTF_STR_SEC("\0e1\0e1_val"),
> },
> },
> +{
> + .descr = "dedup: standalone fwd declaration struct",
> + /*
> + * // CU 1:
> + * struct foo { int x; };
> + * struct foo *a;
I was quite confused what did you mean here.. and I think you meant
just `struct foo;` as FWD, right? But then you also wanted to have
pointers to check ref type resolution, right? And "a" and "b" are
completely unrelated and very misleading, you don't seem to actually
define them.
So why not use typedefs to tie PTR, FWD and STRUCT together? Basically:
// CU1
struct foo { int x; };
struct foo;
typedef struct foo *foo_t;
if you treat this literally, compiler would just omit forward
declaration, so it's not 100% identical, but I think it communicates
intent better. In BTF you can control this better and make sure you
have TYPEDEF -> PTR -> FWD
> + *
> + * // CU 2:
> + * struct foo;
> + * struct foo *b;
> + */
> + .input = {
> + .raw_types = {
> + BTF_STRUCT_ENC(NAME_NTH(1), 1, 4), /* [1] */
> + BTF_MEMBER_ENC(NAME_NTH(2), 2, 0),
> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [2] */
> + BTF_PTR_ENC(1), /* [3] */
> + BTF_FWD_ENC(NAME_TBD, 0), /* [4] */
> + BTF_PTR_ENC(4), /* [5] */
> + BTF_END_RAW,
> + },
> + BTF_STR_SEC("\0foo\0x"),
> + },
> + .expect = {
> + .raw_types = {
> + BTF_STRUCT_ENC(NAME_NTH(1), 1, 4), /* [1] */
> + BTF_MEMBER_ENC(NAME_NTH(2), 2, 0),
> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [2] */
> + BTF_PTR_ENC(1), /* [3] */
> + BTF_END_RAW,
> + },
> + BTF_STR_SEC("\0foo\0x"),
> + },
> +},
> +{
> + .descr = "dedup: standalone fwd declaration union",
> + /*
> + * // CU 1:
> + * union foo { int x; };
> + * union foo *another_global;
> + *
> + * // CU 2:
> + * union foo;
> + * union foo *some_global;
same, those "global" variables are confusing and are not really
present in BTFs you are testing, so I'd avoid specifying them in
comments
> + */
> + .input = {
> + .raw_types = {
> + BTF_UNION_ENC(NAME_NTH(1), 1, 4), /* [1] */
> + BTF_MEMBER_ENC(NAME_NTH(2), 2, 0),
> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [2] */
> + BTF_PTR_ENC(1), /* [3] */
> + BTF_FWD_ENC(NAME_TBD, 1), /* [4] */
> + BTF_PTR_ENC(4), /* [5] */
> + BTF_END_RAW,
> + },
> + BTF_STR_SEC("\0foo\0x"),
> + },
> + .expect = {
> + .raw_types = {
> + BTF_UNION_ENC(NAME_NTH(1), 1, 4), /* [1] */
> + BTF_MEMBER_ENC(NAME_NTH(2), 2, 0),
> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [2] */
> + BTF_PTR_ENC(1), /* [3] */
> + BTF_END_RAW,
> + },
> + BTF_STR_SEC("\0foo\0x"),
> + },
> +},
[...]
