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.

Sign up for GitHub

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

Jump to bottom

Add test with ASAN enabled. #2313

Merged
merged 29 commits into from
Jun 10, 2024
Merged

Add test with ASAN enabled. #2313

merged 29 commits into from
Jun 10, 2024

Conversation

Rot127
Copy link
Collaborator

@Rot127 Rot127 commented Apr 7, 2024 •
edited
Loading

Your checklist for this pull request

  • I've documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
  • I've added tests that prove my fix is effective or that my feature works (if possible)

Detailed description

Add ASAN builds

It does a few things

  • Fixes the CAPSTONE_DIET build. Though it is not tested, since we cannot do it with the current test design, which relies on printing everything.
  • Fix a lot of mem leaks,
  • Fix the build of cstest via CMakeLists.txt. This allows now also to enable ASAN for the cstest. Also it is much easier to build cstest now.
  • Fixes cstest-report.py to work also when cstest is installed and in PATH.
  • Make run_invalid_cstool.sh more verbose to help spotting possible ASAN errors.
  • Add and fix tests/cs_details/issue.cs to the CI.

Test plan

CI green (or, if everything fails horribly, mark it optoinal for now)

Closing issues

closes #2368

@Rot127 Rot127 added build & packaging Build system and packaging related Testing Test related issue labels Apr 7, 2024
@Rot127 Rot127 added this to the v6 milestone Apr 7, 2024
@Rot127 Rot127 force-pushed the ASAN branch 2 times, most recently from af12ea4 to 24616c6 Compare April 7, 2024 11:20
@github-actions github-actions bot added the CS-core-files auto-sync label Apr 17, 2024
@Rot127 Rot127 force-pushed the ASAN branch 8 times, most recently from 2719ec4 to 4ca2ce5 Compare April 22, 2024 09:52
@Rot127 Rot127 marked this pull request as ready for review April 22, 2024 11:00
@github-actions github-actions bot added the ARM Arch label Apr 22, 2024
@Rot127 Rot127 mentioned this pull request May 11, 2024
Merged
6 tasks
@Rot127 Rot127 force-pushed the ASAN branch 2 times, most recently from 6c97062 to 3b53ed9 Compare May 20, 2024 10:52
@Rot127 Rot127 mentioned this pull request May 26, 2024
Closed
2 tasks
@XVilka

This comment was marked as resolved.

Sign in to view
@github-actions github-actions bot added PowerPC Arch HPPA Arch labels Jun 1, 2024
@Rot127
Copy link
Collaborator Author

Rot127 commented Jun 1, 2024 •
edited
Loading

I remove the DIET changes again. Build is still fixed.
They are too complicated to fix for now and not testable. Because we can only test the details and decoding by printing it first, all the test cases of us fail.
So I think if we fix it it should be done once in a separated PR.
Also we can reduce the binary size even more.

XVilka
XVilka approved these changes Jun 1, 2024
View reviewed changes
Copy link
Contributor

@XVilka XVilka left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Cool! In the future we also should enable UBSAN, I know there are many potential places that can trigger it. But that's a story for another PR...

@XVilka
Copy link
Contributor

XVilka commented Jun 1, 2024

@kabeor please take a look and lets merge this quickly, so new PRs will not introduce ASAN warnings.

@XVilka
Copy link
Contributor

XVilka commented Jun 8, 2024 •
edited
Loading

@kabeor @aquynh

@XVilka
Copy link
Contributor

XVilka commented Jun 9, 2024

@kabeor while you are online, could you please merge also this one and the LoongArch PRs?

@kabeor
Copy link
Member

kabeor commented Jun 10, 2024

Great, thanks!

@kabeor kabeor merged commit 0a67596 into capstone-engine:next Jun 10, 2024
15 checks passed
@Rot127 Rot127 deleted the ASAN branch June 15, 2024 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XVilka XVilka XVilka approved these changes

@kabeor kabeor kabeor approved these changes

Assignees
No one assigned
Labels
ARM Arch build & packaging Build system and packaging related CS-core-files auto-sync Documentation HPPA Arch PowerPC Arch Testing Test related issue
Projects
Capstone V6 Plan
Status: Done
Milestone
v6 - Alpha
Development

Successfully merging this pull request may close these issues.

[Tests] Add "DIET" CI job
3 participants
@Rot127 @XVilka @kabeor