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

Various fixes for CI #2763

Merged
merged 2 commits into from
Sep 15, 2023
Merged

Conversation

viktormalik
Copy link
Contributor

@viktormalik viktormalik commented Sep 14, 2023

  • GitHub Actions runners seem to have updated kernel and the old version of biosnoop.bt is no longer necessary.
    Unfortunately, it looks like this is the case for some runners only, so we cannot universally use neither the old nor the new variant. Let's disable the test for now and revisit later when all runners are updated.

  • We're also observing CI failure in memleak-test.sh. It's not related to the memleak test itself, the bug is in ClangParser which one of the memleak tests executes:

      # cat script.bt
      #include <linux/skbuff.h>
      BEGIN {}
    
      # bpftrace script.bt
      [...]
      /lib/modules/[...]/ibt.h:71:10: error: use of undeclared identifier 'true'
      [...]
      /lib/modules/[...]/le.h:36:2: error: use of undeclared identifier 'uintptr_t'
    

    This is fixed by improving our parsing of undefined typedefs from Clang diagnostic messages to be able to pull those typedefs from BTF.

Checklist
  • Language changes are updated in man/adoc/bpftrace.adoc and if needed in docs/reference_guide.md
  • User-visible and non-trivial changes updated in CHANGELOG.md
  • The new behaviour is covered by tests

GitHub Actions runners seem to have updated kernel and the old version
of `biosnoop.bt` is no longer necessary. Unfortunately, it looks like
this is the case for some runners only, so we cannot universally use
neither the old nor the new variant.

Let's disable the test for now and revisit later when all runners are
updated.
@viktormalik
Copy link
Contributor Author

There's one more new CI failure (probably also caused by the kernel update), I'll add it to this PR, soon.

@viktormalik viktormalik added the do-not-merge Changes are not ready to be merged into master yet label Sep 14, 2023
@viktormalik viktormalik changed the title CI: temporarily disable biosnoop.bt test Various fixes for CI Sep 14, 2023
We are seeing failures of some simple scripts using manual includes:

    # cat script.bt
    #include <linux/skbuff.h>
    BEGIN {}

    # bpftrace script.bt
    [...]
    /lib/modules/[...]/ibt.h:71:10: error: use of undeclared identifier 'true'
    [...]
    /lib/modules/[...]/le.h:36:2: error: use of undeclared identifier 'uintptr_t'

Let's improve our parsing of undefined typedefs from Clang diagnostic
messages to be able to pull those typedefs from BTF.
@viktormalik viktormalik removed the do-not-merge Changes are not ready to be merged into master yet label Sep 14, 2023
@viktormalik viktormalik added the do-not-squash Do not squash PR commits label Sep 14, 2023
@viktormalik
Copy link
Contributor Author

@danobi I added one more CI fix, could you please re-check? Thanks!

Copy link
Member

@danobi danobi left a comment

Choose a reason for hiding this comment

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

lgtm!

@danobi danobi merged commit 12c463e into bpftrace:master Sep 15, 2023
21 checks passed
@viktormalik viktormalik deleted the ci-biosnoop-disable branch September 18, 2023 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-squash Do not squash PR commits
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants