google / safeside

Understand and mitigate software-observable side-channels
BSD 3-Clause "New" or "Revised" License
498 stars 54 forks source link

Initial CMake support #13

Closed itzurabhi closed 5 years ago

itzurabhi commented 5 years ago

5 Added CMake build files,build instructions.

googlebot commented 5 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

itzurabhi commented 5 years ago

@googlebot I signed it!

googlebot commented 5 years ago

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

asteinha commented 5 years ago

Does this solution ensure that with GCC we build the examples with -O2? Usually it makes no substantial difference, just the demonstration executes slower, but for example on ARM it makes a difference - the binary compiled without optimizations might flake or not work at all.

itzurabhi commented 5 years ago

Regarding the question by @asteinha , we can set CMAKE_BUILD_TYPE while building. If -O2 is required explicitly then we need to add the specific flag to the compiler options.

https://stackoverflow.com/questions/41361631/optimize-in-cmake-by-default

The above link shows a relevant example.

Which is preferred ?

asteinha commented 5 years ago

Regarding the question by @asteinha , we can set CMAKE_BUILD_TYPE while building. If -O2 is required explicitly then we need to add the specific flag to the compiler options.

https://stackoverflow.com/questions/41361631/optimize-in-cmake-by-default

The above link shows a relevant example.

Which is preferred ?

The release build type should be fine. -O3 should work everywhere where -O2 works. Just make sure that it works for MSVC and that it creates a Release (not Debug) build for MSVC.

itzurabhi commented 5 years ago

@asteinha I have changed the build instructions to have Release mode. I do not have a MSVC setup to verify the optimization level.

googlebot commented 5 years ago

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

itzurabhi commented 5 years ago

This seems good to me! One nit on a variable name.

Can you please squash your commits before merge? Thanks.

Done.

mmdriley commented 5 years ago

Hm, is it me?

mmdriley commented 5 years ago

@googlebot I consent.

asteinha commented 5 years ago

It might be because of the mismatch between mdriley@gmail.com and mattdr@google.com, but I'm not completely sure. You might want to try to sign the CLA with the e-mail address that is listed as the co-author of the branch to be merged. Something similar happened to once upon a time I worked in Montreal.

googlebot commented 5 years ago

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

mmdriley commented 5 years ago

Yep, googlebot is worried about Co-Authored-By: Matthew Riley <mdriley@gmail.com>.

mmdriley commented 5 years ago

Merged. Thank you!