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

Conformance testing in CI/CD skipped for atomic instructions #3188

Closed
dthaler opened this issue Jan 20, 2024 · 0 comments · Fixed by #3195
Closed

Conformance testing in CI/CD skipped for atomic instructions #3188

dthaler opened this issue Jan 20, 2024 · 0 comments · Fixed by #3195
Assignees
Labels
bug Something isn't working tests triaged Discussed in a triage meeting
Milestone

Comments

@dthaler
Copy link
Collaborator

dthaler commented Jan 20, 2024

Describe the bug

PR #1936 added support to the bpf2c code generator for atomic instructions.
The conformance test suite can run and pass for the atomic instructions, but
.github/workflows/cicd.yml and .github/workflows/cicd-release-validation.yml
exclude the atomic tests via --exclude_regex lock*. That regex excludes anything with
"loc" followed by 0 or more 'k' characters. It thus skips all tests with lock in the filename (which is the atomic tests), but also call_local.data (which is not yet supported by bpf2c).

Apparently this is because CI/CD tests seem to run with NO_CRT (not sure how that gets defined)
and https://github.com/microsoft/ebpf-for-windows/blob/main/include/bpf2c.h had

#if defined(NO_CRT)
#else
...
#include <intrin.h>
#endif

As a result, trying to run the atomic conformance tests in CI/CD currently results in

C:\Users\RUNNER~1\AppData\Local\Temp\4543451591205265025.cpp(36): error C3861: '_InterlockedExchangeAdd64': identifier not found

OS information

No response

Steps taken to reproduce bug

Created a PR that removes the exclusion of lock*

Expected behavior

CI/CD should run the atomic conformance tests and they should pass.

Actual outcome

Error

Additional details

The verifier doesn't yet support atomics, but vbpf/ebpf-verifier#558 is waiting to be merged and adds support. bpf2c.exe will then support atomics.
The conformance tests do not use bpf2c.exe or the verifier though.

@dthaler dthaler added bug Something isn't working tests labels Jan 20, 2024
@dahavey dahavey added this to the 2401 milestone Jan 22, 2024
@dahavey dahavey added the triaged Discussed in a triage meeting label Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working tests triaged Discussed in a triage meeting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants