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.

Sign up for GitHub

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

Jump to bottom

WT-9149 Missing sanitizer flags in Evergreen #8005

Merged
merged 2 commits into from
Jun 8, 2022
Merged

WT-9149 Missing sanitizer flags in Evergreen #8005

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5f65f9b
Moving race condition tests to compile wiredtiger address sanitizer
ruby-oujo Jun 7, 2022
841f527
Adding DHAVE_BUILTIN_EXTENSION_ZSTD=1 flag
ruby-oujo Jun 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 4 additions & 6 deletions test/evergreen.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ functions:
cd cmake_build
$CMAKE -DCMAKE_TOOLCHAIN_FILE=../cmake/toolchains/mongodbtoolchain_v4_clang.cmake -DCMAKE_C_FLAGS="-ggdb" -DWITH_PIC=1 \
-DENABLE_STRICT=1 -DHAVE_DIAGNOSTIC=1 ${NON_BARRIER_DIAGNOSTIC_YIELDS|} -DCMAKE_BUILD_TYPE=ASan \
-DHAVE_BUILTIN_EXTENSION_LZ4=1 -DHAVE_BUILTIN_EXTENSION_SNAPPY=1 -DHAVE_BUILTIN_EXTENSION_ZLIB=1 ${configure_python_setting|} \
-DHAVE_BUILTIN_EXTENSION_LZ4=1 -DHAVE_BUILTIN_EXTENSION_SNAPPY=1 -DHAVE_BUILTIN_EXTENSION_ZLIB=1 -DHAVE_BUILTIN_EXTENSION_ZSTD=1 ${configure_python_setting|} \
-G "${cmake_generator|Ninja}" ../.
fi
- *make_wiredtiger
Expand Down Expand Up @@ -749,9 +749,7 @@ variables:
exec_timeout_secs: 25200
commands:
- func: "get project"
- func: "compile wiredtiger"
vars:
posix_configure_flags: -DENABLE_STRICT=1 -DHAVE_DIAGNOSTIC=1 -DHAVE_BUILTIN_EXTENSION_LZ4=1 -DHAVE_BUILTIN_EXTENSION_SNAPPY=1 -DHAVE_BUILTIN_EXTENSION_ZLIB=1 -DHAVE_BUILTIN_EXTENSION_ZSTD=1
- func: "compile wiredtiger address sanitizer"
- func: "format test script"
vars:
format_test_script_args: -R -t 360
Expand Down Expand Up @@ -2883,9 +2881,9 @@ tasks:
exec_timeout_secs: 25200
commands:
- func: "get project"
- func: "compile wiredtiger"
- func: "compile wiredtiger address sanitizer"
vars:
posix_configure_flags: -DENABLE_STRICT=1 -DHAVE_DIAGNOSTIC=1 -DNON_BARRIER_DIAGNOSTIC_YIELDS=1 -DHAVE_BUILTIN_EXTENSION_LZ4=1 -DHAVE_BUILTIN_EXTENSION_SNAPPY=1 -DHAVE_BUILTIN_EXTENSION_ZLIB=1 -DHAVE_BUILTIN_EXTENSION_ZSTD=1
Copy link
Contributor

Choose a reason for hiding this comment

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

compile wiredtiger address sanitizer uses -DHAVE_BUILTIN_EXTENSION_ZLIB=1 while here we have -DHAVE_BUILTIN_EXTENSION_ZSTD=1, is that an issue? Same question for race-condition-stress-sanitizer-test.

Copy link
Contributor Author

@ruby-oujo ruby-oujo Jun 7, 2022 •
edited

Choose a reason for hiding this comment

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

I believe ZSTD is faster than ZLIB but does same thing. I will add in DHAVE_BUILTIN_EXTENSION_ZSTD=1 as it seems like it was being used before. Thanks for spotting!

NON_BARRIER_DIAGNOSTIC_YIELDS: -DNON_BARRIER_DIAGNOSTIC_YIELDS=1
Copy link
Contributor

@etienneptl etienneptl Jun 7, 2022 •
edited

Choose a reason for hiding this comment

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

I am not a fan of having such a specific field. Can we have instead something like extra_flags?

extra_flags: -DNON_BARRIER_DIAGNOSTIC_YIELDS=1

And then we would use it wherever needed:

{extra_flags|}

I just checked WT-9149 and @Siddhartha8899 suggested extra_configure_flags which works too.

Copy link
Contributor Author

@ruby-oujo ruby-oujo Jun 7, 2022 •
edited

Choose a reason for hiding this comment

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

In the function itself the variable is saved into NON_BARRIER_DIAGNOSTIC_YIELDS. This is how it is saved in two compile functions and passed in from many tests using this variable. If we want to change this we should change all of the areas and it is not within the scope of the current PR.

We can either adjust the current ticket scope or implement this change in the future when we need to add more flags.

Copy link
Contributor

Choose a reason for hiding this comment

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

or implement this change in the future when we need to add more flags

Sounds reasonable to me, thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

yup makes sense @ruby-oujo! We can just leave that for the future. If there is the need to add more flags then we can change that variable to extra_flags.

- func: "format test script"
vars:
format_test_script_args: -R -t 360
Expand Down