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

Use strong-xml instead of serde and generalize frontend #67

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

Conversation

TheWastl
Copy link
Contributor

@TheWastl TheWastl commented Jun 4, 2022

Use strong-xml instead of serde

serde doesn't work very well with XML, see here. This PR uses strong-xml instead, and strum for parsing enums from strings. strong-xml's #[derive(XmlRead)] is not as versatile as serde's #[derive(Deserialize)], however, so there are certainly also drawbacks to this switch

Display all errors, not only leaks

Previously, only errors that leaked at least 1 byte were displayed. This excludes double-frees, access to unallocated or freed memory, use of uninitialized values and other errors detected by Valgrind. They are important too, and are handled now.

Display <auxwhat>s and additional stacks

Some error kinds (for instance, double-frees) output more information than a single line of description and one stack trace. This can easily be parsed with strong-xml, and is then displayed to the user. This fixes #20, #54 and #55.

Use the description provided by valgrind instead of providing our own

Previously, all error kinds would simply display the generic message "Leaked <amount> in <n> blocks". We now support other error kinds, so this had to be changed. The alternative would be to provide our own messages for every error kind.

- use strong-xml instead of serde
- display all errors, not only leaks
- display <auxwhat>s and additional stacks
- use the description provided by valgrind instead of providing our own
@TheWastl
Copy link
Contributor Author

TheWastl commented Jun 4, 2022

Ok, so the issue is apparently that libstd uses statx with a null filename parameter to check if statx is implemented. We could ignore all errors of kind SyscallParam (not a fan of that, because it could suppress legitimate bugs, too), or provide default suppressions for libstd (a bit cumbersome).

(the tests ran fine on my machine)

@TheWastl
Copy link
Contributor Author

Apparently Valgrind added a workaround for this "dubious use of statx" in 2a7d3ae76.

@jfrimmel
Copy link
Owner

jfrimmel commented Jul 28, 2022

Hi, @TheWastl,

what is the current state of this PR? Does this require another code change or a change in CI?

Anyways: thanks for your work and patience!

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.

Invalid Free causes parsing the valgrind output to fail
2 participants