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

Enables MSAN for Suricata #4455

Merged
merged 4 commits into from
Sep 30, 2020
Merged

Conversation

catenacyber
Copy link
Contributor

Will work when OISF/suricata#5422 gets merged

@@ -45,6 +45,7 @@ make install
cd ..

export CARGO_BUILD_TARGET="x86_64-unknown-linux-gnu"
rustup component add rust-src --toolchain nightly
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You may want to do this in the Docker image, so that it works for other Rust projects

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you submit a patch to https://github.com/google/oss-fuzz/blob/master/infra/base-images/base-builder/Dockerfile#L79 with a comment on why it is needed. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It comes from this discussion :
#3469 (comment)

We need to have -Z build-std to rebuild Rust standard library with MSAN

And to have this to work, we need a few things including this rustup component add rust-src cf
https://doc.rust-lang.org/cargo/reference/unstable.html#build-std

Copy link
Collaborator

@inferno-chromium inferno-chromium left a comment

Choose a reason for hiding this comment

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

Can you check why msan build is still failing

@@ -45,6 +45,7 @@ make install
cd ..

export CARGO_BUILD_TARGET="x86_64-unknown-linux-gnu"
rustup component add rust-src --toolchain nightly
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you submit a patch to https://github.com/google/oss-fuzz/blob/master/infra/base-images/base-builder/Dockerfile#L79 with a comment on why it is needed. Thanks!

@catenacyber
Copy link
Contributor Author

MSAN build is failing because we need Suricata upstream merge of OISF/suricata#5422

@inferno-chromium
Copy link
Collaborator

Ping us here once this is ready to merge.

@catenacyber
Copy link
Contributor Author

Ping @inferno-chromium
Can we run CI again ?

@jonathanmetzman
Copy link
Contributor

Ping @inferno-chromium
Can we run CI again ?

Tip: You can force this yourself by pushing to this PR.

@catenacyber
Copy link
Contributor Author

Thanks Johnathan, I rebased and force-pushed

@catenacyber
Copy link
Contributor Author

Does the CI take into account the changes to base-builder ?

@jonathanmetzman
Copy link
Contributor

Does the CI take into account the changes to base-builder ?

I think so?

@catenacyber
Copy link
Contributor Author

If the CI succeeds with this new commit, I guess CI was wrong last time

@inferno-chromium
Copy link
Collaborator

Does the CI take into account the changes to base-builder ?

I dont think so. But this should work, let me remove your dummy thing and commit. keep an eye on build tmrw.

@inferno-chromium inferno-chromium merged commit f9f99a9 into google:master Sep 30, 2020
@jonathanmetzman
Copy link
Contributor

Does the CI take into account the changes to base-builder ?

Maybe I misunderstood what you meant. Did you make changes in this PR?
Then CI not taking this into account is expected.

@catenacyber
Copy link
Contributor Author

Looks like it is working :-)
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=26081

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