Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: multi-compiler testing #10

Merged
merged 4 commits into from
Feb 20, 2025
Merged

Conversation

theihor
Copy link
Contributor

@theihor theihor commented Feb 7, 2025

Introduce matrix strategy to test workflow, testing different gcc compiler versions. Factor out build steps into a build.sh script.

Introduce matrix strategy to test workflow, testing different gcc
compiler versions. Factor out build steps into a build.sh script.

Signed-off-by: Ihor Solodrai <[email protected]>
@theihor theihor force-pushed the ci-multi-compiler branch 3 times, most recently from e7136b7 to 5e05121 Compare February 7, 2025 22:12
@theihor theihor marked this pull request as draft February 7, 2025 22:12
@theihor theihor changed the title ci: test various GCC versions ci: multi-arch multi-compiler Feb 7, 2025
@theihor theihor changed the title ci: multi-arch multi-compiler ci: multi-arch multi-compiler testing Feb 7, 2025
@theihor
Copy link
Contributor Author

theihor commented Feb 7, 2025

@danobi I've run into issues trying to get the latest version of bpftrace on arm64. I saw bpftrace/bpftrace#2868, so I understand there is no official arm64 binary/appimage.

I tried building from source using steps from Dockerfile.ubuntu, this doesn't work: cmake fails with a cmake error:

CMake Error at ast/CMakeLists.txt:87 (llvm_config):
  Unknown CMake command "llvm_config".

Locally, in a nixos/nix container, I also tried nix build #.appimage (as suggested in the docs) . This works on x86_64, but fails on --platform=linux/arm64 with:

sh-5.2# nix build #.appimage
error:
       … while setting up the build environment

       error: unable to load seccomp BPF program: Invalid argument

bpftrace can be installed with apt, but currently on ubuntu-arm runners it's a v0.20.2, which seems to not work with the USDT tests here (I don't know if bpftrace at fault here though):

root@f2ac51194176:/ci/workspace# make -C tests test
make: Entering directory '/ci/workspace/tests'
  TESTING  static/arg_consts
Error: no matching expected pattern for line #6
Error: not all patterns matched (stopped at line #6 out of 6)
BPFTRACE OUTPUT MISMTACH:
BPFTRACE SCRIPT:
BEGIN { printf("STARTED!\n"); }
U:.output/static/arg_consts:test:chars { printf("test:chars: arg0=%hhu arg1=%hhd\n", arg0, arg1); }
U:.output/static/arg_consts:test:shorts { printf("test:shorts: arg0=%hu arg1=%hd\n", arg0, arg1); }
U:.output/static/arg_consts:test:ints { printf("test:ints: arg0=%u arg1=%d\n", arg0, arg1); }
U:.output/static/arg_consts:test:longlongs { printf("test:longlongs: arg0=%llu arg1=%lld\n", arg0, arg1); }
U:.output/static/arg_consts:test:ptrs { printf("test:ptrs: arg0=%p arg1=%p\n", arg0, arg1); }
uretprobe:.output/static/arg_consts:main { exit(); }
END { printf("DONE!\n"); }
EXPECTED OUTPUT:
test:chars: arg0=1 arg1=-1
test:shorts: arg0=2 arg1=-2
test:ints: arg0=3 arg1=-3
test:longlongs: arg0=5 arg1=-5
test:ptrs: arg0=(nil) arg1=0x7
ACTUAL OUTPUT:
test:chars: arg0=1 arg1=-1
test:shorts: arg0=2 arg1=-2
test:ints: arg0=3 arg1=-3
test:longlongs: arg0=5 arg1=-5
test:ptrs: arg0=(nil) arg1=0x7
open(/sys/kernel/tracing/uprobe_events): No such file or directory
ERROR: failed to detach probe: uretprobe:.output/static/arg_consts:main
open(/sys/kernel/tracing/uprobe_events): No such file or directory
ERROR: failed to detach probe: usdt:.output/static/arg_consts:test:ptrs
open(/sys/kernel/tracing/uprobe_events): No such file or directory
ERROR: failed to detach probe: usdt:.output/static/arg_consts:test:longlongs
open(/sys/kernel/tracing/uprobe_events): No such file or directory
ERROR: failed to detach probe: usdt:.output/static/arg_consts:test:ints
open(/sys/kernel/tracing/uprobe_events): No such file or directory
ERROR: failed to detach probe: usdt:.output/static/arg_consts:test:shorts
open(/sys/kernel/tracing/uprobe_events): No such file or directory
ERROR: failed to detach probe: usdt:.output/static/arg_consts:test:chars
make: *** [Makefile:132: test-arg_consts] Error 1
make: Leaving directory '/ci/workspace/tests'

From the point of view of this CI the easiest way is to simply download arm64 appimage from github releases. Are there any plans to ship that?

@theihor
Copy link
Contributor Author

theihor commented Feb 7, 2025

@anakryiko a couple of questions to you.

Is it enough to test only x86_64 and arm64, or do we also want all other stuff (s390x, powerpc)?
If we need other arch-es too, then it will be necessary to add qemu/docker steps.

When testing other arch-es, do you want to test cross-compilation on x86_64 or native builds? Maybe both?

I briefly tried LLVM locally, and ran into errors. Haven't tried to debug, just FYI:

root@9fcbd55c4016:/ci/workspace# make -C tests test
make: Entering directory '/ci/workspace/tests'
  TESTING  static/arg_consts
  TESTING  static/arg_nums
  TESTING  static/arg_types
./run_test.sh: line 92: kill: (7271) - No such process
BPFTRACE STARTUP FAILURE!
BPFTRACE SCRIPT:
BEGIN { printf("STARTED!\n"); }
U:.output/static/arg_types:test:chars { printf("test:chars: arg0=%hhu arg1=%hhu arg2=%hhd\n", arg0, arg1, arg2); }
U:.output/static/arg_types:test:shorts { printf("test:shorts: arg0=%hu arg1=%hd\n", arg0, arg1); }
U:.output/static/arg_types:test:ints { printf("test:ints: arg0=%u arg1=%d\n", arg0, arg1); }
U:.output/static/arg_types:test:longlongs { printf("test:longlongs: arg0=%llu arg1=%lld\n", arg0, arg1); }
U:.output/static/arg_types:test:ptrs { printf("test:ptrs: arg0=%p arg1='%s' arg2=&%d\n", arg0, str(arg1), *(int32 *)arg2); }
U:.output/static/arg_types:test:arrs { printf("test:arrs: arg0='%s' arg1=(%d,%d,%d)\n", str(arg0),			*(int32 *)(arg1 + 0), *(int32 *)(arg1 + 4), *(int32 *)(arg1 + 8)); }
U:.output/static/arg_types:test:struct_by_val_reg { printf("test:struct_by_val_reg: arg0=%hhu arg1=%u\n", arg0, arg1); }
U:.output/static/arg_types:test:struct_by_val_reg_pair { printf("test:struct_by_val_reg_pair: s.x=%llx\n", arg0); }
U:.output/static/arg_types:test:structs_by_ref { printf("test:structs_by_ref: a=(%hhu) b=(%d) c=(%lld,%lld) d=(%lld,%lld,%lld)\n", *(uint8 *)arg0, *(int32 *)arg1,							*(int64 *)arg2, *(int64 *)(arg2 + 8),						*(int64 *)arg3, *(int64 *)(arg3 + 8), *(int64 *)(arg3 + 16)); }
uretprobe:.output/static/arg_types:main { exit(); }
END { printf("DONE!\n"); }
BPFTRACE OUTPUT:
Attaching 12 probes...
ERROR: Error loading BPF program for usdt__output_static_arg_types_test_struct_by_val_reg_pair_loc0_9. Use -v for full kernel error log.
ERROR: Loading BPF object(s) failed.
make: *** [Makefile:139: test-arg_types] Error 1
make: Leaving directory '/ci/workspace/tests'

@anakryiko
Copy link
Member

Is it enough to test only x86_64 and arm64, or do we also want all other stuff (s390x, powerpc)? If we need other arch-es too, then it will be necessary to add qemu/docker steps.

Well, as many as possible, within reasonable amount of effort to implement (and maintain!) that. So hard to answer this definitively without knowing how much effort it would require.

But to be practical, I'd start with x86_64 and arm64, as two most popular architectures. So let's avoid extra qemu work for now.

When testing other arch-es, do you want to test cross-compilation on x86_64 or native builds? Maybe both?

See above, all depends on amount of effort. It's all tradeoffs. Your time and effort are not free. So let's make the decision based on projected amount of effort.

Optionally build bpftrace from source.

Signed-off-by: Ihor Solodrai <[email protected]>
The "struct_by_val_reg" case of arg_types tests fails with BPF
verifier error when compiled with clang.

It appears that bpftrace generates a faulty program in such case,
possibly due to the difference between how clang and gcc encode USDT
probe arguments.

Since this is the only problematic test case for LLVM, make it
conditional on __clang__ for now.

Signed-off-by: Ihor Solodrai <[email protected]>
Update CI scripts to also build the tests with various versions of
LLVM. Versions below 17 can't be installed with the official llvm.sh
script, so for now let's test only 17+.

Signed-off-by: Ihor Solodrai <[email protected]>
@theihor theihor changed the title ci: multi-arch multi-compiler testing ci: multi-compiler testing Feb 14, 2025
@theihor theihor marked this pull request as ready for review February 14, 2025 01:02
@theihor theihor requested a review from anakryiko February 14, 2025 01:02
@theihor
Copy link
Contributor Author

theihor commented Feb 14, 2025

@anakryiko I think we can land this in current state. We get testing of a couple of versions of GCC and LLVM. Enabling arm64 will be trivial, if we get bpftrace appimage (or static build) for arm.

Re the problematic test case: I suspect this may be a bug in bpftrace, because it produces a BPF program that fails verification:

Kernel error log: 
0: R1=ctx() R10=fp0
;  @ bpftrace.bpf.o:0
0: (b7) r2 = 8                        ; R2_w=8
1: (7b) *(u64 *)(r10 -24) = r2        ; R2_w=8 R10=fp0 fp-24_w=8
2: (79) r3 = *(u64 *)(r1 +32)         ; R1=ctx() R3_w=scalar()
3: (07) r3 += -16                     ; R3_w=scalar()
4: (bf) r1 = r10                      ; R1_w=fp0 R10=fp0
5: (07) r1 += -8                      ; R1_w=fp-8
6: (b7) r2 = 16                       ; R2_w=16
7: (85) call bpf_probe_read_user#112
invalid indirect access to stack R1 off=-8 size=16
processed 8 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0

If I'm reading this correctly, it tries to read 16 bytes from %rsp-8, going over it.

I am guessing the reason there is a difference with gcc is in how trace probe arguments are passed. Compare:

  • gcc Arguments: 16@%rax
  • LLVM Arguments: 16@-16(%rbp)

This is a case when a struct with two longs is passed by value:

struct st_regpair { long x, y; };

And then bpftrace script is:

U:.output/static/arg_types:test:struct_by_val_reg_pair { printf("test:struct_by_val_reg_pair: s.x=%llx\n", arg0); }

For this PR I conditionalized the case on #ifndef __clang__.

@theihor
Copy link
Contributor Author

theihor commented Feb 14, 2025

Opened an issue at bpftrace: bpftrace/bpftrace#3798

Copy link
Member

@anakryiko anakryiko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not really clang vs gcc issue, it's more of an level of optimization, but let's land it as is while we are figuring out where exactly to fix the issue (usdt.h or bpftrace/bcc)

@anakryiko anakryiko merged commit d1f4677 into libbpf:main Feb 20, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants