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

[Ubuntu Upgrade] Disable MSAN builds on projects that will break. #6236

Closed
wants to merge 3 commits into from

Conversation

jonathanmetzman
Copy link
Contributor

Related: #6180

@jonathanmetzman
Copy link
Contributor Author

jonathanmetzman commented Aug 17, 2021

Note to any project maintainers coming across this: We will explain what's going on here shortly.

continue
with open(project_yaml_path) as fp:
project_yaml = fp.read()
if 'memory' in project_yaml:
Copy link
Contributor

@asraa asraa Aug 17, 2021

Choose a reason for hiding this comment

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

might just be one minor false positive

load_from_memory_fuzzer:

edit might also impact the yaml rewrite below

Copy link
Contributor

Choose a reason for hiding this comment

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

oh did you manually track this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I wont commit this script. Just wanted to include it so I wouldn't lose track of it.

@jonathanmetzman
Copy link
Contributor Author

So my testing revealed that the method we used, looking at instrumented_libraries, does not work.
There are about 44 projects that break with MSAN after the upgrade.
Worse, this method may have false positives.
For example, it says systemd doesn't work with MSAN even though testing (through bad build check) reveals no issue.
Maybe bad build check is not thorough enough. Another possibility explanation for false positives is not every linked library is used at runtime.
I'm not sure why there are so many false negatives. Maybe because of dynamic linking and copying libs to /out? I doubt that explains all of this.
I'm leaning towards ignoring this method and using bad_build_check for determining if MSAN should be disabled. What do you all think?

@asraa
Copy link
Contributor

asraa commented Aug 19, 2021

I'm leaning towards ignoring this method and using bad_build_check for determining if MSAN should be disabled. What do you all think?

Do you think the small number of runs for bad_build_check will catch all the errors? (like maybe some weird runtime errors might pop up later?) What about checking against the seed corpus? I think that's currently disabled, so not sure if there were existing problems with that. Or, are all fuzzers guaranteed to run ok on their GCS corpuses (are crashing entries only uploaded on success?) if so maybe that suffices but that seems like added code to try out

@jonathanmetzman
Copy link
Contributor Author

Do you think the small number of runs for bad_build_check will catch all the errors? (like maybe some weird runtime errors might pop up later?)

Good point. bad build check probably doesn't catch everything. Maybe we should disable msan on the union of this method and bad build check.

What about checking against the seed corpus? I think that's currently disabled, so not sure if there were existing problems with that.

Yes, that feature is disabled, we aren't even testing against the seed corpus.

Or, are all fuzzers guaranteed to run ok on their GCS corpuses (are crashing entries only uploaded on success?) if so maybe that suffices but that seems like added code to try out

Nope we don't do this.

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.

3 participants