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

Enable sanitizers for extended CI host build #1114

Merged
merged 12 commits into from
Jun 21, 2024
Merged

Enable sanitizers for extended CI host build #1114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7d5ca8b
Enable sanitizers for extended CI host build
BenWibking Jun 14, 2024
1b04486
Update ci-extended.yml
BenWibking Jun 14, 2024
0cff441
Update CHANGELOG.md
BenWibking Jun 14, 2024
82cd3d9
Merge branch 'develop' into BenWibking/enable-asan-ci
BenWibking Jun 16, 2024
191338a
Merge branch 'develop' into BenWibking/enable-asan-ci
BenWibking Jun 17, 2024
697c751
Merge branch 'develop' into BenWibking/enable-asan-ci
pgrete Jun 18, 2024
c801ed9
disable leak detection for MPI runs
BenWibking Jun 18, 2024
222ba16
Merge branch 'develop' into BenWibking/enable-asan-ci
BenWibking Jun 19, 2024
810c8fd
Merge branch 'develop' into BenWibking/enable-asan-ci
BenWibking Jun 19, 2024
28dc14b
Merge branch 'develop' into BenWibking/enable-asan-ci
BenWibking Jun 20, 2024
6506acd
disable leak detection entirely
BenWibking Jun 20, 2024
d1bae35
Merge branch 'develop' into BenWibking/enable-asan-ci
BenWibking Jun 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .github/workflows/ci-extended.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,20 @@ jobs:
path: tst/regression/gold_standard/
key: gold-standard

- name: Set vars based on matrix
id: cmake-vars
run: |
if ${{ matrix.device == 'host' }}; then
echo "enable_asan=ON" >> $GITHUB_OUTPUT
else
echo "enable_asan=OFF" >> $GITHUB_OUTPUT
fi

- name: Configure
run: |
cmake -B build \
-DCMAKE_BUILD_TYPE=Release \
-DENABLE_ASAN=${{ steps.cmake-vars.outputs.enable_asan }} \
Comment on lines +47 to +60
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is that a common pattern?

Might sth like

-DENABLE_ASAN=${${{ matrix.device == 'host' }}^^} \

work, too?

Or if that one line logic doesn't work using an env var:

env:
  ENABLE_ASAN: ${{ matrix.device == 'host' }}

with (the ^^ to uppercase the lowercase result in the env var):

-DENABLE_ASAN=${ENABLE_ASAN^^} \

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Bash outputs 1 for true and 0 for false, so I don't think that works, unless CMake translates 1 to ON?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it should
"True if the constant is 1, ON, YES, TRUE, Y, or a non-zero number (including floating point numbers)." from https://cmake.org/cmake/help/latest/command/if.html

But I think GitHub actions only returns (lowercase) true or false for expressions so that's why we need to convert it to uppercase.

-DMACHINE_VARIANT=${{ matrix.device }}-${{ matrix.parallel }}

- name: Build
Expand All @@ -60,6 +70,10 @@ jobs:
cd build
# Pick GPU with most available memory
export CUDA_VISIBLE_DEVICES=$(nvidia-smi --query-gpu=memory.free,index --format=csv,nounits,noheader | sort -nr | head -1 | awk '{ print $NF }')
# Sanitizers options (leak detection is disabled)
export ASAN_OPTIONS=abort_on_error=1:fast_unwind_on_malloc=1
export UBSAN_OPTIONS=print_stacktrace=0
export LSAN_OPTIONS=detect_leaks=0
ctest -L performance -LE perf-reg

# run regression tests
Expand All @@ -68,6 +82,10 @@ jobs:
cd build
# Pick GPU with most available memory
export CUDA_VISIBLE_DEVICES=$(nvidia-smi --query-gpu=memory.free,index --format=csv,nounits,noheader | sort -nr | head -1 | awk '{ print $NF }')
# Sanitizers options (disable leak detection for MPI runs, due to OpenMPI leaks)
export ASAN_OPTIONS=abort_on_error=1:fast_unwind_on_malloc=1
export UBSAN_OPTIONS=print_stacktrace=0
export LSAN_OPTIONS=detect_leaks=0
ctest -L regression -L ${{ matrix.parallel }} -LE perf-reg --timeout 3600

# Test Ascent integration (only most complex setup with MPI and on device)
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
- [[PR 1031]](https://github.com/parthenon-hpc-lab/parthenon/pull/1031) Fix bug in non-cell centered AMR

### Infrastructure (changes irrelevant to downstream codes)
- [[PR 1114]](https://github.com/parthenon-hpc-lab/parthenon/pull/1114) Enable sanitizers for extended CI host build
- [[PR 1123]](https://github.com/parthenon-hpc-lab/parthenon/pull/1123) Default initialize ProResInfo.dir
- [[PR 1121]](https://github.com/parthenon-hpc-lab/parthenon/pull/1121) Default initialize BndInfo.dir
- [[PR 1116]](https://github.com/parthenon-hpc-lab/parthenon/pull/1116) Fix NumPy 2.0 test script breakage
Expand Down
Loading