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

xst: disable lsan when xs exit imminent #1067

Closed

Conversation

raphdev
Copy link
Contributor

@raphdev raphdev commented Mar 20, 2023

This PR affects only oss-fuzz build target for xst.

When ASAN is enabled, by default it reports all allocated memory at exit as memory leaks. However, this interferes with patterns that do not free memory when program exit is imminent as the allocated memory will be reclaimed at exit anyway. In XS, generally this is the case when fatal errors occur; those reports are not true memory leaks.

Libfuzzer, as an in-process fuzzer, additionally tracks the ratio of malloc to free calls during each test case to determine if it should call LeakSanitizer's interface for on-demand leak detection. It uses the on-demand check to find leaks during test cases instead of ASAN's default at exit detection.

Ideally, we want leak detection to work, while not reporting leaks that would have been immediately freed at program exit.

To do this, we implement __lsan_is_turned_off to disallow leak checking at certain parts of the program, independent of whether libfuzzer enables or disables lsan internally. The documentation states that it must return a constant value, but for our case it works as expected (lsan check impl for reference). With this change, we no longer need to suppress callsites where allocations may not be freed on fatal errors.

Tested against all current variants of false-positive memory leaks, and they no longer reproduce.

A tradeoff may be more out-of-memory issues reported, since libfuzzer is in-process and does not exit when XS would exit.

mkellner pushed a commit that referenced this pull request Mar 21, 2023
@raphdev
Copy link
Contributor Author

raphdev commented Mar 21, 2023

Closing as this got merged outside PR.

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.

None yet

1 participant