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

For discussion: Introduce sanitizer options and run with sanitizer on Linux/MacOS #78

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kevemueller
Copy link

Clang has excellent sanitizers, that are very well supported under MacOS and Linux. Add AddressSanitizer, LeakSanitizer and UndefinedBehaviourSanitizer configure options.

Introduce GH action to run the tests with and without sanitizers.

After #77 I have suppressed the leaks during the pkg builds. I thought I would introduce the sanitizers and a corresponding CI run also here.
The patch got bigger than I hoped it would be and has some issues I would like to discuss here:

  • LeakSanitizer is reporting issues and the way kyua is dealing with them is suboptimal
    • kyua cannot list the testcase as LeakSanitizer prints its report to stderr and exits non-zero
    • even if exitcode is overridden, kyua still bails on having something on stderr
    • if both are overridden, all the relevant information (status + report) are lost in a file
    • An example of a manual run if the test is in the logs
  • Under MacOS the stack traces are ugly, must be an optimization I am not catching, tried with -fomit-frame-pointer at no avail.
  • Good news is that UndefinedBehaviour did not report any issue.

The sanitizers affect both compiler flags as well as ldflags, they need to be passed down to the pkg-config script, otherwise an instrumented atf cannot be linked against by a compliant user looking at pkg-config.
I have abandoned automake >20y ago, so the hack that I have put in place should be re-done by somebody actually knowing automake.

Questions:

  • Is there general interest to incorporate this here?
  • What can be done to make kyua pass the listing of the test cases phase even if there is something on stderr. NB: This seems to be something Ubuntu specific, as under MacOS it runs through.
  • Can somebody please look at the automake stuff and clean it up.

I case of interest I can continue supporting this until it produces reasonable output (atf report showing the leaks, bailing out with test failure), but I will not be able to go into fixing all the leaks.

Clang has excellent sanitizers, that are very well supported under MacOS and Linux.
Add AddressSanitizer, LeakSanitizer and UndefinedBehaviourSanitizer configure options.

Introduce GH action to run the tests with and without sanitizers.
@kevemueller
Copy link
Author

Also, I had to disable FORTIFY_SOURCE as that is known to mess with the Sanitizer. This obviously should also be only done using automake magic if the sanitizer is requested.

@kevemueller
Copy link
Author

The action output from this change can be seen under
https://github.com/kevemueller/atf/actions/runs/12201113085

@ngie-eign
Copy link
Contributor

ngie-eign commented Dec 7, 2024

I like this idea in theory, but personally I would really like for the GH Actions piece to be done separate from the *SAN CI introduction work.

FWIW, I don't know why our Cirrus CI integration isn't working for this project. I have lowered privileges in this repo and the others, so I can't confirm/opt-in to whatever infrastructure is needed on the Cirrus CI or GitHub Actions side of things to enable these changes.

PS I really want to get away from the autotools-based build system we inherited with these projects. There are a few hacks/some complexity kicking around the ATF/kyua/lutok source base that deal with libtool, and some of the scripts under admin/ accomplish tasks with bespoke logic that cmake does natively. Leveraging cmake would allow us to modernize/clean up the codebase a bit. The only catch is that it would need to be done in a top-down manner (kyua -> lutok -> atf), since the projects rely on one another (m4 files, pkgconfig files, etc).

CC: @emaste , @lwhsu

@kevemueller
Copy link
Author

This was just a draft to get discussion started. Yes, I agree that the improvements need to be put into separate PRs, but they only show off in this draft when put together.

This work uses GH actions. There is nothing that needs to be set-up, once you have a .github/workflows folder it is going to be evaluated. It can play together with other external providers, pkg uses SourceHut which is way better than GH and CirrusCI.

1/
I can trim down the CI action to run only the uninstrumented builds. That means you only need to look at build.xml and start benefiting from it right away. 5 minutes :)

2/
If you anyway want to get rid of automake, I suggest you take a look at https://mesonbuild.com/ IMO this is the way to go for anybody unhappy with their current build system. If you don't like the dependency on Python, there is a https://github.com/muon-build/muon a pure C implementation.
You can mix automake and meson.build, but obviously the whole chain should move rather swiftly to a coherent system.
I would be more than happy to help on this. I probably spent more time on the above 'small' fix with automake than it would have taken me to move the whole shebang to meson.build.

As detailed above, there are changes needed in kyua to improve its resilience to instrumented builds which fail at places kyua is not expecting. Most notably listing the testcases with -l must handle stderr output and ignore exit code.

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