Fix arm64 testsuite failures

[lmb]

Tests now pass on arm64:

$ go test -exec sudo ./...
ok  	github.com/cilium/ebpf	0.473s
ok  	github.com/cilium/ebpf/asm	0.014s
ok  	github.com/cilium/ebpf/btf	0.671s
ok  	github.com/cilium/ebpf/cmd/bpf2go	0.717s
ok  	github.com/cilium/ebpf/cmd/bpf2go/test	0.040s
ok  	github.com/cilium/ebpf/features	0.143s
ok  	github.com/cilium/ebpf/internal	0.024s
?   	github.com/cilium/ebpf/internal/cmd/gentypes	[no test files]
ok  	github.com/cilium/ebpf/internal/epoll	1.020s
ok  	github.com/cilium/ebpf/internal/sys	0.015s
?   	github.com/cilium/ebpf/internal/testutils	[no test files]
?   	github.com/cilium/ebpf/internal/unix	[no test files]
ok  	github.com/cilium/ebpf/link	1.653s
ok  	github.com/cilium/ebpf/perf	0.260s
ok  	github.com/cilium/ebpf/ringbuf	0.108s
ok  	github.com/cilium/ebpf/rlimit	0.007s

Best review on a per-commit basis.

Updates #266

[lmb]

@rgo3 can you please take a look at e2d485b ?

@timo 90411de needs your attention, I think it might make tests fail on older kernels.

[rgo3]

@lmb The HaveProgramHelper API in the features package also returns a few more unwrapped errors, specifically:

• func validateProgramHelper
• func haveProgramHelper

[lmb]

Thanks! Just saw the following as well:

// EPERM is kept as-is and is not converted or wrapped.

Why is that?

[rgo3]

Only real explanation I can give you is in @ti-mo's commit message. To me returning EPERM directly feels reasonable, I'm not sure if there is any benefit to wrapping it.

[lmb]

Ok, I read that as "don't let EPERM fall into the default switch case". Wrapping the error forces use of errors.Is which gives you more flexibility in the future if you need to change the returned error somehow.

[lmb]

Ugh, finally a green build. I'll merge this tomorrow-ish unless someone objects.

[ti-mo]

// EPERM is kept as-is and is not converted or wrapped.

Why is that?

Ha, I just asked myself the same question today while working on #746. We should probably remove it, it complicates the logic unnecessarily.

I want to wrap everything except ErrNotSupported using a defer closure, as it'll make the switch statement much more straightforward and easier to extend. Looking at your patches here, we might as well wrap every returned error and set the error string containing %w depending on the error type.

[ti-mo]

@lmb I've dropped eb06124 from the PR since it breaks API, and from what I understand this is more cosmetic than functional. Callers might already rely on ENOENT specifically being returned for missing symbols across all kernels, as well as when scanning for valid offsets within a known symbol.

The original meaning of ENOENT in this context is 'symbol not found', which, with some creativity, could be interpreted as 'offset not found' when trying to attach to specific offsets, although that might be a stretch. Could you maybe try to tackle this from another angle? I don't see a simple way out here.

[lmb]

Turning EINVAL into ENOENT is just plain wrong unfortunately, and does obscure bugs. There are a ton of reasons why perf event open will return EINVAL, some of them arch specific, that don't come from the symbol being missing. Trying to paper them over doesn't work, so I think the breakage is inevitable.

[ti-mo]

@lmb Fair enough, will add it back in through a separate PR. This'll make it pop up in the changelog, so we won't forget to add a breaking change to the relnotes.