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

Add 2 more VCPKG patches for OSS-fuzz #207

Merged

Conversation

luckychess
Copy link
Contributor

Description of the Change

Pull request adds 2 more patches for vcpkg to make possible building fuzzing targets inside OSS-Fuzz image. They provide a workaround for building some problematic dependencies (boost-locale and grpc).

Benefits

Ready to open pull request to the OSS-Fuzz!

Possible Drawbacks

None?

Usage Examples or Tests

Ask @luckychess to explain how to build everything inside the OSS-Fuzz image.

Signed-off-by: Konstantin Munichev <toobwn@gmail.com>
Signed-off-by: Konstantin Munichev <toobwn@gmail.com>
Signed-off-by: Konstantin Munichev <toobwn@gmail.com>
boost-type-traits:
boost-unordered:
boost-vcpkg-helpers:
libiconv:

Choose a reason for hiding this comment

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

Why libiconv is put into VCPKG_BOOST_LOCALE_DEPS_LIST ? How it is related to BOOST deps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file contains a list of boost-locale dependencies (you can check them at https://github.com/microsoft/vcpkg/blob/master/ports/boost-locale/CONTROL). The actual reason why this list even exists is that boost-locale is broken - can't be built under sanitizers.
So the whole idea is next: build all dependencies under sanitizers, then build boost-locale without them and then build the rest with sanitizers again.


- string(APPEND CMAKE_C_FLAGS_INIT " ${CMAKE_C_FLAGS} ")
- string(APPEND CMAKE_CXX_FLAGS_INIT " ${CMAKE_CXX_FLAGS} ")
+ string(APPEND CMAKE_C_FLAGS_INIT " ${CMAKE_C_FLAGS} -fsanitize=$ENV{SANITIZER} ")

Choose a reason for hiding this comment

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

Why -fsanitize is not enclosed by "if"?
What will happen when $ENV{SANITIZER} is not set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest, I don't really care. It's intended to work only in a very specific environment (OSS-Fuzz container) which guarantees for specific environment variables are set.

index 70f224da..68d90470 100644
--- a/scripts/toolchains/linux.cmake
+++ b/scripts/toolchains/linux.cmake
@@ -17,6 +17,9 @@ if(NOT _CMAKE_IN_TRY_COMPILE)

Choose a reason for hiding this comment

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

What is the meaning of _CMAKE_IN_TRY_COMPILE ?

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's hard to say. I have to modify build flags globally for all packages but vcpkg doesn't have a way to do it (https://github.com/Microsoft/vcpkg/blob/master/docs/about/faq.md#can-i-use-my-ownspecific-flags-for-rebuilding-libs). So I found a workaround: microsoft/vcpkg#3577.

@luckychess luckychess merged commit 532cd93 into hyperledger-iroha:master Aug 22, 2019
@luckychess luckychess deleted the fix/vcpkg-deps-patches branch August 22, 2019 16:30
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